Received-SPF: pass (google.com: domain of linux-kernel+bounces-56905-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <89d02d5c-4af1-9f40-483f-1efb39b2a33d@quicinc.com>
Date: Wed, 7 Feb 2024 10:25:35 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v3 1/4] drm/msm/mdss: generate MDSS data for MDP5
 platforms
Content-Language: en-US
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Rob Clark
	<robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
        Marijn Suijten
	<marijn.suijten@somainline.org>,
        David Airlie <airlied@gmail.com>, Daniel
 Vetter <daniel@ffwll.ch>
CC: <linux-arm-msm@vger.kernel.org>, <freedreno@lists.freedesktop.org>,
        <linux-kernel@vger.kernel.org>, <dri-devel@lists.freedesktop.org>
References: <20240106-fd-migrate-mdp5-v3-0-3d2750378063@linaro.org>
 <20240106-fd-migrate-mdp5-v3-1-3d2750378063@linaro.org>
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
In-Reply-To: <20240106-fd-migrate-mdp5-v3-1-3d2750378063@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote:
> Older (mdp5) platforms do not use per-SoC compatible strings. Instead
> they use a single compat entry 'qcom,mdss'. To facilitate migrating
> these platforms to the DPU driver provide a way to generate the MDSS /
> UBWC data at runtime, when the DPU driver asks for it.
> 
> It is not possible to generate this data structure at the probe time,
> since some platforms might not have MDP_CLK enabled, which makes reading
> HW_REV register return 0.
> 

I would have expected a crash if clock was not enabled and we tried to 
access the hw_rev register.

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c | 51 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 455b2e3a0cdd..566a5dd5b8e8 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -3,6 +3,7 @@
>    * Copyright (c) 2018, The Linux Foundation
>    */
>   
> +#include <linux/bitfield.h>
>   #include <linux/clk.h>
>   #include <linux/delay.h>
>   #include <linux/interconnect.h>
> @@ -213,6 +214,49 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
>   	}
>   }
>   
> +#define MDSS_HW_MAJ_MIN		GENMASK(31, 16)
> +
> +#define MDSS_HW_MSM8996		0x1007
> +#define MDSS_HW_MSM8937		0x100e
> +#define MDSS_HW_MSM8956		0x1010

This should be 0x100B in the docs I see.

> +#define MDSS_HW_MSM8998		0x3000
> +#define MDSS_HW_SDM660		0x3002
> +#define MDSS_HW_SDM630		0x3003
> +
> +/*
> + * MDP5 platforms use generic qcom,mdp5 compat string, so we have to generate this data
> + */
> +static const struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss)
> +{
> +	struct msm_mdss_data *data;
> +	u32 hw_rev;
> +
> +	data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return NULL;
> +
> +	hw_rev = readl_relaxed(mdss->mmio + HW_REV);
> +	hw_rev = FIELD_GET(MDSS_HW_MAJ_MIN, hw_rev);
> +
> +	if (hw_rev == MDSS_HW_MSM8996 ||
> +	    hw_rev == MDSS_HW_MSM8937 ||
> +	    hw_rev == MDSS_HW_MSM8956 ||
> +	    hw_rev == MDSS_HW_MSM8998 ||
> +	    hw_rev == MDSS_HW_SDM660 ||
> +	    hw_rev == MDSS_HW_SDM630) {
> +		data->ubwc_dec_version = UBWC_1_0;
> +		data->ubwc_enc_version = UBWC_1_0;
> +	}
> +
> +	if (hw_rev == MDSS_HW_MSM8996 ||
> +	    hw_rev == MDSS_HW_MSM8998)
> +		data->highest_bank_bit = 2;
> +	else
> +		data->highest_bank_bit = 1;
> +
> +	return data;
> +}
> +
>   const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev)
>   {
>   	struct msm_mdss *mdss;
> @@ -222,6 +266,13 @@ const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev)
>   
>   	mdss = dev_get_drvdata(dev);
>   
> +	/*
> +	 * We could not do it at the probe time, since hw revision register was
> +	 * not readable. Fill data structure now for the MDP5 platforms.
> +	 */
> +	if (!mdss->mdss_data && mdss->is_mdp5)
> +		mdss->mdss_data = msm_mdss_generate_mdp5_mdss_data(mdss);
> +
>   	return mdss->mdss_data;
>   }
>   
>