| Message ID | 20230911074233.31556-1-shawn.sung@mediatek.com |
|---|---|
| Headers | show |
| Series | Add display driver for MT8188 VDOSYS1 | expand |
Il 11/09/23 09:42, Hsiao Chien Sung ha scritto: > DPI input is in 1T2P mode on MT8195, > align the setting of MT8188 with it, > otherwise the screen will glitch. > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> First of all, this commit needs a Fixes tag... but then, instead of getting a 1:1 duplicate of mt8195_dpintf_conf you can, at this point, entirely remove mt8188_dpintf_conf and assign the 8195 one to the 8188 compatible: please do so. Thanks, Angelo > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 2f931e4e2b60..c6ee21e275ba 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -963,7 +963,7 @@ static const struct mtk_dpi_conf mt8188_dpintf_conf = { > .output_fmts = mt8195_output_fmts, > .num_output_fmts = ARRAY_SIZE(mt8195_output_fmts), > .pixels_per_iter = 4, > - .input_2pixel = false, > + .input_2pixel = true, > .dimension_mask = DPINTF_HPW_MASK, > .hvsize_mask = DPINTF_HSIZE_MASK, > .channel_swap_shift = DPINTF_CH_SWAP, > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > - Adjust indentation to align with other files > - Sort device table in alphabetical order > - Add sentinel to device table Reviewed-by: CK Hu <ck.hu@mediatek.com> > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 114eded8177e..4a5fab5ea51f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -386,17 +386,10 @@ 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,mt8195-vdo1-rdma", > - .data = (void *)OVL_ADAPTOR_TYPE_MDP_RDMA, > - }, { > - .compatible = "mediatek,mt8195-disp-merge", > - .data = (void *)OVL_ADAPTOR_TYPE_MERGE, > - }, { > - .compatible = "mediatek,mt8195-disp-ethdr", > - .data = (void *)OVL_ADAPTOR_TYPE_ETHDR, > - }, > - {}, > + { .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 }, > + { /* sentinel */ } > }; > > static int compare_of(struct device *dev, void *data) > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > Rename OVL_ADAPTOR_TYPE_RDMA to OVL_ADAPTOR_TYPE_MDP_RDMA > to align the naming rule of mtk_ovl_adaptor_comp_id. 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 | 22 +++++++++------ > ---- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 6bf6367853fb..114eded8177e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -27,7 +27,7 @@ > #define MTK_OVL_ADAPTOR_LAYER_NUM 4 > > enum mtk_ovl_adaptor_comp_type { > - OVL_ADAPTOR_TYPE_RDMA = 0, > + OVL_ADAPTOR_TYPE_MDP_RDMA = 0, > OVL_ADAPTOR_TYPE_MERGE, > OVL_ADAPTOR_TYPE_ETHDR, > OVL_ADAPTOR_TYPE_NUM, > @@ -62,20 +62,20 @@ struct mtk_disp_ovl_adaptor { > }; > > static const char * const private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = > { > - [OVL_ADAPTOR_TYPE_RDMA] = "vdo1-rdma", > + [OVL_ADAPTOR_TYPE_MDP_RDMA] = "vdo1-rdma", > [OVL_ADAPTOR_TYPE_MERGE] = "merge", > [OVL_ADAPTOR_TYPE_ETHDR] = "ethdr", > }; > > static const struct ovl_adaptor_comp_match > comp_matches[OVL_ADAPTOR_ID_MAX] = { > - [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_RDMA, 0 }, > - [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_RDMA, 1 }, > - [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_RDMA, 2 }, > - [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_RDMA, 3 }, > - [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_RDMA, 4 }, > - [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_RDMA, 5 }, > - [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_RDMA, 6 }, > - [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_RDMA, 7 }, > + [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 0 }, > + [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 1 }, > + [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 2 }, > + [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 3 }, > + [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 4 }, > + [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 5 }, > + [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 6 }, > + [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 7 }, > [OVL_ADAPTOR_MERGE0] = { OVL_ADAPTOR_TYPE_MERGE, 1 }, > [OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, 2 }, > [OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, 3 }, > @@ -388,7 +388,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,mt8195-vdo1-rdma", > - .data = (void *)OVL_ADAPTOR_TYPE_RDMA, > + .data = (void *)OVL_ADAPTOR_TYPE_MDP_RDMA, > }, { > .compatible = "mediatek,mt8195-disp-merge", > .data = (void *)OVL_ADAPTOR_TYPE_MERGE, > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > Sort OVL adaptor components' names in alphabetical order. 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 | 18 +++++++++------- > -- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 4a5fab5ea51f..72758e41b1e6 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -27,13 +27,14 @@ > #define MTK_OVL_ADAPTOR_LAYER_NUM 4 > > enum mtk_ovl_adaptor_comp_type { > - OVL_ADAPTOR_TYPE_MDP_RDMA = 0, > - OVL_ADAPTOR_TYPE_MERGE, > OVL_ADAPTOR_TYPE_ETHDR, > + OVL_ADAPTOR_TYPE_MDP_RDMA, > + OVL_ADAPTOR_TYPE_MERGE, > OVL_ADAPTOR_TYPE_NUM, > }; > > enum mtk_ovl_adaptor_comp_id { > + OVL_ADAPTOR_ETHDR0, > OVL_ADAPTOR_MDP_RDMA0, > OVL_ADAPTOR_MDP_RDMA1, > OVL_ADAPTOR_MDP_RDMA2, > @@ -46,7 +47,6 @@ enum mtk_ovl_adaptor_comp_id { > OVL_ADAPTOR_MERGE1, > OVL_ADAPTOR_MERGE2, > OVL_ADAPTOR_MERGE3, > - OVL_ADAPTOR_ETHDR0, > OVL_ADAPTOR_ID_MAX > }; > > @@ -62,12 +62,13 @@ struct mtk_disp_ovl_adaptor { > }; > > 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_ETHDR] = "ethdr", > }; > > static const struct ovl_adaptor_comp_match > comp_matches[OVL_ADAPTOR_ID_MAX] = { > + [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, 0 }, > [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 0 }, > [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 1 }, > [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 2 }, > @@ -80,7 +81,6 @@ static const struct ovl_adaptor_comp_match > comp_matches[OVL_ADAPTOR_ID_MAX] = { > [OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, 2 }, > [OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, 3 }, > [OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, 4 }, > - [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, 0 }, > }; > > void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int > idx, > @@ -314,6 +314,7 @@ size_t mtk_ovl_adaptor_get_num_formats(struct > device *dev) > > void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex > *mutex) > { > + mtk_mutex_add_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA0); > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA1); > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA2); > @@ -326,11 +327,11 @@ void mtk_ovl_adaptor_add_comp(struct device > *dev, struct mtk_mutex *mutex) > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE2); > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE3); > mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE4); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > } > > void mtk_ovl_adaptor_remove_comp(struct device *dev, struct > mtk_mutex *mutex) > { > + mtk_mutex_remove_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA0); > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA1); > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA2); > @@ -343,11 +344,11 @@ void mtk_ovl_adaptor_remove_comp(struct device > *dev, struct mtk_mutex *mutex) > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE2); > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE3); > mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE4); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > } > > void mtk_ovl_adaptor_connect(struct device *dev, struct device > *mmsys_dev, unsigned int next) > { > + mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, > next); > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA0, > DDP_COMPONENT_MERGE1); > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA1, > DDP_COMPONENT_MERGE1); > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA2, > DDP_COMPONENT_MERGE2); > @@ -355,11 +356,11 @@ void mtk_ovl_adaptor_connect(struct device > *dev, struct device *mmsys_dev, unsig > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE2, > DDP_COMPONENT_ETHDR_MIXER); > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE3, > DDP_COMPONENT_ETHDR_MIXER); > mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE4, > DDP_COMPONENT_ETHDR_MIXER); > - mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, > next); > } > > void mtk_ovl_adaptor_disconnect(struct device *dev, struct device > *mmsys_dev, unsigned int next) > { > + mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, > next); > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA0, > DDP_COMPONENT_MERGE1); > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA1, > DDP_COMPONENT_MERGE1); > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA2, > DDP_COMPONENT_MERGE2); > @@ -367,7 +368,6 @@ void mtk_ovl_adaptor_disconnect(struct device > *dev, struct device *mmsys_dev, un > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE2, > DDP_COMPONENT_ETHDR_MIXER); > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE3, > DDP_COMPONENT_ETHDR_MIXER); > mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE4, > DDP_COMPONENT_ETHDR_MIXER); > - mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, > next); > } > > static int ovl_adaptor_comp_get_id(struct device *dev, struct > device_node *node, > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > Add component ID to component match structure so we can > configure them with a for-loop. > > The main reason we do such code refactoring is that > there is a new hardware component called "Padding" since > MT8188, while MT8195 doesn't have this module, we can't > use the original logic to manage the components. > > While MT8195 does not define Padding in the device tree, > the corresponding components will be NULL and being skipped > by the functions. 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 | 69 ++++++++--------- > -- > 1 file changed, 30 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 72758e41b1e6..8a52d1301e04 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -52,6 +52,7 @@ enum mtk_ovl_adaptor_comp_id { > > struct ovl_adaptor_comp_match { > enum mtk_ovl_adaptor_comp_type type; > + enum mtk_ddp_comp_id comp_id; > int alias_id; > }; > > @@ -68,19 +69,19 @@ static const char * const > private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = { > }; > > static const struct ovl_adaptor_comp_match > comp_matches[OVL_ADAPTOR_ID_MAX] = { > - [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, 0 }, > - [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 0 }, > - [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 1 }, > - [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 2 }, > - [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 3 }, > - [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 4 }, > - [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 5 }, > - [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 6 }, > - [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_MDP_RDMA, 7 }, > - [OVL_ADAPTOR_MERGE0] = { OVL_ADAPTOR_TYPE_MERGE, 1 }, > - [OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, 2 }, > - [OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, 3 }, > - [OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, 4 }, > + [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, > DDP_COMPONENT_ETHDR_MIXER, 0 }, > + [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA0, 0 }, > + [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA1, 1 }, > + [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA2, 2 }, > + [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA3, 3 }, > + [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA4, 4 }, > + [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA5, 5 }, > + [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA6, 6 }, > + [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA7, 7 }, > + [OVL_ADAPTOR_MERGE0] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE1, 1 }, > + [OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE2, 2 }, > + [OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE3, 3 }, > + [OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE4, 4 }, > }; > > void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int > idx, > @@ -314,36 +315,26 @@ size_t mtk_ovl_adaptor_get_num_formats(struct > device *dev) > > void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex > *mutex) > { > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA0); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA1); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA2); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA3); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA4); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA5); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA6); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA7); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE1); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE2); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE3); > - mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE4); > + int i; > + struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > + > + for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) { > + if (!ovl_adaptor->ovl_adaptor_comp[i]) > + continue; > + mtk_mutex_add_comp(mutex, comp_matches[i].comp_id); > + } > } > > void mtk_ovl_adaptor_remove_comp(struct device *dev, struct > mtk_mutex *mutex) > { > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_ETHDR_MIXER); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA0); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA1); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA2); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA3); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA4); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA5); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA6); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA7); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE1); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE2); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE3); > - mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE4); > + int i; > + struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > + > + for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) { > + if (!ovl_adaptor->ovl_adaptor_comp[i]) > + continue; > + mtk_mutex_remove_comp(mutex, comp_matches[i].comp_id); > + } > } > > void mtk_ovl_adaptor_connect(struct device *dev, struct device > *mmsys_dev, unsigned int next) > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > By registering component related functions to the pointers, > we can easily manage them within a for-loop and simplify the > logic of clock control significantly. 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 | 111 +++++++--------- > -- > 1 file changed, 44 insertions(+), 67 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 8a52d1301e04..84133303a6ec 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -54,6 +54,7 @@ struct ovl_adaptor_comp_match { > enum mtk_ovl_adaptor_comp_type type; > enum mtk_ddp_comp_id comp_id; > int alias_id; > + const struct mtk_ddp_comp_funcs *funcs; > }; > > struct mtk_disp_ovl_adaptor { > @@ -68,20 +69,35 @@ static const char * const > private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = { > [OVL_ADAPTOR_TYPE_MERGE] = "merge", > }; > > +static const struct mtk_ddp_comp_funcs _ethdr = { > + .clk_enable = mtk_ethdr_clk_enable, > + .clk_disable = mtk_ethdr_clk_disable, > +}; > + > +static const struct mtk_ddp_comp_funcs _merge = { > + .clk_enable = mtk_merge_clk_enable, > + .clk_disable = mtk_merge_clk_disable, > +}; > + > +static const struct mtk_ddp_comp_funcs _rdma = { > + .clk_enable = mtk_mdp_rdma_clk_enable, > + .clk_disable = mtk_mdp_rdma_clk_disable, > +}; > + > static const struct ovl_adaptor_comp_match > comp_matches[OVL_ADAPTOR_ID_MAX] = { > - [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, > DDP_COMPONENT_ETHDR_MIXER, 0 }, > - [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA0, 0 }, > - [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA1, 1 }, > - [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA2, 2 }, > - [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA3, 3 }, > - [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA4, 4 }, > - [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA5, 5 }, > - [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA6, 6 }, > - [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA7, 7 }, > - [OVL_ADAPTOR_MERGE0] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE1, 1 }, > - [OVL_ADAPTOR_MERGE1] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE2, 2 }, > - [OVL_ADAPTOR_MERGE2] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE3, 3 }, > - [OVL_ADAPTOR_MERGE3] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE4, 4 }, > + [OVL_ADAPTOR_ETHDR0] = { OVL_ADAPTOR_TYPE_ETHDR, > DDP_COMPONENT_ETHDR_MIXER, 0, &_ethdr }, > + [OVL_ADAPTOR_MDP_RDMA0] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA0, 0, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA1] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA1, 1, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA2] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA2, 2, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA3] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA3, 3, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA4] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA4, 4, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA5] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA5, 5, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA6] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA6, 6, &_rdma }, > + [OVL_ADAPTOR_MDP_RDMA7] = { OVL_ADAPTOR_TYPE_MDP_RDMA, > DDP_COMPONENT_MDP_RDMA7, 7, &_rdma }, > + [OVL_ADAPTOR_MERGE0] = { OVL_ADAPTOR_TYPE_MERGE, > DDP_COMPONENT_MERGE1, 1, &_merge }, > + [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 }, > }; > > void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int > idx, > @@ -187,73 +203,34 @@ void mtk_ovl_adaptor_stop(struct device *dev) > > int mtk_ovl_adaptor_clk_enable(struct device *dev) > { > - struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > - struct device *comp; > - int ret; > int i; > - > - for (i = 0; i < OVL_ADAPTOR_MERGE0; i++) { > - comp = ovl_adaptor->ovl_adaptor_comp[i]; > - ret = pm_runtime_get_sync(comp); > - if (ret < 0) { > - dev_err(dev, "Failed to enable power domain %d, > err %d\n", i, ret); > - goto pwr_err; > - } > - } > + int ret; > + struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > > for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) { > - comp = ovl_adaptor->ovl_adaptor_comp[i]; > - > - if (i < OVL_ADAPTOR_MERGE0) > - ret = mtk_mdp_rdma_clk_enable(comp); > - else if (i < OVL_ADAPTOR_ETHDR0) > - ret = mtk_merge_clk_enable(comp); > - else > - ret = mtk_ethdr_clk_enable(comp); > + dev = ovl_adaptor->ovl_adaptor_comp[i]; > + if (!dev) > + continue; > + ret = comp_matches[i].funcs->clk_enable(dev); > if (ret) { > - dev_err(dev, "Failed to enable clock %d, err > %d\n", i, ret); > - goto clk_err; > + while (--i >= 0) > + comp_matches[i].funcs- > >clk_disable(dev); > + return ret; > } > } > - > - return ret; > - > -clk_err: > - while (--i >= 0) { > - comp = ovl_adaptor->ovl_adaptor_comp[i]; > - if (i < OVL_ADAPTOR_MERGE0) > - mtk_mdp_rdma_clk_disable(comp); > - else if (i < OVL_ADAPTOR_ETHDR0) > - mtk_merge_clk_disable(comp); > - else > - mtk_ethdr_clk_disable(comp); > - } > - i = OVL_ADAPTOR_MERGE0; > - > -pwr_err: > - while (--i >= 0) > - pm_runtime_put(ovl_adaptor->ovl_adaptor_comp[i]); > - > - return ret; > + return 0; > } > > void mtk_ovl_adaptor_clk_disable(struct device *dev) > { > - struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > - struct device *comp; > int i; > + struct mtk_disp_ovl_adaptor *ovl_adaptor = > dev_get_drvdata(dev); > > for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) { > - comp = ovl_adaptor->ovl_adaptor_comp[i]; > - > - if (i < OVL_ADAPTOR_MERGE0) { > - mtk_mdp_rdma_clk_disable(comp); > - pm_runtime_put(comp); > - } else if (i < OVL_ADAPTOR_ETHDR0) { > - mtk_merge_clk_disable(comp); > - } else { > - mtk_ethdr_clk_disable(comp); > - } > + dev = ovl_adaptor->ovl_adaptor_comp[i]; > + if (!dev) > + continue; > + comp_matches[i].funcs->clk_disable(dev); > } > } > > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > Since LARBs (Local ARBiter) have to be powered on before its users, > to ensure the power-on sequence, we created a device link between > RDMA and its LARB, and when pm_runtime_get_sync is called in RDMA, > system will guarantee the LARB is powered on before the RDMA. OVL is one of LARB user, but OVL have no device link with LARB, but it works for years. If all DMA component need device link with LARB, add to all of them not only mdp_rdma. Regards, CK > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > index c3adaeefd551..fce6fbb534b1 100644 > --- a/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > +++ b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > @@ -244,10 +244,23 @@ size_t mtk_mdp_rdma_get_num_formats(struct > device *dev) > > int mtk_mdp_rdma_clk_enable(struct device *dev) > { > + int ret; > struct mtk_mdp_rdma *rdma = dev_get_drvdata(dev); > > - clk_prepare_enable(rdma->clk); > - return 0; > + /* > + * Since LARBs (Local ARBiter) have to be powered on before its > users, > + * to ensure the power-on sequence, we created a device link > between > + * RDMA and its LARB, and when pm_runtime_get_sync is called in > RDMA, > + * system will make sure the LARB is powered on before the RDMA > + */ > + ret = pm_runtime_get_sync(dev); > + > + if (ret < 0) > + dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret); > + else > + ret = clk_prepare_enable(rdma->clk); > + > + return ret; > } > > void mtk_mdp_rdma_clk_disable(struct device *dev) > @@ -255,6 +268,9 @@ void mtk_mdp_rdma_clk_disable(struct device *dev) > struct mtk_mdp_rdma *rdma = dev_get_drvdata(dev); > > clk_disable_unprepare(rdma->clk); > + > + /* Same reason as when enabling clock, turn the LARB off */ > + pm_runtime_put(dev); > } > > static int mtk_mdp_rdma_bind(struct device *dev, struct device > *master, > -- > 2.18.0 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > Padding is a new display module on MT8188, it provides ability > to add pixels to width and height of a layer with specified colors. > > Due to hardware design, Mixer in VDOSYS1 requires width of a layer > to be 2-pixel-align, or 4-pixel-align when ETHDR is enabled, > we need Padding to deal with odd width. > > Please notice that even if the Padding is in bypass mode, > settings in register must be cleared to 0, > or undefined behaviors could happen. You just set padding to bypass mode and not clear settings to 0. Any thing wrong? Regards, CK > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/Makefile | 3 +- > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 + > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 + > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- > drivers/gpu/drm/mediatek/mtk_padding.c | 136 > ++++++++++++++++++++++++ > 5 files changed, 143 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/mediatek/mtk_padding.c > > diff --git a/drivers/gpu/drm/mediatek/Makefile > b/drivers/gpu/drm/mediatek/Makefile > index d4d193f60271..5e4436403b8d 100644 > --- a/drivers/gpu/drm/mediatek/Makefile > +++ b/drivers/gpu/drm/mediatek/Makefile > @@ -16,7 +16,8 @@ mediatek-drm-y := mtk_disp_aal.o \ > mtk_dsi.o \ > mtk_dpi.o \ > mtk_ethdr.o \ > - mtk_mdp_rdma.o > + mtk_mdp_rdma.o \ > + mtk_padding.o > > obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > index 2254038519e1..f9fdb1268aa5 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > @@ -157,4 +157,7 @@ void mtk_mdp_rdma_config(struct device *dev, > struct mtk_mdp_rdma_cfg *cfg, > const u32 *mtk_mdp_rdma_get_formats(struct device *dev); > size_t mtk_mdp_rdma_get_num_formats(struct device *dev); > > +int mtk_padding_clk_enable(struct device *dev); > +void mtk_padding_clk_disable(struct device *dev); > +void mtk_padding_config(struct device *dev, struct cmdq_pkt > *cmdq_pkt); > #endif > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 93552d76b6e7..cde69f39a066 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -973,6 +973,7 @@ static struct platform_driver * const > mtk_drm_drivers[] = { > &mtk_dsi_driver, > &mtk_ethdr_driver, > &mtk_mdp_rdma_driver, > + &mtk_padding_driver, > }; > > static int __init mtk_drm_init(void) > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > index eb2fd45941f0..562f2db47add 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > @@ -64,5 +64,5 @@ extern struct platform_driver mtk_dpi_driver; > extern struct platform_driver mtk_dsi_driver; > extern struct platform_driver mtk_ethdr_driver; > extern struct platform_driver mtk_mdp_rdma_driver; > - > +extern struct platform_driver mtk_padding_driver; > #endif /* MTK_DRM_DRV_H */ > diff --git a/drivers/gpu/drm/mediatek/mtk_padding.c > b/drivers/gpu/drm/mediatek/mtk_padding.c > new file mode 100644 > index 000000000000..bbb9c5e286ce > --- /dev/null > +++ b/drivers/gpu/drm/mediatek/mtk_padding.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 MediaTek Inc. > + */ > + > +#include <linux/clk.h> > +#include <linux/component.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/soc/mediatek/mtk-cmdq.h> > + > +#include "mtk_disp_drv.h" > +#include "mtk_drm_crtc.h" > +#include "mtk_drm_ddp_comp.h" > + > +/** > + * struct mtk_padding - basic information of Padding > + * @clk: Clock of the module > + * @regs: Virtual address of the Padding for CPU to access > + * @cmdq_reg: CMDQ setting of the Padding > + * > + * Every Padding should have different clock source, register base, > and > + * CMDQ settings, we stored these differences all together. > + */ > +struct mtk_padding { > + struct clk *clk; > + void __iomem *regs; > + struct cmdq_client_reg cmdq_reg; > +}; > + > +int mtk_padding_clk_enable(struct device *dev) > +{ > + struct mtk_padding *padding = dev_get_drvdata(dev); > + > + return clk_prepare_enable(padding->clk); > +} > + > +void mtk_padding_clk_disable(struct device *dev) > +{ > + struct mtk_padding *padding = dev_get_drvdata(dev); > + > + clk_disable_unprepare(padding->clk); > +} > + > +void mtk_padding_config(struct device *dev, struct cmdq_pkt > *cmdq_pkt) > +{ > + struct mtk_padding *padding = dev_get_drvdata(dev); > + > + /* bypass padding */ > + mtk_ddp_write_mask(cmdq_pkt, GENMASK(1, 0), &padding->cmdq_reg, > padding->regs, 0, > + GENMASK(1, 0)); > +} > + > +static int mtk_padding_bind(struct device *dev, struct device > *master, void *data) > +{ > + return 0; > +} > + > +static void mtk_padding_unbind(struct device *dev, struct device > *master, void *data) > +{ > +} > + > +static const struct component_ops mtk_padding_component_ops = { > + .bind = mtk_padding_bind, > + .unbind = mtk_padding_unbind, > +}; > + > +static int mtk_padding_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_padding *priv; > + struct resource *res; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(priv->clk)) { > + dev_err(dev, "failed to get clk\n"); > + return PTR_ERR(priv->clk); > + } > + > + priv->regs = devm_platform_get_and_ioremap_resource(pdev, 0, > &res); > + if (IS_ERR(priv->regs)) { > + dev_err(dev, "failed to do ioremap\n"); > + return PTR_ERR(priv->regs); > + } > + > +#if IS_REACHABLE(CONFIG_MTK_CMDQ) > + ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0); > + if (ret) { > + dev_err(dev, "failed to get gce client reg\n"); > + return ret; > + } > +#endif > + > + platform_set_drvdata(pdev, priv); > + > + ret = devm_pm_runtime_enable(dev); > + if (ret) > + return ret; > + > + ret = component_add(dev, &mtk_padding_component_ops); > + if (ret) { > + pm_runtime_disable(dev); > + return dev_err_probe(dev, ret, "failed to add > component\n"); > + } > + > + return 0; > +} > + > +static int mtk_padding_remove(struct platform_device *pdev) > +{ > + component_del(&pdev->dev, &mtk_padding_component_ops); > + return 0; > +} > + > +static const struct of_device_id mtk_padding_driver_dt_match[] = { > + { .compatible = "mediatek,mt8188-padding" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, mtk_padding_driver_dt_match); > + > +struct platform_driver mtk_padding_driver = { > + .probe = mtk_padding_probe, > + .remove = mtk_padding_remove, > + .driver = { > + .name = "mediatek-padding", > + .owner = THIS_MODULE, > + .of_match_table = mtk_padding_driver_dt_match, > + }, > +}; > -- > 2.18.0 >
Hi CK, On Thu, 2023-09-28 at 03:05 +0000, CK Hu (胡俊光) wrote: > Hi, Hsiao-chien: > > On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > > Padding is a new display module on MT8188, it provides ability > > to add pixels to width and height of a layer with specified colors. > > > > Due to hardware design, Mixer in VDOSYS1 requires width of a layer > > to be 2-pixel-align, or 4-pixel-align when ETHDR is enabled, > > we need Padding to deal with odd width. > > > > Please notice that even if the Padding is in bypass mode, > > settings in register must be cleared to 0, > > or undefined behaviors could happen. > > You just set padding to bypass mode and not clear settings to 0. Any > thing wrong? > Since the deafult value of all the registers in Padding is zero, and we are not using Padding currently, it's fine if we just set padding to bypass mode witout clearing other registers. The comment is just a reminder in case we forget it in the future. Regards, Hsiao Chien Sung
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > 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 | 33 > +++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > index 84133303a6ec..217c39af27bd 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > @@ -30,6 +30,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, > }; > > @@ -47,6 +48,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 > }; > > @@ -67,6 +76,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 = { > @@ -79,6 +89,11 @@ 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, > +}; > + > static const struct mtk_ddp_comp_funcs _rdma = { > .clk_enable = mtk_mdp_rdma_clk_enable, > .clk_disable = mtk_mdp_rdma_clk_disable, > @@ -98,6 +113,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, > @@ -109,6 +132,8 @@ void mtk_ovl_adaptor_layer_config(struct device > *dev, unsigned int idx, > struct mtk_mdp_rdma_cfg rdma_config = {0}; > struct device *rdma_l; > struct device *rdma_r; > + struct device *padding_l; > + struct device *padding_r; > struct device *merge; > struct device *ethdr; > const struct drm_format_info *fmt_info = > drm_format_info(pending->format); > @@ -125,6 +150,8 @@ void mtk_ovl_adaptor_layer_config(struct device > *dev, unsigned int idx, > > rdma_l = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0 + > 2 * idx]; > rdma_r = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0 + > 2 * idx + 1]; > + padding_l = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_PADDING0 > + 2 * idx]; > + padding_r = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_PADDING0 > + 2 * idx + 1]; > merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + > idx]; > ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]; > > @@ -160,10 +187,15 @@ void mtk_ovl_adaptor_layer_config(struct device > *dev, unsigned int idx, > rdma_config.color_encoding = pending->color_encoding; > mtk_mdp_rdma_config(rdma_l, &rdma_config, cmdq_pkt); > > + if (padding_l) > + mtk_padding_config(padding_l, cmdq_pkt); > + > if (use_dual_pipe) { > rdma_config.x_left = l_w; > rdma_config.width = r_w; > mtk_mdp_rdma_config(rdma_r, &rdma_config, cmdq_pkt); > + if (padding_r) > + mtk_padding_config(padding_r, cmdq_pkt); > } > > mtk_merge_start_cmdq(merge, cmdq_pkt); > @@ -354,6 +386,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 >
Hi, Hsiao-chien: On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > - The mmsys_dev_num in MT8188 VDOSYS0 was set to 1 since > VDOSYS1 was not available before. Increase it to support > VDOSYS1 in display driver. > - Add compatible name for MT8188 VDOSYS1 > (shares the same driver data with MT8195 VDOSYS1) Reviewed-by: CK Hu <ck.hu@mediatek.com> > > Reviewed-by: AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index cde69f39a066..212475436f47 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -288,6 +288,7 @@ static const struct mtk_mmsys_driver_data > mt8186_mmsys_driver_data = { > static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data > = { > .main_path = mt8188_mtk_ddp_main, > .main_len = ARRAY_SIZE(mt8188_mtk_ddp_main), > + .mmsys_dev_num = 2, > }; > > static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = > { > @@ -328,6 +329,8 @@ static const struct of_device_id mtk_drm_of_ids[] > = { > .data = &mt8186_mmsys_driver_data}, > { .compatible = "mediatek,mt8188-vdosys0", > .data = &mt8188_vdosys0_driver_data}, > + { .compatible = "mediatek,mt8188-vdosys1", > + .data = &mt8195_vdosys1_driver_data}, > { .compatible = "mediatek,mt8192-mmsys", > .data = &mt8192_mmsys_driver_data}, > { .compatible = "mediatek,mt8195-mmsys", > -- > 2.18.0 >
Il 28/09/23 05:39, Shawn Sung (宋孝謙) ha scritto: > Hi CK, > > On Thu, 2023-09-28 at 03:05 +0000, CK Hu (胡俊光) wrote: >> Hi, Hsiao-chien: >> >> On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: >>> Padding is a new display module on MT8188, it provides ability >>> to add pixels to width and height of a layer with specified colors. >>> >>> Due to hardware design, Mixer in VDOSYS1 requires width of a layer >>> to be 2-pixel-align, or 4-pixel-align when ETHDR is enabled, >>> we need Padding to deal with odd width. >>> >>> Please notice that even if the Padding is in bypass mode, >>> settings in register must be cleared to 0, >>> or undefined behaviors could happen. >> >> You just set padding to bypass mode and not clear settings to 0. Any >> thing wrong? >> > > Since the deafult value of all the registers in Padding is zero, and > we are not using Padding currently, it's fine if we just set padding to > bypass mode witout clearing other registers. > > The comment is just a reminder in case we forget it in the future. Do *not* rely on default register values, because you don't know what booted Linux in the first place: you shall *not* expect a clean state and you shall *not* expect a clean boot. Besides, what I see is that you're setting GENMASK(1, 0) without explaining why in the code: you have to add at least the definitions for PADDING_EN and PADDING_BYPASS. I also don't see why you shouldn't add at least basic handling for this block, as it looks easy enough: after all, you anyway have to make sure that the registers are cleared - might as well just add a little more effort on top and actually set them to meaningful values? That's ultimately your choice, but I don't want to see any GENMASK(31,0) write even for register clearing. Please make this driver proper. Thanks, Angelo > > Regards, > Hsiao Chien Sung
Hi Angelo, On Thu, 2023-09-28 at 12:24 +0200, AngeloGioacchino Del Regno wrote: > Il 28/09/23 05:39, Shawn Sung (宋孝謙) ha scritto: > > Hi CK, > > > > On Thu, 2023-09-28 at 03:05 +0000, CK Hu (胡俊光) wrote: > > > Hi, Hsiao-chien: > > > > > > On Mon, 2023-09-11 at 15:42 +0800, Hsiao Chien Sung wrote: > > > > Padding is a new display module on MT8188, it provides ability > > > > to add pixels to width and height of a layer with specified > > > > colors. > > > > > > > > Due to hardware design, Mixer in VDOSYS1 requires width of a > > > > layer > > > > to be 2-pixel-align, or 4-pixel-align when ETHDR is enabled, > > > > we need Padding to deal with odd width. > > > > > > > > Please notice that even if the Padding is in bypass mode, > > > > settings in register must be cleared to 0, > > > > or undefined behaviors could happen. > > > > > > You just set padding to bypass mode and not clear settings to 0. > > > Any > > > thing wrong? > > > > > > > Since the deafult value of all the registers in Padding is zero, > > and > > we are not using Padding currently, it's fine if we just set > > padding to > > bypass mode witout clearing other registers. > > > > The comment is just a reminder in case we forget it in the future. > > Do *not* rely on default register values, because you don't know what > booted > Linux in the first place: you shall *not* expect a clean state and > you shall > *not* expect a clean boot. > > Besides, what I see is that you're setting GENMASK(1, 0) without > explaining > why in the code: you have to add at least the definitions for > PADDING_EN and > PADDING_BYPASS. > > I also don't see why you shouldn't add at least basic handling for > this block, > as it looks easy enough: after all, you anyway have to make sure that > the > registers are cleared - might as well just add a little more effort > on top > and actually set them to meaningful values? That's ultimately your > choice, but > I don't want to see any GENMASK(31,0) write even for register > clearing. > > Please make this driver proper. > Thank you for the suggestions. I'll implement it in the next version. Thanks, Shawn