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

[Alex Bee <knaerzche@gmail.com>]

Hi Andy,

> From: Andy Yan <andy.yan@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@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.

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)
...