[PATCH v3 7/7] drm/rockchip: inno-hdmi: Convert to drm bridge

[Dmitry Baryshkov]

On Tue, Apr 08, 2025 at 08:12:29PM +0800, Andy Yan wrote:
> 
> 
> Hi Alex，
> 
> At 2025-04-03 01:24:22, "Alex Bee" <knaerzche at gmail.com> wrote:
> >
> >Hi Andy,
> >
> >> From: Andy Yan <andy.yan at 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.yan at rock-chips.com>
> >> 
> >> ---
> >> 
> >> Changes in v3:
> >> - First included in v3
> >> - Link to V2: https://lore.kernel.org/dri-devel/20250325132944.171111-1-andyshrk@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