linux-kernel - Re: [PATCH v1 2/3] drm/msm/adreno: Add support for X185 GPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 26 Jun 2024 13:54:22 +0530
From: Akhil P Oommen <quic_akhilpo@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@...aro.org>
CC: freedreno <freedreno@...ts.freedesktop.org>,
        <dri-devel@...ts.freedesktop.org>, <linux-arm-msm@...r.kernel.org>,
        Rob Clark
	<robdclark@...il.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Abhinav Kumar
	<quic_abhinavk@...cinc.com>,
        Daniel Vetter <daniel@...ll.ch>, David Airlie
	<airlied@...il.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        "Marijn
 Suijten" <marijn.suijten@...ainline.org>,
        Sean Paul <sean@...rly.run>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/3] drm/msm/adreno: Add support for X185 GPU

On Mon, Jun 24, 2024 at 03:53:48PM +0200, Konrad Dybcio wrote:
> 
> 
> On 6/23/24 13:06, Akhil P Oommen wrote:
> > Add support in drm/msm driver for the Adreno X185 gpu found in
> > Snapdragon X1 Elite chipset.
> > 
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@...cinc.com>
> > ---
> > 
> >   drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 19 +++++++++++++++----
> >   drivers/gpu/drm/msm/adreno/a6xx_gpu.c      |  6 ++----
> >   drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  5 +++++
> >   4 files changed, 36 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 0e3dfd4c2bc8..168a4bddfaf2 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -830,8 +830,10 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, unsigned int state)
> >   	 */
> >   	gmu_write(gmu, REG_A6XX_GMU_CM3_CFG, 0x4052);
> > +	if (adreno_is_x185(adreno_gpu)) {
> > +		chipid = 0x7050001;
> 
> What's wrong with using the logic below?

patchid is BITS(7, 0), not (15, 8) in the case of x185. Due to the
changes in the chipid scheme within the a7x family, this is a bit
confusing. I will try to improve here in another series.

> 
> >   	/* NOTE: A730 may also fall in this if-condition with a future GMU fw update. */
> > -	if (adreno_is_a7xx(adreno_gpu) && !adreno_is_a730(adreno_gpu)) {
> > +	} else if (adreno_is_a7xx(adreno_gpu) && !adreno_is_a730(adreno_gpu)) {
> >   		/* A7xx GPUs have obfuscated chip IDs. Use constant maj = 7 */
> >   		chipid = FIELD_PREP(GENMASK(31, 24), 0x7);
> > @@ -1329,9 +1331,18 @@ static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
> >   	if (!pri_count)
> >   		return -EINVAL;
> > -	sec = cmd_db_read_aux_data("mx.lvl", &sec_count);
> > -	if (IS_ERR(sec))
> > -		return PTR_ERR(sec);
> > +	/*
> > +	 * Some targets have a separate gfx mxc rail. So try to read that first and then fall back
> > +	 * to regular mx rail if it is missing
> > +	 */
> > +	sec = cmd_db_read_aux_data("gmxc.lvl", &sec_count);
> > +	if (PTR_ERR_OR_ZERO(sec) == -EPROBE_DEFER) {
> > +		return -EPROBE_DEFER;
> > +	} else if (IS_ERR(sec)) {
> > +		sec = cmd_db_read_aux_data("mx.lvl", &sec_count);
> > +		if (IS_ERR(sec))
> > +			return PTR_ERR(sec);
> > +	}
> 
> I assume GMXC would always be used if present, although please use the
> approach Dmitry suggested

Correct.

-Akhil
> 
> 
> The rest looks good!
> 
> Konrad