| Message ID | 20240221165643.1679073-1-greenjustin@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/mediatek: Add MT8188 Overlay Driver Data | expand |
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);
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 >
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 >
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 --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);