Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650555 > unrolled thread

[PATCH v5 00/10] Introduce new mode validation callbacks

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 00/10] Introduce new mode validation callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-25 16:30 +0200
    [PATCH v5 03/10] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-25 16:30 +0200
      Re: [PATCH v5 03/10] drm: Use new mode_valid() helpers in connector  probe helper Neil Armstrong <narmstrong@baylibre.com> - 2017-05-30 09:30 +0200
    [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-25 16:30 +0200
      Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use  bridge->mode_valid() callback Neil Armstrong <narmstrong@baylibre.com> - 2017-05-29 11:50 +0200
      Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use  bridge->mode_valid() callback Philipp Zabel <p.zabel@pengutronix.de> - 2017-05-30 12:30 +0200
        Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use  bridge->mode_valid() callback Archit Taneja <architt@codeaurora.org> - 2017-06-05 10:00 +0200
      Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use  bridge->mode_valid() callback Archit Taneja <architt@codeaurora.org> - 2017-05-30 12:30 +0200

#1650555 — [PATCH v5 00/10] Introduce new mode validation callbacks

NOTE: In this version I just did a rebase onto today's drm-next and
added the tags we've collected plus the changelog plus the missing
maintainers that I forgot to cc in previous version :/

This series is a follow up from the discussion at [1]. We start by
introducing crtc->mode_valid(), encoder->mode_valid() and
bridge->mode_valid() callbacks which will be used in followup
patches and also by cleaning the documentation a little bit. This
patch is available at [2] and the series depends on it.

We proceed by introducing new helpers to call this new callbacks
at 1/10.

At 2/10 a helper function is introduced that calls all mode_valid()
from a set of bridges.

Next, at 3/10 we modify the connector probe helper so that only modes
which are supported by a given bridge+encoder+crtc combination are
probbed.

At 4/10 we call all the mode_valid() callbacks for a given pipeline,
except the connector->mode_valid one, so that the mode is validated.
This is done before calling mode_fixup().

Finally, from 5/10 to 10/10 we use the new callbacks in drivers that
can implement it.

[1] https://patchwork.kernel.org/patch/9702233/
[2] https://lists.freedesktop.org/archives/dri-devel/2017-May/141761.html

Jose Abreu (10):
  drm: Add drm_{crtc/encoder/connector}_mode_valid()
  drm: Introduce drm_bridge_mode_valid()
  drm: Use new mode_valid() helpers in connector probe helper
  drm: Use mode_valid() in atomic modeset
  drm: arcpgu: Use crtc->mode_valid() callback
  drm/bridge: analogix-anx78xx: Use bridge->mode_valid() callback
  drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback
  drm/arm: malidp: Use crtc->mode_valid() callback
  drm/atmel-hlcdc: Use crtc->mode_valid() callback
  drm: vc4: Use crtc->mode_valid() and encoder->mode_valid() callbacks

Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@linux.ie>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

 drivers/gpu/drm/arc/arcpgu_crtc.c              |  29 ++++---
 drivers/gpu/drm/arm/malidp_crtc.c              |  11 ++-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |   9 +--
 drivers/gpu/drm/bridge/analogix-anx78xx.c      |  13 ++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c      |  40 +++-------
 drivers/gpu/drm/drm_atomic_helper.c            |  76 +++++++++++++++++-
 drivers/gpu/drm/drm_bridge.c                   |  33 ++++++++
 drivers/gpu/drm/drm_crtc_helper_internal.h     |  13 +++
 drivers/gpu/drm/drm_probe_helper.c             | 105 ++++++++++++++++++++++++-
 drivers/gpu/drm/imx/dw_hdmi-imx.c              |   4 +-
 drivers/gpu/drm/meson/meson_dw_hdmi.c          |   2 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c    |   2 +-
 drivers/gpu/drm/vc4/vc4_crtc.c                 |  13 ++-
 drivers/gpu/drm/vc4/vc4_dpi.c                  |  13 ++-
 include/drm/bridge/dw_hdmi.h                   |   2 +-
 include/drm/drm_bridge.h                       |   2 +
 16 files changed, 280 insertions(+), 87 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1650560 — [PATCH v5 03/10] drm: Use new mode_valid() helpers in connector probe helper

This changes the connector probe helper function to use the new
encoder->mode_valid(), bridge->mode_valid() and crtc->mode_valid()
helper callbacks to validate the modes.

The new callbacks are optional so the behaviour remains the same
if they are not implemented. If they are, then the code loops
through all the connector's encodersXbridgesXcrtcs and calls the
callback.

If at least a valid encoderXbridgeXcrtc combination is found which
accepts the mode then the function returns MODE_OK.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Dave Airlie <airlied@linux.ie>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Changes v3->v4:
	- Change function name (Laurent)
Changes v2->v3:
	- Call also bridge->mode_valid (Daniel)
Changes v1->v2:
	- Use new helpers suggested by Ville
	- Change documentation (Daniel)

---
 drivers/gpu/drm/drm_probe_helper.c | 67 +++++++++++++++++++++++++++++++++++---
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index f01abdc..00e6832 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -83,6 +83,61 @@
 	return MODE_OK;
 }
 
+static enum drm_mode_status
+drm_mode_validate_pipeline(struct drm_display_mode *mode,
+			    struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	uint32_t *ids = connector->encoder_ids;
+	enum drm_mode_status ret = MODE_OK;
+	unsigned int i;
+
+	/* Step 1: Validate against connector */
+	ret = drm_connector_mode_valid(connector, mode);
+	if (ret != MODE_OK)
+		return ret;
+
+	/* Step 2: Validate against encoders and crtcs */
+	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+		struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
+		struct drm_crtc *crtc;
+
+		if (!encoder)
+			continue;
+
+		ret = drm_encoder_mode_valid(encoder, mode);
+		if (ret != MODE_OK) {
+			/* No point in continuing for crtc check as this encoder
+			 * will not accept the mode anyway. If all encoders
+			 * reject the mode then, at exit, ret will not be
+			 * MODE_OK. */
+			continue;
+		}
+
+		ret = drm_bridge_mode_valid(encoder->bridge, mode);
+		if (ret != MODE_OK) {
+			/* There is also no point in continuing for crtc check
+			 * here. */
+			continue;
+		}
+
+		drm_for_each_crtc(crtc, dev) {
+			if (!drm_encoder_crtc_ok(encoder, crtc))
+				continue;
+
+			ret = drm_crtc_mode_valid(crtc, mode);
+			if (ret == MODE_OK) {
+				/* If we get to this point there is at least
+				 * one combination of encoder+crtc that works
+				 * for this mode. Lets return now. */
+				return ret;
+			}
+		}
+	}
+
+	return ret;
+}
+
 static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
 {
 	struct drm_cmdline_mode *cmdline_mode;
@@ -322,7 +377,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
  *    - drm_mode_validate_flag() checks the modes against basic connector
  *      capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
  *    - the optional &drm_connector_helper_funcs.mode_valid helper can perform
- *      driver and/or hardware specific checks
+ *      driver and/or sink specific checks
+ *    - the optional &drm_crtc_helper_funcs.mode_valid,
+ *      &drm_bridge_funcs.mode_valid and &drm_encoder_helper_funcs.mode_valid
+ *      helpers can perform driver and/or source specific checks which are also
+ *      enforced by the modeset/atomic helpers
  *
  * 5. Any mode whose status is not OK is pruned from the connector's modes list,
  *    accompanied by a debug message indicating the reason for the mode's
@@ -466,9 +525,9 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		if (mode->status == MODE_OK)
 			mode->status = drm_mode_validate_flag(mode, mode_flags);
 
-		if (mode->status == MODE_OK && connector_funcs->mode_valid)
-			mode->status = connector_funcs->mode_valid(connector,
-								   mode);
+		if (mode->status == MODE_OK)
+			mode->status = drm_mode_validate_pipeline(mode,
+								  connector);
 	}
 
 prune:
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1652890 — Re: [PATCH v5 03/10] drm: Use new mode_valid() helpers in connector probe helper

On 05/25/2017 04:19 PM, Jose Abreu wrote:
> This changes the connector probe helper function to use the new
> encoder->mode_valid(), bridge->mode_valid() and crtc->mode_valid()
> helper callbacks to validate the modes.
> 
> The new callbacks are optional so the behaviour remains the same
> if they are not implemented. If they are, then the code loops
> through all the connector's encodersXbridgesXcrtcs and calls the
> callback.
> 
> If at least a valid encoderXbridgeXcrtc combination is found which
> accepts the mode then the function returns MODE_OK.
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Dave Airlie <airlied@linux.ie>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Changes v3->v4:
> 	- Change function name (Laurent)
> Changes v2->v3:
> 	- Call also bridge->mode_valid (Daniel)
> Changes v1->v2:
> 	- Use new helpers suggested by Ville
> 	- Change documentation (Daniel)
> 
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 67 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index f01abdc..00e6832 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -83,6 +83,61 @@
>  	return MODE_OK;
>  }
>  
> +static enum drm_mode_status
> +drm_mode_validate_pipeline(struct drm_display_mode *mode,
> +			    struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	uint32_t *ids = connector->encoder_ids;
> +	enum drm_mode_status ret = MODE_OK;
> +	unsigned int i;
> +
> +	/* Step 1: Validate against connector */
> +	ret = drm_connector_mode_valid(connector, mode);
> +	if (ret != MODE_OK)
> +		return ret;
> +
> +	/* Step 2: Validate against encoders and crtcs */
> +	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> +		struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]);
> +		struct drm_crtc *crtc;
> +
> +		if (!encoder)
> +			continue;
> +
> +		ret = drm_encoder_mode_valid(encoder, mode);
> +		if (ret != MODE_OK) {
> +			/* No point in continuing for crtc check as this encoder
> +			 * will not accept the mode anyway. If all encoders
> +			 * reject the mode then, at exit, ret will not be
> +			 * MODE_OK. */
> +			continue;
> +		}
> +
> +		ret = drm_bridge_mode_valid(encoder->bridge, mode);
> +		if (ret != MODE_OK) {
> +			/* There is also no point in continuing for crtc check
> +			 * here. */
> +			continue;
> +		}
> +
> +		drm_for_each_crtc(crtc, dev) {
> +			if (!drm_encoder_crtc_ok(encoder, crtc))
> +				continue;
> +
> +			ret = drm_crtc_mode_valid(crtc, mode);
> +			if (ret == MODE_OK) {
> +				/* If we get to this point there is at least
> +				 * one combination of encoder+crtc that works
> +				 * for this mode. Lets return now. */
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>  {
>  	struct drm_cmdline_mode *cmdline_mode;
> @@ -322,7 +377,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>   *    - drm_mode_validate_flag() checks the modes against basic connector
>   *      capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>   *    - the optional &drm_connector_helper_funcs.mode_valid helper can perform
> - *      driver and/or hardware specific checks
> + *      driver and/or sink specific checks
> + *    - the optional &drm_crtc_helper_funcs.mode_valid,
> + *      &drm_bridge_funcs.mode_valid and &drm_encoder_helper_funcs.mode_valid
> + *      helpers can perform driver and/or source specific checks which are also
> + *      enforced by the modeset/atomic helpers
>   *
>   * 5. Any mode whose status is not OK is pruned from the connector's modes list,
>   *    accompanied by a debug message indicating the reason for the mode's
> @@ -466,9 +525,9 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  		if (mode->status == MODE_OK)
>  			mode->status = drm_mode_validate_flag(mode, mode_flags);
>  
> -		if (mode->status == MODE_OK && connector_funcs->mode_valid)
> -			mode->status = connector_funcs->mode_valid(connector,
> -								   mode);
> +		if (mode->status == MODE_OK)
> +			mode->status = drm_mode_validate_pipeline(mode,
> +								  connector);
>  	}
>  
>  prune:
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[toc] | [prev] | [next] | [standalone]

#1650561 — [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback

Now that we have a callback to check if bridge supports a given mode
we can use it in Synopsys Designware HDMI bridge so that we restrict
the number of probbed modes to the ones we can actually display.

Also, there is no need to use mode_fixup() callback as mode_valid()
will handle the mode validation.

NOTE: Only compile tested
NOTE 2: I also had to change the pdata declaration of mode_valid
custom callback so that the passed modes are const. I also changed
in the platforms I found. Not even compiled it though.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Carlo Caione <carlo@caione.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Mark Yao <mark.yao@rock-chips.com>
Cc: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 40 +++++++++--------------------
 drivers/gpu/drm/imx/dw_hdmi-imx.c           |  4 +--
 drivers/gpu/drm/meson/meson_dw_hdmi.c       |  2 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  2 +-
 include/drm/bridge/dw_hdmi.h                |  2 +-
 5 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 8737de8..038dc43 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1907,24 +1907,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 	return ret;
 }
 
-static enum drm_mode_status
-dw_hdmi_connector_mode_valid(struct drm_connector *connector,
-			     struct drm_display_mode *mode)
-{
-	struct dw_hdmi *hdmi = container_of(connector,
-					   struct dw_hdmi, connector);
-	enum drm_mode_status mode_status = MODE_OK;
-
-	/* We don't support double-clocked modes */
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		return MODE_BAD;
-
-	if (hdmi->plat_data->mode_valid)
-		mode_status = hdmi->plat_data->mode_valid(connector, mode);
-
-	return mode_status;
-}
-
 static void dw_hdmi_connector_force(struct drm_connector *connector)
 {
 	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
@@ -1950,7 +1932,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
 
 static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = {
 	.get_modes = dw_hdmi_connector_get_modes,
-	.mode_valid = dw_hdmi_connector_mode_valid,
 	.best_encoder = drm_atomic_helper_best_encoder,
 };
 
@@ -1973,18 +1954,21 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
 	return 0;
 }
 
-static bool dw_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
-				      const struct drm_display_mode *orig_mode,
-				      struct drm_display_mode *mode)
+static enum drm_mode_status dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
+						      const struct drm_display_mode *mode)
 {
 	struct dw_hdmi *hdmi = bridge->driver_private;
 	struct drm_connector *connector = &hdmi->connector;
-	enum drm_mode_status status;
+	enum drm_mode_status mode_status = MODE_OK;
 
-	status = dw_hdmi_connector_mode_valid(connector, mode);
-	if (status != MODE_OK)
-		return false;
-	return true;
+	/* We don't support double-clocked modes */
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		return MODE_BAD;
+
+	if (hdmi->plat_data->mode_valid)
+		mode_status = hdmi->plat_data->mode_valid(connector, mode);
+
+	return mode_status;
 }
 
 static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
@@ -2028,7 +2012,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
 	.enable = dw_hdmi_bridge_enable,
 	.disable = dw_hdmi_bridge_disable,
 	.mode_set = dw_hdmi_bridge_mode_set,
-	.mode_fixup = dw_hdmi_bridge_mode_fixup,
+	.mode_valid = dw_hdmi_bridge_mode_valid,
 };
 
 static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi)
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index f039641..5f561c8 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -148,7 +148,7 @@ static int dw_hdmi_imx_atomic_check(struct drm_encoder *encoder,
 };
 
 static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
-						  struct drm_display_mode *mode)
+						  const struct drm_display_mode *mode)
 {
 	if (mode->clock < 13500)
 		return MODE_CLOCK_LOW;
@@ -160,7 +160,7 @@ static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
 }
 
 static enum drm_mode_status imx6dl_hdmi_mode_valid(struct drm_connector *con,
-						   struct drm_display_mode *mode)
+						   const struct drm_display_mode *mode)
 {
 	if (mode->clock < 13500)
 		return MODE_CLOCK_LOW;
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 7b86eb7..f043904 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -537,7 +537,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
 
 /* TOFIX Enable support for non-vic modes */
 static enum drm_mode_status dw_hdmi_mode_valid(struct drm_connector *connector,
-					       struct drm_display_mode *mode)
+					       const struct drm_display_mode *mode)
 {
 	unsigned int vclk_freq;
 	unsigned int venc_freq;
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 63dab6f..f820848 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -155,7 +155,7 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 
 static enum drm_mode_status
 dw_hdmi_rockchip_mode_valid(struct drm_connector *connector,
-			    struct drm_display_mode *mode)
+			    const struct drm_display_mode *mode)
 {
 	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
 	int pclk = mode->clock * 1000;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index ed599be..4c8d4c8 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -125,7 +125,7 @@ struct dw_hdmi_phy_ops {
 struct dw_hdmi_plat_data {
 	struct regmap *regm;
 	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
-					   struct drm_display_mode *mode);
+					   const struct drm_display_mode *mode);
 	unsigned long input_bus_format;
 	unsigned long input_bus_encoding;
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1652385 — Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback

On 05/25/2017 04:19 PM, Jose Abreu wrote:
> Now that we have a callback to check if bridge supports a given mode
> we can use it in Synopsys Designware HDMI bridge so that we restrict
> the number of probbed modes to the ones we can actually display.
> 
> Also, there is no need to use mode_fixup() callback as mode_valid()
> will handle the mode validation.
> 
> NOTE: Only compile tested
> NOTE 2: I also had to change the pdata declaration of mode_valid
> custom callback so that the passed modes are const. I also changed
> in the platforms I found. Not even compiled it though.
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Carlo Caione <carlo@caione.org>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 40 +++++++++--------------------
>  drivers/gpu/drm/imx/dw_hdmi-imx.c           |  4 +--
>  drivers/gpu/drm/meson/meson_dw_hdmi.c       |  2 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  2 +-
>  include/drm/bridge/dw_hdmi.h                |  2 +-
>  5 files changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 8737de8..038dc43 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1907,24 +1907,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
>  
> -static enum drm_mode_status
> -dw_hdmi_connector_mode_valid(struct drm_connector *connector,
> -			     struct drm_display_mode *mode)
> -{
> -	struct dw_hdmi *hdmi = container_of(connector,
> -					   struct dw_hdmi, connector);
> -	enum drm_mode_status mode_status = MODE_OK;
> -
> -	/* We don't support double-clocked modes */
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		return MODE_BAD;
> -
> -	if (hdmi->plat_data->mode_valid)
> -		mode_status = hdmi->plat_data->mode_valid(connector, mode);
> -
> -	return mode_status;
> -}
> -
>  static void dw_hdmi_connector_force(struct drm_connector *connector)
>  {
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> @@ -1950,7 +1932,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>  
>  static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = {
>  	.get_modes = dw_hdmi_connector_get_modes,
> -	.mode_valid = dw_hdmi_connector_mode_valid,
>  	.best_encoder = drm_atomic_helper_best_encoder,
>  };
>  
> @@ -1973,18 +1954,21 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
>  	return 0;
>  }
>  
> -static bool dw_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> -				      const struct drm_display_mode *orig_mode,
> -				      struct drm_display_mode *mode)
> +static enum drm_mode_status dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> +						      const struct drm_display_mode *mode)
>  {
>  	struct dw_hdmi *hdmi = bridge->driver_private;
>  	struct drm_connector *connector = &hdmi->connector;
> -	enum drm_mode_status status;
> +	enum drm_mode_status mode_status = MODE_OK;
>  
> -	status = dw_hdmi_connector_mode_valid(connector, mode);
> -	if (status != MODE_OK)
> -		return false;
> -	return true;
> +	/* We don't support double-clocked modes */
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		return MODE_BAD;
> +
> +	if (hdmi->plat_data->mode_valid)
> +		mode_status = hdmi->plat_data->mode_valid(connector, mode);
> +
> +	return mode_status;
>  }
>  
>  static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> @@ -2028,7 +2012,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>  	.enable = dw_hdmi_bridge_enable,
>  	.disable = dw_hdmi_bridge_disable,
>  	.mode_set = dw_hdmi_bridge_mode_set,
> -	.mode_fixup = dw_hdmi_bridge_mode_fixup,
> +	.mode_valid = dw_hdmi_bridge_mode_valid,
>  };
>  
>  static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi)
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index f039641..5f561c8 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -148,7 +148,7 @@ static int dw_hdmi_imx_atomic_check(struct drm_encoder *encoder,
>  };
>  
>  static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
> -						  struct drm_display_mode *mode)
> +						  const struct drm_display_mode *mode)
>  {
>  	if (mode->clock < 13500)
>  		return MODE_CLOCK_LOW;
> @@ -160,7 +160,7 @@ static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
>  }
>  
>  static enum drm_mode_status imx6dl_hdmi_mode_valid(struct drm_connector *con,
> -						   struct drm_display_mode *mode)
> +						   const struct drm_display_mode *mode)
>  {
>  	if (mode->clock < 13500)
>  		return MODE_CLOCK_LOW;
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 7b86eb7..f043904 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -537,7 +537,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
>  
>  /* TOFIX Enable support for non-vic modes */
>  static enum drm_mode_status dw_hdmi_mode_valid(struct drm_connector *connector,
> -					       struct drm_display_mode *mode)
> +					       const struct drm_display_mode *mode)
>  {
>  	unsigned int vclk_freq;
>  	unsigned int venc_freq;
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 63dab6f..f820848 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -155,7 +155,7 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
>  
>  static enum drm_mode_status
>  dw_hdmi_rockchip_mode_valid(struct drm_connector *connector,
> -			    struct drm_display_mode *mode)
> +			    const struct drm_display_mode *mode)
>  {
>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
>  	int pclk = mode->clock * 1000;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index ed599be..4c8d4c8 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -125,7 +125,7 @@ struct dw_hdmi_phy_ops {
>  struct dw_hdmi_plat_data {
>  	struct regmap *regm;
>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> -					   struct drm_display_mode *mode);
> +					   const struct drm_display_mode *mode);
>  	unsigned long input_bus_format;
>  	unsigned long input_bus_encoding;
>  
> 


Hi Jose,

Tested-by: Neil Armstrong <narmstrong@baylibre.com>

On Meson GXL, no issues.

Thanks,
Neil

[toc] | [prev] | [next] | [standalone]

#1653082 — Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback

Hi Archit,

On Tue, 2017-05-30 at 15:54 +0530, Archit Taneja wrote:
> Hi,
> 
> On 05/25/2017 07:49 PM, Jose Abreu wrote:
> > Now that we have a callback to check if bridge supports a given mode
> > we can use it in Synopsys Designware HDMI bridge so that we restrict
> > the number of probbed modes to the ones we can actually display.
> >
> > Also, there is no need to use mode_fixup() callback as mode_valid()
> > will handle the mode validation.
> >
> > NOTE: Only compile tested
> > NOTE 2: I also had to change the pdata declaration of mode_valid
> > custom callback so that the passed modes are const. I also changed
> > in the platforms I found. Not even compiled it though.
> 
> This compiles fine and has been tested on Meson by Neil.
> 
> Since this also touches rockchip/imx drm driver files, can I get an
> OK from the maintainers to pull the changes via drm-misc?

Of course,
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1657331 — Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback

On 05/30/2017 03:59 PM, Philipp Zabel wrote:
> Hi Archit,
>
> On Tue, 2017-05-30 at 15:54 +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 05/25/2017 07:49 PM, Jose Abreu wrote:
>>> Now that we have a callback to check if bridge supports a given mode
>>> we can use it in Synopsys Designware HDMI bridge so that we restrict
>>> the number of probbed modes to the ones we can actually display.
>>>
>>> Also, there is no need to use mode_fixup() callback as mode_valid()
>>> will handle the mode validation.
>>>
>>> NOTE: Only compile tested
>>> NOTE 2: I also had to change the pdata declaration of mode_valid
>>> custom callback so that the passed modes are const. I also changed
>>> in the platforms I found. Not even compiled it though.
>>
>> This compiles fine and has been tested on Meson by Neil.
>>
>> Since this also touches rockchip/imx drm driver files, can I get an
>> OK from the maintainers to pull the changes via drm-misc?
>
> Of course,
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks for the Ack.

I didn't get an ack from Rockchip, but still going ahead with
the push since it's been a while and the change is very minor.

Thanks,
Archit

>
> regards
> Philipp
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1653087 — Re: [PATCH v5 07/10] drm/bridge/synopsys: dw-hdmi: Use bridge->mode_valid() callback

Hi,

On 05/25/2017 07:49 PM, Jose Abreu wrote:
> Now that we have a callback to check if bridge supports a given mode
> we can use it in Synopsys Designware HDMI bridge so that we restrict
> the number of probbed modes to the ones we can actually display.
>
> Also, there is no need to use mode_fixup() callback as mode_valid()
> will handle the mode validation.
>
> NOTE: Only compile tested
> NOTE 2: I also had to change the pdata declaration of mode_valid
> custom callback so that the passed modes are const. I also changed
> in the platforms I found. Not even compiled it though.

This compiles fine and has been tested on Meson by Neil.

Since this also touches rockchip/imx drm driver files, can I get an
OK from the maintainers to pull the changes via drm-misc?

Thanks,
Archit

>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Carlo Caione <carlo@caione.org>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c   | 40 +++++++++--------------------
>  drivers/gpu/drm/imx/dw_hdmi-imx.c           |  4 +--
>  drivers/gpu/drm/meson/meson_dw_hdmi.c       |  2 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  2 +-
>  include/drm/bridge/dw_hdmi.h                |  2 +-
>  5 files changed, 17 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 8737de8..038dc43 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1907,24 +1907,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
>
> -static enum drm_mode_status
> -dw_hdmi_connector_mode_valid(struct drm_connector *connector,
> -			     struct drm_display_mode *mode)
> -{
> -	struct dw_hdmi *hdmi = container_of(connector,
> -					   struct dw_hdmi, connector);
> -	enum drm_mode_status mode_status = MODE_OK;
> -
> -	/* We don't support double-clocked modes */
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		return MODE_BAD;
> -
> -	if (hdmi->plat_data->mode_valid)
> -		mode_status = hdmi->plat_data->mode_valid(connector, mode);
> -
> -	return mode_status;
> -}
> -
>  static void dw_hdmi_connector_force(struct drm_connector *connector)
>  {
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
> @@ -1950,7 +1932,6 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>
>  static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = {
>  	.get_modes = dw_hdmi_connector_get_modes,
> -	.mode_valid = dw_hdmi_connector_mode_valid,
>  	.best_encoder = drm_atomic_helper_best_encoder,
>  };
>
> @@ -1973,18 +1954,21 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
>  	return 0;
>  }
>
> -static bool dw_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> -				      const struct drm_display_mode *orig_mode,
> -				      struct drm_display_mode *mode)
> +static enum drm_mode_status dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
> +						      const struct drm_display_mode *mode)
>  {
>  	struct dw_hdmi *hdmi = bridge->driver_private;
>  	struct drm_connector *connector = &hdmi->connector;
> -	enum drm_mode_status status;
> +	enum drm_mode_status mode_status = MODE_OK;
>
> -	status = dw_hdmi_connector_mode_valid(connector, mode);
> -	if (status != MODE_OK)
> -		return false;
> -	return true;
> +	/* We don't support double-clocked modes */
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		return MODE_BAD;
> +
> +	if (hdmi->plat_data->mode_valid)
> +		mode_status = hdmi->plat_data->mode_valid(connector, mode);
> +
> +	return mode_status;
>  }
>
>  static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> @@ -2028,7 +2012,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>  	.enable = dw_hdmi_bridge_enable,
>  	.disable = dw_hdmi_bridge_disable,
>  	.mode_set = dw_hdmi_bridge_mode_set,
> -	.mode_fixup = dw_hdmi_bridge_mode_fixup,
> +	.mode_valid = dw_hdmi_bridge_mode_valid,
>  };
>
>  static irqreturn_t dw_hdmi_i2c_irq(struct dw_hdmi *hdmi)
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index f039641..5f561c8 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -148,7 +148,7 @@ static int dw_hdmi_imx_atomic_check(struct drm_encoder *encoder,
>  };
>
>  static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
> -						  struct drm_display_mode *mode)
> +						  const struct drm_display_mode *mode)
>  {
>  	if (mode->clock < 13500)
>  		return MODE_CLOCK_LOW;
> @@ -160,7 +160,7 @@ static enum drm_mode_status imx6q_hdmi_mode_valid(struct drm_connector *con,
>  }
>
>  static enum drm_mode_status imx6dl_hdmi_mode_valid(struct drm_connector *con,
> -						   struct drm_display_mode *mode)
> +						   const struct drm_display_mode *mode)
>  {
>  	if (mode->clock < 13500)
>  		return MODE_CLOCK_LOW;
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 7b86eb7..f043904 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -537,7 +537,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
>
>  /* TOFIX Enable support for non-vic modes */
>  static enum drm_mode_status dw_hdmi_mode_valid(struct drm_connector *connector,
> -					       struct drm_display_mode *mode)
> +					       const struct drm_display_mode *mode)
>  {
>  	unsigned int vclk_freq;
>  	unsigned int venc_freq;
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 63dab6f..f820848 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -155,7 +155,7 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
>
>  static enum drm_mode_status
>  dw_hdmi_rockchip_mode_valid(struct drm_connector *connector,
> -			    struct drm_display_mode *mode)
> +			    const struct drm_display_mode *mode)
>  {
>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
>  	int pclk = mode->clock * 1000;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index ed599be..4c8d4c8 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -125,7 +125,7 @@ struct dw_hdmi_phy_ops {
>  struct dw_hdmi_plat_data {
>  	struct regmap *regm;
>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> -					   struct drm_display_mode *mode);
> +					   const struct drm_display_mode *mode);
>  	unsigned long input_bus_format;
>  	unsigned long input_bus_encoding;
>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web