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]
Message-ID: <CACu1E7EFK7dzR=hm-J58jz77pMxn2SoJVrfQvV0RHiESi1mkzA@mail.gmail.com>
Date: Thu, 8 May 2025 14:33:08 -0400
From: Connor Abbott <cwabbott0@...il.com>
To: Konrad Dybcio <konradybcio@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, Rob Clark <robdclark@...il.com>, 
	Abhinav Kumar <quic_abhinavk@...cinc.com>, Dmitry Baryshkov <lumag@...nel.org>, 
	Akhil P Oommen <quic_akhilpo@...cinc.com>, Sean Paul <sean@...rly.run>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Marijn Suijten <marijn.suijten@...ainline.org>, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	freedreno@...ts.freedesktop.org, 
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH RFT 10/14] drm/msm/a6xx: Stop tracking macrotile_mode (again)

On Thu, May 8, 2025 at 2:14 PM Konrad Dybcio <konradybcio@...nel.org> wrote:
>
> From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
>
> SC8180X (A680) and SA8775P (A663) require a write to that register,
> while other SKUs are fine with the default value. Don't overwrite it
> needlessly, requiring the developer to read the value back from
> hardware just to put it in the driver again, introducing much more room
> for error.

I'm not sure I understand that last sentence. The original reason I
always wrote it was that for host image copy we need to know the value
of macrotile_mode, so again the value exposed to userspace must match
what's set in the HW. We can't read the value from the HW and send it
to userspace, because userspace queries this when creating the
physical device during device enumeration and we really don't want to
spuriously turn on the device then. That means the safest thing is to
always program it, guaranteeing that it always matches. Otherwise we
just have to hope that the default value matches what we expect it to
be.

I know you're copying this from kgsl, but kgsl doesn't expose the
macrotile_mode to userspace. I expect that HIC was added afterwards
and only works via hacks there (if it's even supported at all on the
relevant SoCs).

Connor

>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 60f89a2d851a5c383fc14cce4c483f630132a9a6..bee7e9685aa3ea282fb20ef479e4d243d28418f7 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -594,7 +594,6 @@ static int a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>
>         gpu->ubwc_config.min_acc_len = 0;
>         gpu->ubwc_config.ubwc_swizzle = 0x6;
> -       gpu->ubwc_config.macrotile_mode = 0;
>         gpu->ubwc_config.highest_bank_bit = 2;
>
>         if (adreno_is_a610(gpu)) {
> @@ -616,13 +615,8 @@ static int a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>         if (adreno_is_a621(gpu))
>                 gpu->ubwc_config.highest_bank_bit = 0;
>
> -       if (adreno_is_a623(gpu)) {
> +       if (adreno_is_a623(gpu))
>                 gpu->ubwc_config.highest_bank_bit = 3;
> -               gpu->ubwc_config.macrotile_mode = 1;
> -       }
> -
> -       if (adreno_is_a680(gpu))
> -               gpu->ubwc_config.macrotile_mode = 1;
>
>         if (adreno_is_a650(gpu) ||
>             adreno_is_a660(gpu) ||
> @@ -631,19 +625,15 @@ static int a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>             adreno_is_a740_family(gpu)) {
>                 /* TODO: get ddr type from bootloader and use 2 for LPDDR4 */
>                 gpu->ubwc_config.highest_bank_bit = 3;
> -               gpu->ubwc_config.macrotile_mode = 1;
>         }
>
>         if (adreno_is_a663(gpu)) {
>                 gpu->ubwc_config.highest_bank_bit = 0;
> -               gpu->ubwc_config.macrotile_mode = 1;
>                 gpu->ubwc_config.ubwc_swizzle = 0x4;
>         }
>
> -       if (adreno_is_7c3(gpu)) {
> +       if (adreno_is_7c3(gpu))
>                 gpu->ubwc_config.highest_bank_bit = 1;
> -               gpu->ubwc_config.macrotile_mode = 1;
> -       }
>
>         if (adreno_is_a702(gpu)) {
>                 gpu->ubwc_config.highest_bank_bit = 1;
> @@ -691,8 +681,9 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
>         gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL,
>                   adreno_gpu->ubwc_config.min_acc_len << 23 | hbb_lo << 21);
>
> -       gpu_write(gpu, REG_A6XX_RBBM_NC_MODE_CNTL,
> -                 adreno_gpu->ubwc_config.macrotile_mode);
> +       /* The reset value only needs altering in some cases */
> +       if (adreno_is_a680(adreno_gpu) || adreno_is_a663(adreno_gpu))
> +               gpu_write(gpu, REG_A6XX_RBBM_NC_MODE_CNTL, BIT(0));
>  }
>
>  static void a7xx_patch_pwrup_reglist(struct msm_gpu *gpu)
>
> --
> 2.49.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ