On 1.02.2023 10:30, Akhil P Oommen wrote: > On 1/26/2023 8:46 PM, Konrad Dybcio wrote: >> Port setting min_access_length, ubwc_mode and upper_bit from downstream. >> Values were validated using downstream device trees for SM8[123]50 and >> left default (as per downstream) elsewhere. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 26 ++++++++++++++++++-------- >> 1 file changed, 18 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index c5f5d0bb3fdc..ad5d791b804c 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -786,17 +786,22 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu) >> static void a6xx_set_ubwc_config(struct msm_gpu *gpu) >> { >> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >> - u32 lower_bit = 2; >> + u32 lower_bit = 1; > Wouldn't this break a630? // highest_bank_bit = 15 on 845, from "qcom,highest-bank-bit" in dt bit = adreno_dev->highest_bank_bit ? adreno_dev->highest_bank_bit - 13 : 0; // => bit = 2 lower_bit = bit & 0x3; // => lower_bit = 2 Yes it would! Thanks for catching that, I'll add the A630 case in v2. The 1 default value comes from the fact that highest_bank_bit is 13 when it's unset in dt, which makes lower_bit 1. Konrad > > -Akhil. >> + u32 upper_bit = 0; >> u32 amsbc = 0; >> u32 rgb565_predicator = 0; >> u32 uavflagprd_inv = 0; >> + u32 min_acc_len = 0; >> + u32 ubwc_mode = 0; >> >> /* a618 is using the hw default values */ >> if (adreno_is_a618(adreno_gpu)) >> return; >> >> - if (adreno_is_a640_family(adreno_gpu)) >> + if (adreno_is_a640_family(adreno_gpu)) { >> amsbc = 1; >> + lower_bit = 2; >> + } >> >> if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu)) { >> /* TODO: get ddr type from bootloader and use 2 for LPDDR4 */ >> @@ -807,18 +812,23 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu) >> } >> >> if (adreno_is_7c3(adreno_gpu)) { >> - lower_bit = 1; >> amsbc = 1; >> rgb565_predicator = 1; >> uavflagprd_inv = 2; >> } >> >> gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, >> - rgb565_predicator << 11 | amsbc << 4 | lower_bit << 1); >> - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, lower_bit << 1); >> - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, >> - uavflagprd_inv << 4 | lower_bit << 1); >> - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, lower_bit << 21); >> + rgb565_predicator << 11 | upper_bit << 10 | amsbc << 4 | >> + min_acc_len << 3 | lower_bit << 1 | ubwc_mode); >> + >> + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, upper_bit << 4 | >> + min_acc_len << 3 | lower_bit << 1 | ubwc_mode); >> + >> + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, upper_bit << 10 | >> + uavflagprd_inv << 4 | min_acc_len << 3 | >> + lower_bit << 1 | ubwc_mode); >> + >> + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, min_acc_len << 23 | lower_bit << 21); >> } >> >> static int a6xx_cp_init(struct msm_gpu *gpu) >