[PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor

Hsiao Chien Sung posted 24 patches 1 year ago
There is a newer version of this series
[PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor
Posted by Hsiao Chien Sung 1 year ago
Add MT8188 Padding to OVL adaptor to probe the driver.

Reviewed-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
index f46ca1c68610..f693b47745ba 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
@@ -29,6 +29,7 @@ enum mtk_ovl_adaptor_comp_type {
 	OVL_ADAPTOR_TYPE_ETHDR,
 	OVL_ADAPTOR_TYPE_MDP_RDMA,
 	OVL_ADAPTOR_TYPE_MERGE,
+	OVL_ADAPTOR_TYPE_PADDING,
 	OVL_ADAPTOR_TYPE_NUM,
 };
 
@@ -46,6 +47,14 @@ enum mtk_ovl_adaptor_comp_id {
 	OVL_ADAPTOR_MERGE1,
 	OVL_ADAPTOR_MERGE2,
 	OVL_ADAPTOR_MERGE3,
+	OVL_ADAPTOR_PADDING0,
+	OVL_ADAPTOR_PADDING1,
+	OVL_ADAPTOR_PADDING2,
+	OVL_ADAPTOR_PADDING3,
+	OVL_ADAPTOR_PADDING4,
+	OVL_ADAPTOR_PADDING5,
+	OVL_ADAPTOR_PADDING6,
+	OVL_ADAPTOR_PADDING7,
 	OVL_ADAPTOR_ID_MAX
 };
 
@@ -66,6 +75,7 @@ static const char * const private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = {
 	[OVL_ADAPTOR_TYPE_ETHDR]	= "ethdr",
 	[OVL_ADAPTOR_TYPE_MDP_RDMA]	= "vdo1-rdma",
 	[OVL_ADAPTOR_TYPE_MERGE]	= "merge",
+	[OVL_ADAPTOR_TYPE_PADDING]	= "padding",
 };
 
 static const struct mtk_ddp_comp_funcs ethdr = {
@@ -80,6 +90,13 @@ static const struct mtk_ddp_comp_funcs merge = {
 	.clk_disable = mtk_merge_clk_disable,
 };
 
+static const struct mtk_ddp_comp_funcs padding = {
+	.clk_enable = mtk_padding_clk_enable,
+	.clk_disable = mtk_padding_clk_disable,
+	.start = mtk_padding_start,
+	.stop = mtk_padding_stop,
+};
+
 static const struct mtk_ddp_comp_funcs rdma = {
 	.power_on = mtk_mdp_rdma_power_on,
 	.power_off = mtk_mdp_rdma_power_off,
@@ -101,6 +118,14 @@ static const struct ovl_adaptor_comp_match comp_matches[OVL_ADAPTOR_ID_MAX] = {
 	[OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE2, 2, &merge },
 	[OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE3, 3, &merge },
 	[OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE4, 4, &merge },
+	[OVL_ADAPTOR_PADDING0] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING0, 0, &padding },
+	[OVL_ADAPTOR_PADDING1] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING1, 1, &padding },
+	[OVL_ADAPTOR_PADDING2] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING2, 2, &padding },
+	[OVL_ADAPTOR_PADDING3] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING3, 3, &padding },
+	[OVL_ADAPTOR_PADDING4] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING4, 4, &padding },
+	[OVL_ADAPTOR_PADDING5] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING5, 5, &padding },
+	[OVL_ADAPTOR_PADDING6] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING6, 6, &padding },
+	[OVL_ADAPTOR_PADDING7] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING7, 7, &padding },
 };
 
 void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
@@ -436,6 +461,7 @@ static int ovl_adaptor_comp_get_id(struct device *dev, struct device_node *node,
 }
 
 static const struct of_device_id mtk_ovl_adaptor_comp_dt_ids[] = {
+	{ .compatible = "mediatek,mt8188-padding", .data = (void *)OVL_ADAPTOR_TYPE_PADDING },
 	{ .compatible = "mediatek,mt8195-disp-ethdr", .data = (void *)OVL_ADAPTOR_TYPE_ETHDR },
 	{ .compatible = "mediatek,mt8195-disp-merge", .data = (void *)OVL_ADAPTOR_TYPE_MERGE },
 	{ .compatible = "mediatek,mt8195-vdo1-rdma", .data = (void *)OVL_ADAPTOR_TYPE_MDP_RDMA },
-- 
2.18.0
Re: [PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor
Posted by AngeloGioacchino Del Regno 1 year ago
Il 19/10/23 07:56, Hsiao Chien Sung ha scritto:
> Add MT8188 Padding to OVL adaptor to probe the driver.
> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>   .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   | 26 +++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index f46ca1c68610..f693b47745ba 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -29,6 +29,7 @@ enum mtk_ovl_adaptor_comp_type {
>   	OVL_ADAPTOR_TYPE_ETHDR,
>   	OVL_ADAPTOR_TYPE_MDP_RDMA,
>   	OVL_ADAPTOR_TYPE_MERGE,
> +	OVL_ADAPTOR_TYPE_PADDING,
>   	OVL_ADAPTOR_TYPE_NUM,
>   };
>   
> @@ -46,6 +47,14 @@ enum mtk_ovl_adaptor_comp_id {
>   	OVL_ADAPTOR_MERGE1,
>   	OVL_ADAPTOR_MERGE2,
>   	OVL_ADAPTOR_MERGE3,
> +	OVL_ADAPTOR_PADDING0,
> +	OVL_ADAPTOR_PADDING1,
> +	OVL_ADAPTOR_PADDING2,
> +	OVL_ADAPTOR_PADDING3,
> +	OVL_ADAPTOR_PADDING4,
> +	OVL_ADAPTOR_PADDING5,
> +	OVL_ADAPTOR_PADDING6,
> +	OVL_ADAPTOR_PADDING7,
>   	OVL_ADAPTOR_ID_MAX
>   };
>   
> @@ -66,6 +75,7 @@ static const char * const private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = {
>   	[OVL_ADAPTOR_TYPE_ETHDR]	= "ethdr",
>   	[OVL_ADAPTOR_TYPE_MDP_RDMA]	= "vdo1-rdma",
>   	[OVL_ADAPTOR_TYPE_MERGE]	= "merge",
> +	[OVL_ADAPTOR_TYPE_PADDING]	= "padding",
>   };
>   
>   static const struct mtk_ddp_comp_funcs ethdr = {
> @@ -80,6 +90,13 @@ static const struct mtk_ddp_comp_funcs merge = {
>   	.clk_disable = mtk_merge_clk_disable,
>   };
>   
> +static const struct mtk_ddp_comp_funcs padding = {
> +	.clk_enable = mtk_padding_clk_enable,
> +	.clk_disable = mtk_padding_clk_disable,
> +	.start = mtk_padding_start,
> +	.stop = mtk_padding_stop,
> +};
> +
>   static const struct mtk_ddp_comp_funcs rdma = {
>   	.power_on = mtk_mdp_rdma_power_on,
>   	.power_off = mtk_mdp_rdma_power_off,
> @@ -101,6 +118,14 @@ static const struct ovl_adaptor_comp_match comp_matches[OVL_ADAPTOR_ID_MAX] = {
>   	[OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE2, 2, &merge },
>   	[OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE3, 3, &merge },
>   	[OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, DDP_COMPONENT_MERGE4, 4, &merge },
> +	[OVL_ADAPTOR_PADDING0] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING0, 0, &padding },
> +	[OVL_ADAPTOR_PADDING1] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING1, 1, &padding },
> +	[OVL_ADAPTOR_PADDING2] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING2, 2, &padding },
> +	[OVL_ADAPTOR_PADDING3] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING3, 3, &padding },
> +	[OVL_ADAPTOR_PADDING4] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING4, 4, &padding },
> +	[OVL_ADAPTOR_PADDING5] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING5, 5, &padding },
> +	[OVL_ADAPTOR_PADDING6] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING6, 6, &padding },
> +	[OVL_ADAPTOR_PADDING7] = { OVL_ADAPTOR_TYPE_PADDING, DDP_COMPONENT_PADDING7, 7, &padding },
>   };
>   
>   void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
> @@ -436,6 +461,7 @@ static int ovl_adaptor_comp_get_id(struct device *dev, struct device_node *node,
>   }
>   
>   static const struct of_device_id mtk_ovl_adaptor_comp_dt_ids[] = {
> +	{ .compatible = "mediatek,mt8188-padding", .data = (void *)OVL_ADAPTOR_TYPE_PADDING },

Uhm, for consistency I'd call this "mediatek,mt8188-disp-padding" (you don't have
to drop Reviewed-by tags for such a change, not here and not in the yaml commit),
but it's fine if you have reasons against that.

So, regardless of this being changed or not

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

>   	{ .compatible = "mediatek,mt8195-disp-ethdr", .data = (void *)OVL_ADAPTOR_TYPE_ETHDR },
>   	{ .compatible = "mediatek,mt8195-disp-merge", .data = (void *)OVL_ADAPTOR_TYPE_MERGE },
>   	{ .compatible = "mediatek,mt8195-vdo1-rdma", .data = (void *)OVL_ADAPTOR_TYPE_MDP_RDMA },
Re: [PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor
Posted by Shawn Sung (宋孝謙) 1 year ago
Hi Angelo,

On Thu, 2023-10-19 at 11:10 +0200, AngeloGioacchino Del Regno wrote:  
> >   static const struct of_device_id mtk_ovl_adaptor_comp_dt_ids[] =
> > {
> > +	{ .compatible = "mediatek,mt8188-padding", .data = (void
> > *)OVL_ADAPTOR_TYPE_PADDING },
> 
> Uhm, for consistency I'd call this "mediatek,mt8188-disp-padding"
> (you don't have
> to drop Reviewed-by tags for such a change, not here and not in the
> yaml commit),
> but it's fine if you have reasons against that.
> 
> So, regardless of this being changed or not
> 
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> 
> >   	{ .compatible = "mediatek,mt8195-disp-ethdr", .data = (void
> > *)OVL_ADAPTOR_TYPE_ETHDR },
> >   	{ .compatible = "mediatek,mt8195-disp-merge", .data = (void
> > *)OVL_ADAPTOR_TYPE_MERGE },
> >   	{ .compatible = "mediatek,mt8195-vdo1-rdma", .data = (void
> > *)OVL_ADAPTOR_TYPE_MDP_RDMA },
> 

Thanks for pointing this out. Had changed Padding driver's name to
"mtk-disp-padding", but I just notice that Padding will also be used by
MDP and they will share the same driver with display. Should we change
the name again or is it just fine to use "mtk-disp-padding"?

Thanks,
Shawn
Re: [PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor
Posted by AngeloGioacchino Del Regno 1 year ago
Il 19/10/23 11:20, Shawn Sung (宋孝謙) ha scritto:
> Hi Angelo,
> 
> On Thu, 2023-10-19 at 11:10 +0200, AngeloGioacchino Del Regno wrote:
>>>    static const struct of_device_id mtk_ovl_adaptor_comp_dt_ids[] =
>>> {
>>> +	{ .compatible = "mediatek,mt8188-padding", .data = (void
>>> *)OVL_ADAPTOR_TYPE_PADDING },
>>
>> Uhm, for consistency I'd call this "mediatek,mt8188-disp-padding"
>> (you don't have
>> to drop Reviewed-by tags for such a change, not here and not in the
>> yaml commit),
>> but it's fine if you have reasons against that.
>>
>> So, regardless of this being changed or not
>>
>> Reviewed-by: AngeloGioacchino Del Regno <
>> angelogioacchino.delregno@collabora.com>
>>
>>>    	{ .compatible = "mediatek,mt8195-disp-ethdr", .data = (void
>>> *)OVL_ADAPTOR_TYPE_ETHDR },
>>>    	{ .compatible = "mediatek,mt8195-disp-merge", .data = (void
>>> *)OVL_ADAPTOR_TYPE_MERGE },
>>>    	{ .compatible = "mediatek,mt8195-vdo1-rdma", .data = (void
>>> *)OVL_ADAPTOR_TYPE_MDP_RDMA },
>>
> 
> Thanks for pointing this out. Had changed Padding driver's name to
> "mtk-disp-padding", but I just notice that Padding will also be used by
> MDP and they will share the same driver with display. Should we change
> the name again or is it just fine to use "mtk-disp-padding"?
> 

That's like many other components in MediaTek, so we can keep the mtk-disp-padding
name.... in devicetree, we will anyway use "mediatek,mt8195-mdp3-padding" as one of
the compatible string(s).

This is the only way that we have to actually distinguish between components used
for MDP3 and components used for the display subsystem, if we keep them "generic"
we won't understand what's going on in case of issues.

The driver name should contain "disp" for consistency with all of the component
drivers in mediatek-drm; if this wasn't in this folder, we could've dropped the
"disp" in the name, but that's not the case.

Consistency is #1.

Cheers,
Angelo

> Thanks,
> Shawn



Re: [PATCH v10 20/24] drm/mediatek: Add Padding to OVL adaptor
Posted by Shawn Sung (宋孝謙) 1 year ago
Hi Angelo,

On Thu, 2023-10-19 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
> 
> That's like many other components in MediaTek, so we can keep the
> mtk-disp-padding
> name.... in devicetree, we will anyway use "mediatek,mt8195-mdp3-
> padding" as one of
> the compatible string(s).
> 
> This is the only way that we have to actually distinguish between
> components used
> for MDP3 and components used for the display subsystem, if we keep
> them "generic"
> we won't understand what's going on in case of issues.
> 
> The driver name should contain "disp" for consistency with all of the
> component
> drivers in mediatek-drm; if this wasn't in this folder, we could've
> dropped the
> "disp" in the name, but that's not the case.
> 
> Consistency is #1.
> 
> Cheers,
> Angelo
> 

Got it. Thank you for making that clear.
Will change it in the next version.

Cheers,
Shawn