linux-kernel - Re: [PATCH RFT v2 11/15] drm/msm/adreno: Switch to the common UBWC config struct

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: <qhmayxvlyz2txum5rs5vaf3iqzniz6nktr4zatru6bhgp6tdah@uq24obw2ro5f>
Date: Wed, 14 May 2025 22:22:56 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Konrad Dybcio <konradybcio@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, Rob Clark <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        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 v2 11/15] drm/msm/adreno: Switch to the common UBWC
 config struct

On Wed, May 14, 2025 at 05:10:31PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> 
> Now that Adreno specifics are out of the way, use the common config
> (but leave the HBB hardcoding in place until that is wired up on the
> other side).
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 20 +++++------
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 64 ++++++++++++++++-----------------
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  6 ++--
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h | 45 ++++-------------------
>  4 files changed, 50 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 650e5bac225f372e819130b891f1d020b464f17f..611e0eb26d0e19d431673d08e042162375fd400f 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -833,8 +833,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
>  
>  	gpu_write(gpu, REG_A5XX_RBBM_AHB_CNTL2, 0x0000003F);
>  
> -	BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13);
> -	hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13;
> +	BUG_ON(adreno_gpu->ubwc_config->highest_bank_bit < 13);
> +	hbb = adreno_gpu->ubwc_config->highest_bank_bit - 13;
>  
>  	gpu_write(gpu, REG_A5XX_TPL1_MODE_CNTL, hbb << 7);
>  	gpu_write(gpu, REG_A5XX_RB_MODE_CNTL, hbb << 1);
> @@ -1754,6 +1754,7 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
>  	struct msm_drm_private *priv = dev->dev_private;
>  	struct platform_device *pdev = priv->gpu_pdev;
>  	struct adreno_platform_config *config = pdev->dev.platform_data;
> +	const struct qcom_ubwc_cfg_data *common_cfg;
>  	struct a5xx_gpu *a5xx_gpu = NULL;
>  	struct adreno_gpu *adreno_gpu;
>  	struct msm_gpu *gpu;
> @@ -1790,15 +1791,14 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
>  	/* Set up the preemption specific bits and pieces for each ringbuffer */
>  	a5xx_preempt_init(gpu);
>  
> -	/* Set the highest bank bit */
> -	if (adreno_is_a540(adreno_gpu) || adreno_is_a530(adreno_gpu))
> -		adreno_gpu->ubwc_config.highest_bank_bit = 15;
> -	else
> -		adreno_gpu->ubwc_config.highest_bank_bit = 14;
> +	/* Inherit the common config and make some necessary fixups */
> +	common_cfg = qcom_ubwc_config_get_data();
> +	if (IS_ERR(common_cfg))
> +		return ERR_PTR(-EINVAL);
>  
> -	/* a5xx only supports UBWC 1.0, these are not configurable */
> -	adreno_gpu->ubwc_config.macrotile_mode = 0;
> -	adreno_gpu->ubwc_config.ubwc_swizzle = 0x7;
> +	/* Copy the data into the internal struct to drop the const qualifier (temporarily) */
> +	adreno_gpu->_ubwc_config = *common_cfg;
> +	adreno_gpu->ubwc_config = &adreno_gpu->_ubwc_config;

Ugh. I'd rather keep the ubwc config r/o.

>  
>  	adreno_gpu->uche_trap_base = 0x0001ffffffff0000ull;
>  
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index fdc843c47c075a92ec8305217c355e4ccee876dc..ae0bb7934e7ed203aa3b81e28767de204f0a4d60 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -587,64 +587,62 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
>  
>  static int a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>  {
> +	const struct qcom_ubwc_cfg_data *common_cfg;
> +	struct qcom_ubwc_cfg_data *cfg = &gpu->_ubwc_config;
> +
>  	/* Inherit the common config and make some necessary fixups */
> -	gpu->common_ubwc_cfg = qcom_ubwc_config_get_data();
> -	if (IS_ERR(gpu->common_ubwc_cfg))
> +	common_cfg = qcom_ubwc_config_get_data();
> +	if (IS_ERR(common_cfg))
>  		return -EINVAL;
>  
> -	gpu->ubwc_config.ubwc_swizzle = 0x6;
> -	gpu->ubwc_config.macrotile_mode = 0;
> -	gpu->ubwc_config.highest_bank_bit = 15;
> +	/* Copy the data into the internal struct to drop the const qualifier (temporarily) */
> +	*cfg = *common_cfg;
> +
> +	cfg->ubwc_swizzle = 0x6;
> +	cfg->highest_bank_bit = 15;
>  

This begs for WARN_ON(cfg->ubwc_swizzle !=
gpu->common_ubwc_cfg->ubwc_swizzle) and similar change for HBB. Then
after testing we should be able to drop r/w part of the config.


>  	if (adreno_is_a610(gpu)) {
> -		gpu->ubwc_config.highest_bank_bit = 13;
> -		gpu->ubwc_config.ubwc_swizzle = 0x7;
> +		cfg->highest_bank_bit = 13;
> +		cfg->ubwc_swizzle = 0x7;
>  	}
>  
> 

-- 
With best wishes
Dmitry