| Message ID | 20230214173145.2482651-3-konrad.dybcio@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/14] drm/msm/a6xx: De-staticize sptprac en/disable functions | expand |
On 14/02/2023 19:31, 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@linaro.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 29 +++++++++++++++++++-------- > 1 file changed, 21 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..8855d798bbb3 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -786,17 +786,25 @@ 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; Any reason to change the default value here? If it is to match chipsets you are adding, it might be worth splitting this change to that patch. > + 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_a630(adreno_gpu)) > + lower_bit = 2; > + > + 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 +815,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)
On 17.02.2023 21:46, Dmitry Baryshkov wrote: > On 14/02/2023 19:31, 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@linaro.org> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 29 +++++++++++++++++++-------- >> 1 file changed, 21 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..8855d798bbb3 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -786,17 +786,25 @@ 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; > > Any reason to change the default value here? > If it is to match chipsets you are adding, it might be worth splitting this change to that patch. Not really now that I think about it, especially since the correct default value should be zero: -- part of msm-4.19 -- bit = adreno_dev->highest_bank_bit ? adreno_dev->highest_bank_bit - 13 : 0; lower_bit = bit & 0x3; upper_bit = (bit >> 0x2) & 1; where adreno_dev->highest_bank_bit is read from the dt property "qcom,highest-bank-bit" Anyway, I should be able to verify it for all the SoCs which we support. Konrad > >> + 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_a630(adreno_gpu)) >> + lower_bit = 2; >> + >> + 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 +815,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) >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index c5f5d0bb3fdc..8855d798bbb3 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -786,17 +786,25 @@ 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; + 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_a630(adreno_gpu)) + lower_bit = 2; + + 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 +815,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)
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@linaro.org> --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 29 +++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-)