On 5/14/25 9:22 PM, Dmitry Baryshkov wrote:
> On Wed, May 14, 2025 at 05:10:31PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dyb...@oss.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.dyb...@oss.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.

I'd rather put the warn in ubwc_config.c

Konrad

Reply via email to