Re: [PATCH] drivers: gpu: drm: Add MDP5 configuration for MSM8x36 and its derivatives, such as MSM8939.

From: Shawn Guo <shawn.guo@linaro.org>
To: Konrad Dybcio <konradybcio@gmail.com>
Cc: skrzynka@konradybcio.pl, Rob Clark <robdclark@gmail.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	Jeffrey Hugo <jeffrey.l.hugo@gmail.com>,
	Allison Randal <allison@lohutok.net>,
	Alexios Zavras <alexios.zavras@intel.com>,
	zhengbin <zhengbin13@huawei.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	AngeloGioacchino Del Regno <kholk11@gmail.com>,
	Ben Dooks <ben.dooks@codethink.co.uk>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers: gpu: drm: Add MDP5 configuration for MSM8x36 and its derivatives, such as MSM8939.
Date: Fri, 8 May 2020 21:51:59 +0800	[thread overview]
Message-ID: <20200508135157.GD30486@dragon> (raw)
In-Reply-To: <20200501205201.149804-1-konradybcio@gmail.com>

Hi Konrad,

On Fri, May 01, 2020 at 10:51:59PM +0200, Konrad Dybcio wrote:
> Signed-off-by: Konrad Dybcio <konradybcio@gmail.com>

Please write up some commit log.  Since this is based on msm8x16_config,
maybe document the differences from it in commit log?

> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 70 ++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> index e3c4c250238b7..1c7de7d6870cf 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> @@ -342,6 +342,75 @@ static const struct mdp5_cfg_hw msm8x16_config = {
>  	.max_clk = 320000000,
>  };
>  
> +static const struct mdp5_cfg_hw msm8x36_config = {
> +	.name = "msm8x36",
> +	.mdp = {
> +		.count = 1,
> +		.base = { 0x0 },
> +		.caps = MDP_CAP_SMP |
> +			0,
> +	},
> +	.smp = {
> +		.mmb_count = 8,
> +		.mmb_size = 10240,
> +		.clients = {
> +			[SSPP_VIG0] = 1, [SSPP_DMA0] = 4,
> +			[SSPP_RGB0] = 7, [SSPP_RGB1] = 8,
> +		},
> +	},
> +	.ctl = {
> +		.count = 3,
> +		.base = { 0x01000, 0x01200, 0x01400 },
> +		.flush_hw_mask = 0x4003ffff,
> +	},
> +	.pipe_vig = {
> +		.count = 1,
> +		.base = { 0x04000 },
> +		.caps = MDP_PIPE_CAP_HFLIP | MDP_PIPE_CAP_VFLIP |
> +				MDP_PIPE_CAP_SCALE | MDP_PIPE_CAP_CSC |
> +				MDP_PIPE_CAP_DECIMATION,
> +	},
> +	.pipe_rgb = {
> +		.count = 2,
> +		.base = { 0x14000, 0x16000 },
> +		.caps = MDP_PIPE_CAP_HFLIP | MDP_PIPE_CAP_VFLIP |
> +				MDP_PIPE_CAP_DECIMATION,
> +	},
> +	.pipe_dma = {
> +		.count = 1,
> +		.base = { 0x24000 },
> +		.caps = MDP_PIPE_CAP_HFLIP | MDP_PIPE_CAP_VFLIP,
> +	},
> +	.lm = {
> +		.count = 1,

From what I read on downstream 3.10 kernel, there should be two mixers
just like msm8x16.

	qcom,mdss-mixer-intf-off = <0x00045000>;
	qcom,mdss-mixer-wb-off = <0x00048000>;

> +		.base = { 0x44000 },
> +		.instances = {
> +				{ .id = 0, .pp = 0, .dspp = 0,
> +				  .caps = MDP_LM_CAP_DISPLAY, },
> +				},
> +		.nb_stages = 8,
> +		.max_width = 2048,

It should probably be 2560 from downstream below.

	qcom,max-mixer-width = <2560>;

> +		.max_height = 0xFFFF,
> +	},
> +	.pp = {
> +		.count = 1,
> +		.base = { 0x70000 },
> +	},
> +

For consistency, we may want to drop this newline.  And it looks like
there is a .ad block on msm8x36.

	qcom,mdss-ad-off = <0x0079000>;

> +	.dspp = {
> +		.count = 1,
> +		.base = { 0x54000 },
> +	},
> +	.intf = {
> +		.base = { 0x00000, 0x6a800, 0x6b000 },
> +		.connect = {
> +			[0] = INTF_DISABLED,
> +			[1] = INTF_DSI,
> +			[2] = INTF_DSI,
> +		},
> +	},
> +	.max_clk = 366670000,
> +};

Need a newline here.

Shawn

>  static const struct mdp5_cfg_hw msm8x94_config = {
>  	.name = "msm8x94",
>  	.mdp = {
> @@ -840,6 +909,7 @@ static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
>  	{ .revision = 2, .config = { .hw = &msm8x74v2_config } },
>  	{ .revision = 3, .config = { .hw = &apq8084_config } },
>  	{ .revision = 6, .config = { .hw = &msm8x16_config } },
> +	{ .revision = 8, .config = { .hw = &msm8x36_config } },
>  	{ .revision = 9, .config = { .hw = &msm8x94_config } },
>  	{ .revision = 7, .config = { .hw = &msm8x96_config } },
>  	{ .revision = 11, .config = { .hw = &msm8x76_config } },
> -- 
> 2.26.1
> 