On Tue, Nov 26, 2024 at 02:12:04PM +0000, Sandor Yu wrote: > > > > > > On 5 November 2024 14:05:51 GMT, Sandor Yu <sandor.yu@xxxxxxx> wrote: > > >> > > >> On Tue, Oct 29, 2024 at 02:02:14PM +0800, Sandor Yu wrote: > > >> > Add Cadence HDP-TX DisplayPort and HDMI PHY driver for i.MX8MQ. > > >> > > > >> > Cadence HDP-TX PHY could be put in either DP mode or HDMI mode > > base > > >> > on the configuration chosen. > > >> > DisplayPort or HDMI PHY mode is configured in the driver. > > >> > > > >> > Signed-off-by: Sandor Yu <Sandor.yu@xxxxxxx> > > >> > Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> > > >> > --- > > >> > v17->v18: > > >> > - fix build error as code rebase to latest kernel version. > > >> > > > >> > drivers/phy/freescale/Kconfig | 10 + > > >> > drivers/phy/freescale/Makefile | 1 + > > >> > drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c | 1337 > > >> ++++++++++++++++++ > > >> > 3 files changed, 1348 insertions(+) create mode 100644 > > >> > drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c > > >> > > > >> > diff --git a/drivers/phy/freescale/Kconfig > > >> > b/drivers/phy/freescale/Kconfig index dcd9acff6d01a..2b1210367b31c > > >> > 100644 > > >> > --- a/drivers/phy/freescale/Kconfig > > >> > +++ b/drivers/phy/freescale/Kconfig > > >> > @@ -35,6 +35,16 @@ config PHY_FSL_IMX8M_PCIE > > >> > Enable this to add support for the PCIE PHY as found on > > >> > i.MX8M family of SOCs. > > >> > > > >> > +config PHY_FSL_IMX8MQ_HDPTX > > >> > + tristate "Freescale i.MX8MQ DP/HDMI PHY support" > > >> > + depends on OF && HAS_IOMEM > > >> > + depends on COMMON_CLK > > >> > + select GENERIC_PHY > > >> > + select CDNS_MHDP_HELPER > > >> > > >> This can have problems with being satisfied on randconfig builds, > > >> because CDNS_MHDP_HELPER is deep inside the DRM tree. > > > > > >Yes, it should be. Change it to "depend on CDNS_MHDP_HELPER" will > > eliminate this problem. > > > > No, depending on a non-user-selectable symbol is a bad idea. You should > > either depend/select all necessary symbols or, better in my opinion, move > > your helpers out of the DRM tree. > > How about change CDNS_MHDP_HELPER to user selectable? such as > > config CDNS_MHDP_HELPER > tristate "Cadence MHDP Helper driver" > help > Enable Cadence MHDP helpers for mailbox, HDMI and DP. > This driver provides a foundational layer of mailbox communication for > various Cadence MHDP IP implementations, such as HDMI and DisplayPort I'd say, it's a bad idea. Helpers should be automatically selected. > > Finding a suitable location for the helper code is challenging. > It needs to be shared among various IP versions (essentially different SoCs) > and across different driver types to facilitate mailbox access. > I've searched the kernel code but haven't found a good precedent. > Placing this helper in either drivers/gpu/drm/bridge/cadence or drivers/soc/ (as you previously suggested) has its drawbacks. > drivers/gpu/drm/bridge/cadence at least provides better context for readers. Yes, I understand the issue. However you might as well use drivers/phy/ for the helpers: your DRM driver already depends on GENERIC_PHY, but there is no dependency from the PHY onto the DRM. > > > > > > > > > > >> > > >> > + help > > >> > + Enable this to support the Cadence HDPTX DP/HDMI PHY > > driver > > >> > + on i.MX8MQ SOC. > > >> > + > > >> > config PHY_FSL_IMX8QM_HSIO > > >> > tristate "Freescale i.MX8QM HSIO PHY" > > >> > depends on OF && HAS_IOMEM -- With best wishes Dmitry
