[RFC PATCH 0/2] drm/mediatek/hdmi : split legacy driver and add mt8195

[Guillaume Ranquet]

Hi Chun-Kuang,
Quoting Guillaume Ranquet (2021-11-08 01:08:45)
>
> Following the review of "[v1,4/4] drm/mediatek: add mt8195 hdmi TX support" [1] , I've been asked to try to merge the "legacy
> HDMI driver with the "new" mt8195 HDMI driver. Though the registers and IPs are rather different, I've made an attempt at the
> excercise on which I would like some comments before submitting a new version of the patchset.
>
> I've created a new mtk_hdmi_common.{c,h} which contains what I could identify as common enough to be added in there.
> I have not renamed the mtk_hdmi.{c,h} driver to something else yet.
> I've then added mtk8195 hdmi driver on top of this work.
>
> The code is still rather sketchy and I have some questions on best practices for that kind of exercices?
> In no particular order, here's a bunch of random questions:
>
> 1 would you rather have:
> - as presented here: a boolean with a bunch of if/else in the common code to handle the discrepencies between the two drivers
> - function pointers, with an "ops" structure set in the drvdata... though the ops are a bit random and wouldn't really
> make sense to be grouped together?
>
> 2 I'm not really happy with how the mt8195 driver and ddc are sharing the same __iomem regs and not sure how to exactly
> handle this now that both hdmi drivers are tied together... it was hackish from the begining, but now I'm a bit out of
> solutions to handle this peculiar "feature" of the mt8195 hdmi/ddc driver. any hints would be appreciated.
>
> 3 Should the "legacy" and mt8195 drivers be under the same menu entry?
>
> 4 I'm looking into the clock framework and I'm wondering if there would be benefits to switch to the "bulk" interface? as
> both driver seem to request them all once and for all? the code might end up easier to read.
>
> 5 Would it be beneficial to further split the driver with audio on one side and video on the other? from what I understood,
> the main complaint was that the driver was too big to review... that could help with that at least?
>
>
> Thx for your patience... any further comment is welcome.
>
> Thx,
> Guillaume.
>
> [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20210929094425.745-5-granquet@baylibre.com/
>
> --
> Guillaume Ranquet (2):
>   drm/mediatek: extract common functions from the mtk hdmi driver
>   drm/mediatek: add mtk8195 hdmi display driver
>
>  drivers/gpu/drm/mediatek/Kconfig              |   10 +
>  drivers/gpu/drm/mediatek/Makefile             |    5 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |  635 +-----
>  drivers/gpu/drm/mediatek/mtk_hdmi_common.c    |  521 +++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_common.h    |  258 +++
>  drivers/gpu/drm/mediatek/mtk_mt8195_hdmi.c    | 1835 +++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_mt8195_hdmi.h    |   26 +
>  .../gpu/drm/mediatek/mtk_mt8195_hdmi_ddc.c    |  530 +++++
>  .../gpu/drm/mediatek/mtk_mt8195_hdmi_ddc.h    |   20 +
>  .../gpu/drm/mediatek/mtk_mt8195_hdmi_regs.h   |  329 +++
>  10 files changed, 3560 insertions(+), 609 deletions(-)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_common.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_common.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8195_hdmi.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8195_hdmi.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8195_hdmi_ddc.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8195_hdmi_ddc.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8195_hdmi_regs.h
>
> --
> 2.32.0
>

Since you have requested that I merge both drivers, could you give me
your feedback on this RFC so
that I know how to move forward? I'm planning on making a v2 soon
based on this RFC (and hopefully its
outcome).

Thx,
Guillaume.