[v2,00/13] drm/bridge: lt9611: several fixes and improvements

A series of patches to fix mode programming for the Lontium lt9611
DSI-to-HDMI bridge (found e.g. on the Thundercomm RB3/Dragonboard845c
platform).

Changes since v1:
 - Fixed the double-DSI check to look for the lt9611->dsi1_node rather
   than lt9611->dsi1, modesetting happens before lt9611->dsi1 is set.

 - Added full dual-DSI support, properly enabling the 4k modes on RB3.

Dmitry Baryshkov (13):
  drm/bridge: lt9611: fix sleep mode setup
  drm/bridge: lt9611: fix HPD reenablement
  drm/bridge: lt9611: fix polarity programming
  drm/bridge: lt9611: fix programming of video modes
  drm/bridge: lt9611: fix clock calculation
  drm/bridge: lt9611: pass a pointer to the of node
  drm/bridge: lt9611: rework the mode_set function
  drm/bridge: lt9611: attach to the next bridge
  drm/bridge: lt9611: fix sync polarity for DVI output
  drm/bridge: lt9611: simplify video timings programming
  drm/bridge: lt9611: rework infoframes handling
  drm/bridge: lt9611: stop filtering modes via the table
  drm/bridge: lt9611: properly program the dual host mode

 drivers/gpu/drm/bridge/lontium-lt9611.c | 332 ++++++++++--------------
 1 file changed, 133 insertions(+), 199 deletions(-)

On 08/01/2023 17:56, Dmitry Baryshkov wrote:
> The mode_set callback is deprectated for drm_bridges in favour of using
> atomic_enable callback. Move corresponding code into the function
> lt9611_bridge_atomic_enable() and turn lt9611_bridge_pre_enable() into
> the proper atomic_pre_enable callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/bridge/lontium-lt9611.c | 61 +++++++++++++++----------
>   1 file changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 3b77238ca4af..1b65a573be27 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -713,6 +713,39 @@ lt9611_bridge_atomic_enable(struct drm_bridge *bridge,
>   			    struct drm_bridge_state *old_bridge_state)
>   {
>   	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> +	struct drm_atomic_state *state = old_bridge_state->base.state;
> +	struct drm_connector *connector;
> +	struct drm_connector_state *conn_state;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_display_mode *mode;
> +	struct hdmi_avi_infoframe avi_frame;
> +	unsigned int postdiv;
> +	int ret;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	if (WARN_ON(!connector))
> +		return;
> +
> +	conn_state = drm_atomic_get_new_connector_state(state, connector);
> +	if (WARN_ON(!conn_state))
> +		return;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> +	if (WARN_ON(!crtc_state))
> +		return;
> +
> +	mode = &crtc_state->adjusted_mode;
> +
> +	lt9611_mipi_input_digital(lt9611, mode);
> +	lt9611_pll_setup(lt9611, mode, &postdiv);
> +	lt9611_mipi_video_setup(lt9611, mode);
> +	lt9611_pcr_setup(lt9611, mode, postdiv);
> +
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame,
> +						       connector,
> +						       mode);
> +	if (!ret)
> +		lt9611->vic = avi_frame.video_code;
>   
>   	if (lt9611_power_on(lt9611)) {
>   		dev_err(lt9611->dev, "power on failed\n");
> @@ -856,7 +889,8 @@ static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>   		return MODE_OK;
>   }
>   
> -static void lt9611_bridge_pre_enable(struct drm_bridge *bridge)
> +static void lt9611_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> +					    struct drm_bridge_state *old_bridge_state)
>   {
>   	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>   	static const struct reg_sequence reg_cfg[] = {
> @@ -884,29 +918,6 @@ lt9611_bridge_atomic_post_disable(struct drm_bridge *bridge,
>   	lt9611_sleep_setup(lt9611);
>   }
>   
> -static void lt9611_bridge_mode_set(struct drm_bridge *bridge,
> -				   const struct drm_display_mode *mode,
> -				   const struct drm_display_mode *adj_mode)
> -{
> -	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> -	struct hdmi_avi_infoframe avi_frame;
> -	unsigned int postdiv;
> -	int ret;
> -
> -	lt9611_bridge_pre_enable(bridge);
> -
> -	lt9611_mipi_input_digital(lt9611, mode);
> -	lt9611_pll_setup(lt9611, mode, &postdiv);
> -	lt9611_mipi_video_setup(lt9611, mode);
> -	lt9611_pcr_setup(lt9611, mode, postdiv);
> -
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame,
> -						       &lt9611->connector,
> -						       mode);
> -	if (!ret)
> -		lt9611->vic = avi_frame.video_code;
> -}
> -
>   static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
>   {
>   	return __lt9611_detect(bridge_to_lt9611(bridge));
> @@ -957,11 +968,11 @@ lt9611_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>   static const struct drm_bridge_funcs lt9611_bridge_funcs = {
>   	.attach = lt9611_bridge_attach,
>   	.mode_valid = lt9611_bridge_mode_valid,
> -	.mode_set = lt9611_bridge_mode_set,
>   	.detect = lt9611_bridge_detect,
>   	.get_edid = lt9611_bridge_get_edid,
>   	.hpd_enable = lt9611_bridge_hpd_enable,
>   
> +	.atomic_pre_enable = lt9611_bridge_atomic_pre_enable,
>   	.atomic_enable = lt9611_bridge_atomic_enable,
>   	.atomic_disable = lt9611_bridge_atomic_disable,
>   	.atomic_post_disable = lt9611_bridge_atomic_post_disable,


Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

On 08/01/2023 17:56, Dmitry Baryshkov wrote:
> The bindings require that there is a next bridge after the lt9611. If
> nothing else it can be the hdmi-connector (as used on the RB3 platform,
> see sdm845-db845c.dts).
> 
> Bring in the next bridge into the drm bridges chain and attach to it.
> 
> Since lt9611 is not anymore the last bridge in the chain, this also
> allows us to drop all the !DRM_BRIDGE_ATTACH_NO_CONNECTOR functionality.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/bridge/lontium-lt9611.c | 99 ++-----------------------
>   1 file changed, 7 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 1b65a573be27..773d7a56f86f 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -19,6 +19,7 @@
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_bridge.h>
>   #include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_probe_helper.h>
>   
> @@ -33,7 +34,7 @@
>   struct lt9611 {
>   	struct device *dev;
>   	struct drm_bridge bridge;
> -	struct drm_connector connector;
> +	struct drm_bridge *next_bridge;
>   
>   	struct regmap *regmap;
>   
> @@ -107,11 +108,6 @@ static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
>   	return container_of(bridge, struct lt9611, bridge);
>   }
>   
> -static struct lt9611 *connector_to_lt9611(struct drm_connector *connector)
> -{
> -	return container_of(connector, struct lt9611, connector);
> -}
> -
>   static int lt9611_mipi_input_analog(struct lt9611 *lt9611)
>   {
>   	const struct reg_sequence reg_cfg[] = {
> @@ -581,9 +577,9 @@ static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode)
>   	return NULL;
>   }
>   
> -/* connector funcs */
> -static enum drm_connector_status __lt9611_detect(struct lt9611 *lt9611)
> +static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
>   {
> +	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>   	unsigned int reg_val = 0;
>   	int connected = 0;
>   
> @@ -596,12 +592,6 @@ static enum drm_connector_status __lt9611_detect(struct lt9611 *lt9611)
>   	return lt9611->status;
>   }
>   
> -static enum drm_connector_status
> -lt9611_connector_detect(struct drm_connector *connector, bool force)
> -{
> -	return __lt9611_detect(connector_to_lt9611(connector));
> -}
> -
>   static int lt9611_read_edid(struct lt9611 *lt9611)
>   {
>   	unsigned int temp;
> @@ -683,30 +673,6 @@ lt9611_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
>   	return 0;
>   }
>   
> -static int lt9611_connector_get_modes(struct drm_connector *connector)
> -{
> -	struct lt9611 *lt9611 = connector_to_lt9611(connector);
> -	unsigned int count;
> -	struct edid *edid;
> -
> -	lt9611_power_on(lt9611);
> -	edid = drm_do_get_edid(connector, lt9611_get_edid_block, lt9611);
> -	drm_connector_update_edid_property(connector, edid);
> -	count = drm_add_edid_modes(connector, edid);
> -	kfree(edid);
> -
> -	return count;
> -}
> -
> -static enum drm_mode_status
> -lt9611_connector_mode_valid(struct drm_connector *connector,
> -			    struct drm_display_mode *mode)
> -{
> -	struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
> -
> -	return lt9611_mode ? MODE_OK : MODE_BAD;
> -}
> -
>   /* bridge funcs */
>   static void
>   lt9611_bridge_atomic_enable(struct drm_bridge *bridge,
> @@ -784,21 +750,6 @@ lt9611_bridge_atomic_disable(struct drm_bridge *bridge,
>   	}
>   }
>   
> -static struct
> -drm_connector_helper_funcs lt9611_bridge_connector_helper_funcs = {
> -	.get_modes = lt9611_connector_get_modes,
> -	.mode_valid = lt9611_connector_mode_valid,
> -};
> -
> -static const struct drm_connector_funcs lt9611_bridge_connector_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.detect = lt9611_connector_detect,
> -	.destroy = drm_connector_cleanup,
> -	.reset = drm_atomic_helper_connector_reset,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
>   static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
>   						 struct device_node *dsi_node)
>   {
> @@ -834,44 +785,13 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
>   	return dsi;
>   }
>   
> -static int lt9611_connector_init(struct drm_bridge *bridge, struct lt9611 *lt9611)
> -{
> -	int ret;
> -
> -	ret = drm_connector_init(bridge->dev, &lt9611->connector,
> -				 &lt9611_bridge_connector_funcs,
> -				 DRM_MODE_CONNECTOR_HDMIA);
> -	if (ret) {
> -		DRM_ERROR("Failed to initialize connector with drm\n");
> -		return ret;
> -	}
> -
> -	drm_connector_helper_add(&lt9611->connector,
> -				 &lt9611_bridge_connector_helper_funcs);
> -
> -	if (!bridge->encoder) {
> -		DRM_ERROR("Parent encoder object not found");
> -		return -ENODEV;
> -	}
> -
> -	drm_connector_attach_encoder(&lt9611->connector, bridge->encoder);
> -
> -	return 0;
> -}
> -
>   static int lt9611_bridge_attach(struct drm_bridge *bridge,
>   				enum drm_bridge_attach_flags flags)
>   {
>   	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> -	int ret;
> -
> -	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> -		ret = lt9611_connector_init(bridge, lt9611);
> -		if (ret < 0)
> -			return ret;
> -	}
>   
> -	return 0;
> +	return drm_bridge_attach(bridge->encoder, lt9611->next_bridge,
> +				 bridge, flags);
>   }
>   
>   static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
> @@ -918,11 +838,6 @@ lt9611_bridge_atomic_post_disable(struct drm_bridge *bridge,
>   	lt9611_sleep_setup(lt9611);
>   }
>   
> -static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
> -{
> -	return __lt9611_detect(bridge_to_lt9611(bridge));
> -}
> -
>   static struct edid *lt9611_bridge_get_edid(struct drm_bridge *bridge,
>   					   struct drm_connector *connector)
>   {
> @@ -995,7 +910,7 @@ static int lt9611_parse_dt(struct device *dev,
>   
>   	lt9611->ac_mode = of_property_read_bool(dev->of_node, "lt,ac-mode");
>   
> -	return 0;
> +	return drm_of_find_panel_or_bridge(dev->of_node, 2, -1, NULL, &lt9611->next_bridge);
>   }
>   
>   static int lt9611_gpio_init(struct lt9611 *lt9611)

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>