On Tue, Apr 08, 2025 at 08:12:29PM +0800, Andy Yan wrote: > > > Hi Alex, > > At 2025-04-03 01:24:22, "Alex Bee" <knaerz...@gmail.com> wrote: > > > >Hi Andy, > > > >> From: Andy Yan <andy....@rock-chips.com> > >> > >> Convert it to drm bridge driver, it will be convenient for us to > >> migrate the connector part to the display driver later. > >> > >> Signed-off-by: Andy Yan <andy....@rock-chips.com> > >> > >> --- > >> > >> Changes in v3: > >> - First included in v3 > >> - Link to V2: > >> https://lore.kernel.org/dri-devel/20250325132944.171111-1-andys...@163.com/ > >> > >> drivers/gpu/drm/bridge/Kconfig | 7 + > >> drivers/gpu/drm/bridge/Makefile | 1 + > >> .../inno_hdmi.c => bridge/inno-hdmi.c} | 924 ++++++++++-------- > >> drivers/gpu/drm/rockchip/Kconfig | 1 + > >> drivers/gpu/drm/rockchip/Makefile | 2 +- > >> drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c | 187 ++++ > >> drivers/gpu/drm/rockchip/inno_hdmi.h | 349 ------- > >> include/drm/bridge/inno_hdmi.h | 33 + > >> 8 files changed, 741 insertions(+), 763 deletions(-) > >> rename drivers/gpu/drm/{rockchip/inno_hdmi.c => bridge/inno-hdmi.c} (52%) > >> create mode 100644 drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c > >> delete mode 100644 drivers/gpu/drm/rockchip/inno_hdmi.h > >> create mode 100644 include/drm/bridge/inno_hdmi.h > >> > > > >... > > > >> +#define m_RX_DONE (1 << 0) > >> + > >> +#define HDMI_CEC_TX_INT 0xda > >> +#define HDMI_CEC_RX_INT 0xdb > >> +#define HDMI_CEC_BUSFREETIME_L 0xdc > >> +#define HDMI_CEC_BUSFREETIME_H 0xdd > >> +#define HDMI_CEC_LOGICADDR 0xde > >> + > >> struct inno_hdmi_i2c { > >> struct i2c_adapter adap; > >> > >> @@ -68,41 +395,18 @@ struct inno_hdmi_i2c { > >> > >> struct inno_hdmi { > >> struct device *dev; > >> - > >> + struct drm_bridge bridge; > >> struct clk *pclk; > >> struct clk *refclk; > >> void __iomem *regs; > >> struct regmap *grf; > >> > >> - struct drm_connector connector; > >> - struct rockchip_encoder encoder; > >> - > >> struct inno_hdmi_i2c *i2c; > >> struct i2c_adapter *ddc; > >> - > >> - const struct inno_hdmi_variant *variant; > >> + const struct inno_hdmi_plat_data *plat_data; > >> + unsigned int colorimetry; > > > >thanks a lot for doing the bridge conversion for this driver. > >Please keep the custom connector state which was introduced after Maxim's > >review during the last rework of this [0] driver. The colorimetry is not > >part of the device, but of the connector and thus should not be part of the > >device struct. > >It's, however, likely that the common (hdmi-)connector framework will once > >hold its own colorimetry property and then the custom connector state in > >this driver can go away, but until than we have to keep it here. > > After converting to a bridge driver, this driver no longer has a connector. > In this case, how should I create customized connector states?
You can subclass drm_bridge_state. Another option is to follow rk3066_hdmi.c and to pass mode to inno_hdmi_config_video_csc(). Finally, you can just extend the drm_connector_hdmi_state. My preference would lean towards the second option. > > > > >Thanks, > >Alex > > > >[0] > >https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ceeb0f0104a62c867656c2730a51df47e7350b8f > > > > > >> }; > >> > >> -struct inno_hdmi_connector_state { > >> - struct drm_connector_state base; > >> - unsigned int colorimetry; > >> -}; > >> - > >> -static struct inno_hdmi *encoder_to_inno_hdmi(struct drm_encoder *encoder) > >> -{ > >> - struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); > >> - > >> - return container_of(rkencoder, struct inno_hdmi, encoder); > >> -} > >> - > >> -static struct inno_hdmi *connector_to_inno_hdmi(struct drm_connector > >> *connector) > >... -- With best wishes Dmitry