diff mbox series

drm/mediatek: Add MT8188 Overlay Driver Data

Message ID 20240221165643.1679073-1-greenjustin@chromium.org (mailing list archive)
State New, archived
Headers show
Series drm/mediatek: Add MT8188 Overlay Driver Data | expand

Commit Message

Justin Green Feb. 21, 2024, 4:56 p.m. UTC
Add MT8188 overlay driver configuration data. This change consequently
enables 10-bit overlay support on MT8188 devices.

Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
BA30 overlays are confirmed to work from modetest.

Signed-off-by: Justin Green <greenjustin@chromium.org>
Tested-by: Justin Green <greenjustin@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

AngeloGioacchino Del Regno Feb. 22, 2024, 8:42 a.m. UTC | #1
Il 21/02/24 17:56, Justin Green ha scritto:
> Add MT8188 overlay driver configuration data. This change consequently
> enables 10-bit overlay support on MT8188 devices.
> 
> Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
> BA30 overlays are confirmed to work from modetest.
> 
> Signed-off-by: Justin Green <greenjustin@chromium.org>
> Tested-by: Justin Green <greenjustin@chromium.org>

Hello Justin,

I'm 99.9% sure that you don't need this, you can just use compatibles

compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl";

as they *are* indeed compatible, and MT8188 does support AFBC as well.

Cheers,
Angelo

> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 2bffe4245466..696aabe124c2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -635,6 +635,17 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>   	.supports_clrfmt_ext = true,
>   };
>   
> +static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = {
> +	.addr = DISP_REG_OVL_ADDR_MT8173,
> +	.gmc_bits = 10,
> +	.layer_nr = 4,
> +	.fmt_rgb565_is_0 = true,
> +	.smi_id_en = true,
> +	.formats = mt8195_formats,
> +	.num_formats = ARRAY_SIZE(mt8195_formats),
> +	.supports_clrfmt_ext = true,
> +};
> +
>   static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
>   	{ .compatible = "mediatek,mt2701-disp-ovl",
>   	  .data = &mt2701_ovl_driver_data},
> @@ -650,6 +661,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
>   	  .data = &mt8192_ovl_2l_driver_data},
>   	{ .compatible = "mediatek,mt8195-disp-ovl",
>   	  .data = &mt8195_ovl_driver_data},
> +	{ .compatible = "mediatek,mt8188-disp-ovl",
> +	  .data = &mt8188_ovl_driver_data},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
Fei Shao Feb. 26, 2024, 8:21 a.m. UTC | #2
On Thu, Feb 22, 2024 at 4:43 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 21/02/24 17:56, Justin Green ha scritto:
> > Add MT8188 overlay driver configuration data. This change consequently
> > enables 10-bit overlay support on MT8188 devices.
> >
> > Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
> > BA30 overlays are confirmed to work from modetest.
> >
> > Signed-off-by: Justin Green <greenjustin@chromium.org>
> > Tested-by: Justin Green <greenjustin@chromium.org>
>
> Hello Justin,
>
> I'm 99.9% sure that you don't need this, you can just use compatibles
>
> compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl";
>
> as they *are* indeed compatible, and MT8188 does support AFBC as well.

Hi,

I confirmed that I can lit up the MT8188 display with that plus a
follow-up patch [1].
Otherwise a compatible sequence of mt8188, mt8195 and mt8192 would be
needed, but that would be somewhat redundant.

[1]: https://lore.kernel.org/all/20240226080721.3331649-1-fshao@chromium.org/

Regards,
Fei

>
> Cheers,
> Angelo
>
AngeloGioacchino Del Regno Feb. 26, 2024, 8:43 a.m. UTC | #3
Il 26/02/24 09:21, Fei Shao ha scritto:
> On Thu, Feb 22, 2024 at 4:43 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 21/02/24 17:56, Justin Green ha scritto:
>>> Add MT8188 overlay driver configuration data. This change consequently
>>> enables 10-bit overlay support on MT8188 devices.
>>>
>>> Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
>>> BA30 overlays are confirmed to work from modetest.
>>>
>>> Signed-off-by: Justin Green <greenjustin@chromium.org>
>>> Tested-by: Justin Green <greenjustin@chromium.org>
>>
>> Hello Justin,
>>
>> I'm 99.9% sure that you don't need this, you can just use compatibles
>>
>> compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl";
>>
>> as they *are* indeed compatible, and MT8188 does support AFBC as well.
> 
> Hi,
> 
> I confirmed that I can lit up the MT8188 display with that plus a
> follow-up patch [1].
> Otherwise a compatible sequence of mt8188, mt8195 and mt8192 would be
> needed, but that would be somewhat redundant.
> 

Actually, that wouldn't be MT8192, but MT8183 - following MT8195's devicetree,

so it would be

compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl",
              "mediatek,mt8183-disp-ovl";

And yes that could be redundant, but I'm not sure that adding a compatible
string to the mtk_drm_drv matches is a good idea.

I'd be more for using the triple compatible strings in there instead, as it'd
be like that for only *one* node and not more than that - IMO, that is totally
acceptable and it's also the best (and lightest) solution.

Cheers,
Angelo

> [1]: https://lore.kernel.org/all/20240226080721.3331649-1-fshao@chromium.org/
> 
> Regards,
> Fei
>
Fei Shao Feb. 26, 2024, 8:51 a.m. UTC | #4
On Mon, Feb 26, 2024 at 4:43 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 26/02/24 09:21, Fei Shao ha scritto:
> > On Thu, Feb 22, 2024 at 4:43 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 21/02/24 17:56, Justin Green ha scritto:
> >>> Add MT8188 overlay driver configuration data. This change consequently
> >>> enables 10-bit overlay support on MT8188 devices.
> >>>
> >>> Tested by running ChromeOS UI on MT8188 and using modetest -P. AR30 and
> >>> BA30 overlays are confirmed to work from modetest.
> >>>
> >>> Signed-off-by: Justin Green <greenjustin@chromium.org>
> >>> Tested-by: Justin Green <greenjustin@chromium.org>
> >>
> >> Hello Justin,
> >>
> >> I'm 99.9% sure that you don't need this, you can just use compatibles
> >>
> >> compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl";
> >>
> >> as they *are* indeed compatible, and MT8188 does support AFBC as well.
> >
> > Hi,
> >
> > I confirmed that I can lit up the MT8188 display with that plus a
> > follow-up patch [1].
> > Otherwise a compatible sequence of mt8188, mt8195 and mt8192 would be
> > needed, but that would be somewhat redundant.
> >
>
> Actually, that wouldn't be MT8192, but MT8183 - following MT8195's devicetree,

Ah yes, you're right.

>
> so it would be
>
> compatible = "mediatek,mt8188-disp-ovl", "mediatek,mt8195-disp-ovl",
>               "mediatek,mt8183-disp-ovl";
>
> And yes that could be redundant, but I'm not sure that adding a compatible
> string to the mtk_drm_drv matches is a good idea.
>
> I'd be more for using the triple compatible strings in there instead, as it'd
> be like that for only *one* node and not more than that - IMO, that is totally
> acceptable and it's also the best (and lightest) solution.

I see, I'm also fine with this approach. Thanks for the feedback!

Regards,
Fei

>
> Cheers,
> Angelo
>
> > [1]: https://lore.kernel.org/all/20240226080721.3331649-1-fshao@chromium.org/
> >
> > Regards,
> > Fei
> >
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 2bffe4245466..696aabe124c2 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -635,6 +635,17 @@  static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
 	.supports_clrfmt_ext = true,
 };
 
+static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = {
+	.addr = DISP_REG_OVL_ADDR_MT8173,
+	.gmc_bits = 10,
+	.layer_nr = 4,
+	.fmt_rgb565_is_0 = true,
+	.smi_id_en = true,
+	.formats = mt8195_formats,
+	.num_formats = ARRAY_SIZE(mt8195_formats),
+	.supports_clrfmt_ext = true,
+};
+
 static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
 	{ .compatible = "mediatek,mt2701-disp-ovl",
 	  .data = &mt2701_ovl_driver_data},
@@ -650,6 +661,8 @@  static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
 	  .data = &mt8192_ovl_2l_driver_data},
 	{ .compatible = "mediatek,mt8195-disp-ovl",
 	  .data = &mt8195_ovl_driver_data},
+	{ .compatible = "mediatek,mt8188-disp-ovl",
+	  .data = &mt8188_ovl_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);