Hi Maxime, At 2024-05-21 18:14:01, "Maxime Ripard" <mripard@xxxxxxxxxx> wrote: >The new HDMI connector infrastructure allows to remove some boilerplate, >especially to generate infoframes. Let's switch to it. > >Reviewed-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx> >Acked-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx> >Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx> Reviewed-by: Andy Yan <andyshrk@xxxxxxx> >--- > drivers/gpu/drm/sun4i/Kconfig | 3 ++ > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 81 +++++++++++++++++++++------------- > 2 files changed, 54 insertions(+), 30 deletions(-) > >diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig >index 4741d9f6544c..4037e085430e 100644 >--- a/drivers/gpu/drm/sun4i/Kconfig >+++ b/drivers/gpu/drm/sun4i/Kconfig >@@ -16,10 +16,13 @@ config DRM_SUN4I > if DRM_SUN4I > > config DRM_SUN4I_HDMI > tristate "Allwinner A10/A10s/A20/A31 HDMI Controller Support" > depends on ARM || COMPILE_TEST >+ select DRM_DISPLAY_HDMI_HELPER >+ select DRM_DISPLAY_HDMI_STATE_HELPER >+ select DRM_DISPLAY_HELPER > default DRM_SUN4I > help > Choose this option if you have an Allwinner A10/A10s/A20/A31 > SoC with an HDMI controller. > >diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >index 1c6cda2bfb14..0e652dd480c9 100644 >--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >@@ -38,34 +38,28 @@ > container_of_const(e, struct sun4i_hdmi, encoder) > > #define drm_connector_to_sun4i_hdmi(c) \ > container_of_const(c, struct sun4i_hdmi, connector) > >-static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi, >- struct drm_display_mode *mode) >+static int sun4i_hdmi_write_infoframe(struct drm_connector *connector, >+ enum hdmi_infoframe_type type, >+ const u8 *buffer, size_t len) > { >- struct hdmi_avi_infoframe frame; >- u8 buffer[17]; >- int i, ret; >+ struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); >+ int i; > >- ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, >- &hdmi->connector, mode); >- if (ret < 0) { >- DRM_ERROR("Failed to get infoframes from mode\n"); >- return ret; >+ if (type != HDMI_INFOFRAME_TYPE_AVI) { >+ drm_err(connector->dev, >+ "Unsupported infoframe type: %u\n", type); >+ return 0; > } > >- ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer)); >- if (ret < 0) { >- DRM_ERROR("Failed to pack infoframes\n"); >- return ret; >- } >- >- for (i = 0; i < sizeof(buffer); i++) >+ for (i = 0; i < len; i++) > writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i)); > > return 0; >+ > } > > static void sun4i_hdmi_disable(struct drm_encoder *encoder, > struct drm_atomic_state *state) > { >@@ -84,18 +78,22 @@ static void sun4i_hdmi_disable(struct drm_encoder *encoder, > static void sun4i_hdmi_enable(struct drm_encoder *encoder, > struct drm_atomic_state *state) > { > struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; > struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder); >- struct drm_display_info *display = &hdmi->connector.display_info; >+ struct drm_connector *connector = &hdmi->connector; >+ struct drm_display_info *display = &connector->display_info; >+ struct drm_connector_state *conn_state = >+ drm_atomic_get_new_connector_state(state, connector); >+ unsigned long long tmds_rate = conn_state->hdmi.tmds_char_rate; > unsigned int x, y; > u32 val = 0; > > DRM_DEBUG_DRIVER("Enabling the HDMI Output\n"); > >- clk_set_rate(hdmi->mod_clk, mode->crtc_clock * 1000); >- clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000); >+ clk_set_rate(hdmi->mod_clk, tmds_rate); >+ clk_set_rate(hdmi->tmds_clk, tmds_rate); > > /* Set input sync enable */ > writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC, > hdmi->base + SUN4I_HDMI_UNKNOWN_REG); > >@@ -144,11 +142,12 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder, > > writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG); > > clk_prepare_enable(hdmi->tmds_clk); > >- sun4i_hdmi_setup_avi_infoframes(hdmi, mode); >+ drm_atomic_helper_connector_hdmi_update_infoframes(connector, state); >+ > val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI); > val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END); > writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0)); > > val = SUN4I_HDMI_VID_CTRL_ENABLE; >@@ -197,23 +196,26 @@ static int sun4i_hdmi_connector_atomic_check(struct drm_connector *connector, > struct drm_crtc_state *crtc_state = crtc->state; > struct drm_display_mode *mode = &crtc_state->adjusted_mode; > enum drm_mode_status status; > > status = sun4i_hdmi_connector_clock_valid(connector, mode, >- mode->clock * 1000); >+ conn_state->hdmi.tmds_char_rate); > if (status != MODE_OK) > return -EINVAL; > > return 0; > } > > static enum drm_mode_status > sun4i_hdmi_connector_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > { >- return sun4i_hdmi_connector_clock_valid(connector, mode, >- mode->clock * 1000); >+ unsigned long long rate = >+ drm_connector_hdmi_compute_mode_clock(mode, 8, >+ HDMI_COLORSPACE_RGB); >+ >+ return sun4i_hdmi_connector_clock_valid(connector, mode, rate); > } > > static int sun4i_hdmi_get_modes(struct drm_connector *connector) > { > struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); >@@ -259,10 +261,15 @@ static struct i2c_adapter *sun4i_hdmi_get_ddc(struct device *dev) > return ERR_PTR(-EPROBE_DEFER); > > return ddc; > } > >+static const struct drm_connector_hdmi_funcs sun4i_hdmi_hdmi_connector_funcs = { >+ .tmds_char_rate_valid = sun4i_hdmi_connector_clock_valid, >+ .write_infoframe = sun4i_hdmi_write_infoframe, >+}; >+ > static const struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = { > .atomic_check = sun4i_hdmi_connector_atomic_check, > .mode_valid = sun4i_hdmi_connector_mode_valid, > .get_modes = sun4i_hdmi_get_modes, > }; >@@ -280,15 +287,20 @@ sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) > } > > return connector_status_connected; > } > >+static void sun4i_hdmi_connector_reset(struct drm_connector *connector) >+{ >+ drm_atomic_helper_connector_reset(connector); >+ __drm_atomic_helper_connector_hdmi_reset(connector, connector->state); >+} >+ > static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = { > .detect = sun4i_hdmi_connector_detect, > .fill_modes = drm_helper_probe_single_connector_modes, >- .destroy = drm_connector_cleanup, >- .reset = drm_atomic_helper_connector_reset, >+ .reset = sun4i_hdmi_connector_reset, > .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > #ifdef CONFIG_DRM_SUN4I_HDMI_CEC >@@ -643,14 +655,23 @@ static int sun4i_hdmi_bind(struct device *dev, struct device *master, > hdmi->base + SUN4I_HDMI_CEC); > #endif > > drm_connector_helper_add(&hdmi->connector, > &sun4i_hdmi_connector_helper_funcs); >- ret = drm_connector_init_with_ddc(drm, &hdmi->connector, >- &sun4i_hdmi_connector_funcs, >- DRM_MODE_CONNECTOR_HDMIA, >- hdmi->ddc_i2c); >+ ret = drmm_connector_hdmi_init(drm, &hdmi->connector, >+ /* >+ * NOTE: Those are likely to be >+ * wrong, but I couldn't find the >+ * actual ones in the BSP. >+ */ >+ "AW", "HDMI", >+ &sun4i_hdmi_connector_funcs, >+ &sun4i_hdmi_hdmi_connector_funcs, >+ DRM_MODE_CONNECTOR_HDMIA, >+ hdmi->ddc_i2c, >+ BIT(HDMI_COLORSPACE_RGB), >+ 8); > if (ret) { > dev_err(dev, > "Couldn't initialise the HDMI connector\n"); > goto err_cleanup_connector; > } > >-- >2.45.0