linux-kernel - Re: [PATCH 3/5] drm/msm/a6xx: Store gmu_cgc_mode in struct a6xx

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] [day] [month] [year] [list]

Message-ID: <CAF6AEGtAMF4efKZo-ogwXc5JGHdoYJ0EfMTtOrkF1J4=zc9yPg@mail.gmail.com>
Date: Wed, 28 Aug 2024 06:44:42 -0700
From: Rob Clark <robdclark@...il.com>
To: Konrad Dybcio <konradybcio@...nel.org>
Cc: Sean Paul <sean@...rly.run>, Abhinav Kumar <quic_abhinavk@...cinc.com>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, David Airlie <airlied@...il.com>, 
	Daniel Vetter <daniel@...ll.ch>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	Konrad Dybcio <konrad.dybcio@...aro.org>
Subject: Re: [PATCH 3/5] drm/msm/a6xx: Store gmu_cgc_mode in struct a6xx_info

On Wed, Aug 28, 2024 at 4:16 AM Konrad Dybcio <konradybcio@...nel.org> wrote:
>
> On 27.08.2024 10:12 PM, Rob Clark wrote:
> > resending with updated Konrad email addr
> >
> > On Mon, Aug 26, 2024 at 2:09 PM Rob Clark <robdclark@...il.com> wrote:
> >>
> >> On Mon, Aug 26, 2024 at 2:07 PM Rob Clark <robdclark@...il.com> wrote:
> >>>
> >>> On Fri, Jul 19, 2024 at 3:03 AM Konrad Dybcio <konrad.dybcio@...aro.org> wrote:
> >>>>
> >>>> This was apparently almost never set on a6xx.. move the existing values
> >>>> and fill out the remaining ones within the catalog.
> >>>>
> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> >>>> ---
>
> [...]
>
> >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >>>> @@ -402,7 +402,7 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
> >>>>         struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
> >>>>         const struct adreno_reglist *reg;
> >>>>         unsigned int i;
> >>>> -       u32 val, clock_cntl_on, cgc_mode;
> >>>> +       u32 val, clock_cntl_on;
> >>>>
> >>>>         if (!(adreno_gpu->info->a6xx->hwcg || adreno_is_a7xx(adreno_gpu)))
> >>>>                 return;
> >>>> @@ -417,10 +417,8 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
> >>>>                 clock_cntl_on = 0x8aa8aa82;
> >>>>
> >>>>         if (adreno_is_a7xx(adreno_gpu)) {
> >>>> -               cgc_mode = adreno_is_a740_family(adreno_gpu) ? 0x20222 : 0x20000;
> >>>> -
> >>>
> >>> This does appear to change the gmu_cgc_mode in nearly all cases.. was
> >>> this intended?
> >>
> >> Hmm, and this will only get written for a7xx, so we're dropping the
> >> reg write for a690..
>
> Right, this patch is a lot to chew through.. It:
>
> - adds the proper magic value per gpu gen
> - removes the sneaky a690 write
> - uses the new struct entry
>
> but also
>
> - fails to remove the if (a7xx) check
>
> so I suppose for v2 I can split it into:
>
> 1. add the magic values
> 2. fix the if (a7xx) check
> 3. use the struct value and drop the a690 one
>
> does that sound good?

Yeah, I would prefer if it were split up to make it clear that the
magic value changes were intentional

ps.  there is a _bit_ more time to get this in to msm-next for v6.12,
but not much

BR,
-R

> Konrad