Re: [PATCH 4/6] drm/msm/mdss: populate missing data

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 5/21/2023 10:10 AM, Dmitry Baryshkov wrote:
> As we are going to use MDSS data for DPU programming, populate missing
> MDSS data. The UBWC 1.0 and no UBWC cases do not require MDSS
> programming, so skip them.
> 

Can you pls point me to the downstream references you used for msm8998?

Was that just taken from catalog? If so I would ask for the reference 
for the catalog too.

As per the register the decoder version is 0x0 and not 1.

Even encoder version is 0 from what i see and not 1. Thats why a 
dec_version of UBWC_1_0 is not doing anything i assume.

Some additional questions:

1) Does the whole chunk in dpu_hw_sspp_setup_format() which handles ubwc 
programming need to be protected by if (ctx->ubwc) now ?

2) The values of UBWC_x_x dont match the values of DPU_HW_UBWC_VER_xx.
What was the reason for the catalog to go with DPU_HW_UBWC_VER_xx in the 
catalog for the encoder version in the first place? Because looking at 
the registers UBWC_x_x is the correct value.


> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index ed836c659688..9bb7be4b9ebb 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -264,6 +264,10 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
>   	 * UBWC_n and the rest of params comes from hw data.
>   	 */
>   	switch (msm_mdss->mdss_data->ubwc_dec_version) {
> +	case 0: /* no UBWC */
> +	case UBWC_1_0:
> +		/* do nothing */
> +		break;
>   	case UBWC_2_0:
>   		msm_mdss_setup_ubwc_dec_20(msm_mdss);
>   		break;
> @@ -502,10 +506,22 @@ static int mdss_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +static const struct msm_mdss_data msm8998_data = {
> +	.ubwc_enc_version = UBWC_1_0,
> +	.ubwc_dec_version = UBWC_1_0,
> +	.highest_bank_bit = 1,
> +};
> +
> +static const struct msm_mdss_data qcm2290_data = {
> +	/* no UBWC */
> +	.highest_bank_bit = 0x2,
> +};
> +
>   static const struct msm_mdss_data sc7180_data = {
>   	.ubwc_enc_version = UBWC_2_0,
>   	.ubwc_dec_version = UBWC_2_0,
>   	.ubwc_static = 0x1e,
> +	.highest_bank_bit = 0x3,
>   };
>   
>   static const struct msm_mdss_data sc7280_data = {
> @@ -550,6 +566,7 @@ static const struct msm_mdss_data sm6115_data = {
>   	.ubwc_dec_version = UBWC_2_0,
>   	.ubwc_swizzle = 7,
>   	.ubwc_static = 0x11f,
> +	.highest_bank_bit = 0x1,
>   };
>   
>   static const struct msm_mdss_data sm8250_data = {
> @@ -574,8 +591,8 @@ static const struct msm_mdss_data sm8550_data = {
>   
>   static const struct of_device_id mdss_dt_match[] = {
>   	{ .compatible = "qcom,mdss" },
> -	{ .compatible = "qcom,msm8998-mdss" },
> -	{ .compatible = "qcom,qcm2290-mdss" },
> +	{ .compatible = "qcom,msm8998-mdss", .data = &msm8998_data },
> +	{ .compatible = "qcom,qcm2290-mdss", .data = &qcm2290_data },
>   	{ .compatible = "qcom,sdm845-mdss", .data = &sdm845_data },
>   	{ .compatible = "qcom,sc7180-mdss", .data = &sc7180_data },
>   	{ .compatible = "qcom,sc7280-mdss", .data = &sc7280_data },