[U-Boot] [PATCH 0/3] video: add support for EDID timings validation

[U-Boot] [PATCH 0/3] video: add support for EDID timings validation

[Neil Armstrong]

This patch introduces an edid_get_timing_validate() variant function with
a mode_valid() callback in order to filter supported timings from a display
driver while adding a new mode_valid() display op.

This permits selecting a valid timing on Amlogic SoCs when plugged on
4K monitors/TVs for example.

Neil Armstrong (3):
  edid: add edid_get_timing_validate() variant to filter out edid modes
  video: display: use edid_get_timing_validate() variant to filter
    supported EDID modes
  video: meson: dw-hdmi: add EDID mode filtering to only select
    supported modes

 common/edid.c                       | 22 +++++++++++++++++++---
 drivers/video/display-uclass.c      | 15 ++++++++++++++-
 drivers/video/meson/meson_dw_hdmi.c |  7 +++++++
 include/display.h                   | 10 ++++++++++
 include/edid.h                      | 22 ++++++++++++++++++++++
 5 files changed, 72 insertions(+), 4 deletions(-)

-- 
2.21.0

[U-Boot] [PATCH 1/3] edid: add edid_get_timing_validate() variant to filter out edid modes

[Neil Armstrong]

The original edid_get_timing() function returns the first valid timing,
but on some plaforms, we could only supports a subset of the listed
monitot's navite timing.

Let's introduce a edid_get_timing_validate() adding a mode_valid callback
including a private cookie pointer.

If the callback returns false, the current timing is discared and the next
one is checked. If no valid & supported timings are found, the function
would return an error.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 common/edid.c  | 22 +++++++++++++++++++---
 include/edid.h | 22 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/common/edid.c b/common/edid.c
index 90d1167f6e..f244d26e04 100644
--- a/common/edid.c
+++ b/common/edid.c
@@ -168,8 +168,12 @@ static bool cea_is_hdmi_vsdb_present(struct edid_cea861_info *info)
 	return false;
 }
 
-int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
-		    int *panel_bits_per_colourp)
+int edid_get_timing_validate(u8 *buf, int buf_size,
+			     struct display_timing *timing,
+			     int *panel_bits_per_colourp,
+			     bool (*mode_valid)(void *priv,
+					const struct display_timing *timing),
+			     void *mode_valid_priv)
 {
 	struct edid1_info *edid = (struct edid1_info *)buf;
 	bool timing_done;
@@ -193,7 +197,11 @@ int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
 		desc = &edid->monitor_details.descriptor[i];
 		if (desc->zero_flag_1 != 0) {
 			decode_timing((u8 *)desc, timing);
-			timing_done = true;
+			if (mode_valid)
+				timing_done = mode_valid(mode_valid_priv,
+							 timing);
+			else
+				timing_done = true;
 			break;
 		}
 	}
@@ -225,6 +233,14 @@ int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
 	return 0;
 }
 
+int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
+		    int *panel_bits_per_colourp)
+{
+	return edid_get_timing_validate(buf, buf_size, timing,
+					panel_bits_per_colourp, NULL, NULL);
+}
+
+
 /**
  * Snip the tailing whitespace/return of a string.
  *
diff --git a/include/edid.h b/include/edid.h
index f05d2b82f2..2562733061 100644
--- a/include/edid.h
+++ b/include/edid.h
@@ -306,6 +306,28 @@ int edid_get_ranges(struct edid1_info *edid, unsigned int *hmin,
 
 struct display_timing;
 
+/**
+ * edid_get_timing_validate() - Get basic digital display parameters with
+ * mode selection callback
+ *
+ * @param buf		Buffer containing EDID data
+ * @param buf_size	Size of buffer in bytes
+ * @param timing	Place to put preferring timing information
+ * @param panel_bits_per_colourp	Place to put the number of bits per
+ *			colour supported by the panel. This will be set to
+ *			-1 if not available
+ * @param mode_valid	Callback validating mode, returning true is mode is
+ *			supported, false otherwise.
+ * @parem valid_priv	Pointer to private data for mode_valid callback
+ * @return 0 if timings are OK, -ve on error
+ */
+int edid_get_timing_validate(u8 *buf, int buf_size,
+			     struct display_timing *timing,
+			     int *panel_bits_per_colourp,
+			     bool (*mode_valid)(void *priv,
+					const struct display_timing *timing),
+			     void *mode_valid_priv);
+
 /**
  * edid_get_timing() - Get basic digital display parameters
  *
-- 
2.21.0

[U-Boot] [PATCH 1/3] edid: add edid_get_timing_validate() variant to filter out edid modes

[Neil Armstrong]

On 04/07/2019 15:52, Neil Armstrong wrote:
> The original edid_get_timing() function returns the first valid timing,
> but on some plaforms, we could only supports a subset of the listed
> monitot's navite timing.
> 
> Let's introduce a edid_get_timing_validate() adding a mode_valid callback
> including a private cookie pointer.
> 
> If the callback returns false, the current timing is discared and the next
> one is checked. If no valid & supported timings are found, the function
> would return an error.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  common/edid.c  | 22 +++++++++++++++++++---
>  include/edid.h | 22 ++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/common/edid.c b/common/edid.c
> index 90d1167f6e..f244d26e04 100644
> --- a/common/edid.c
> +++ b/common/edid.c
> @@ -168,8 +168,12 @@ static bool cea_is_hdmi_vsdb_present(struct edid_cea861_info *info)
>  	return false;
>  }
>  
> -int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
> -		    int *panel_bits_per_colourp)
> +int edid_get_timing_validate(u8 *buf, int buf_size,
> +			     struct display_timing *timing,
> +			     int *panel_bits_per_colourp,
> +			     bool (*mode_valid)(void *priv,
> +					const struct display_timing *timing),
> +			     void *mode_valid_priv)
>  {
>  	struct edid1_info *edid = (struct edid1_info *)buf;
>  	bool timing_done;
> @@ -193,7 +197,11 @@ int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
>  		desc = &edid->monitor_details.descriptor[i];
>  		if (desc->zero_flag_1 != 0) {
>  			decode_timing((u8 *)desc, timing);
> -			timing_done = true;
> +			if (mode_valid)
> +				timing_done = mode_valid(mode_valid_priv,
> +							 timing);
> +			else
> +				timing_done = true;
>  			break;

the break is wrong, it should be :

			if (timing_done)
				break;

I'll send a v2.

Neil

>  		}
>  	}
> @@ -225,6 +233,14 @@ int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
>  	return 0;
>  }
>  
> +int edid_get_timing(u8 *buf, int buf_size, struct display_timing *timing,
> +		    int *panel_bits_per_colourp)
> +{
> +	return edid_get_timing_validate(buf, buf_size, timing,
> +					panel_bits_per_colourp, NULL, NULL);
> +}
> +
> +
>  /**
>   * Snip the tailing whitespace/return of a string.
>   *
> diff --git a/include/edid.h b/include/edid.h
> index f05d2b82f2..2562733061 100644
> --- a/include/edid.h
> +++ b/include/edid.h
> @@ -306,6 +306,28 @@ int edid_get_ranges(struct edid1_info *edid, unsigned int *hmin,
>  
>  struct display_timing;
>  
> +/**
> + * edid_get_timing_validate() - Get basic digital display parameters with
> + * mode selection callback
> + *
> + * @param buf		Buffer containing EDID data
> + * @param buf_size	Size of buffer in bytes
> + * @param timing	Place to put preferring timing information
> + * @param panel_bits_per_colourp	Place to put the number of bits per
> + *			colour supported by the panel. This will be set to
> + *			-1 if not available
> + * @param mode_valid	Callback validating mode, returning true is mode is
> + *			supported, false otherwise.
> + * @parem valid_priv	Pointer to private data for mode_valid callback
> + * @return 0 if timings are OK, -ve on error
> + */
> +int edid_get_timing_validate(u8 *buf, int buf_size,
> +			     struct display_timing *timing,
> +			     int *panel_bits_per_colourp,
> +			     bool (*mode_valid)(void *priv,
> +					const struct display_timing *timing),
> +			     void *mode_valid_priv);
> +
>  /**
>   * edid_get_timing() - Get basic digital display parameters
>   *
> 

[U-Boot] [PATCH 2/3] video: display: use edid_get_timing_validate() variant to filter supported EDID modes

[Neil Armstrong]

Introduce a new display op, mode_valid() to be used with the newly
introduced edid_get_timing_validate() function, to filter supported
monitor timings if handled by the display driver.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/video/display-uclass.c | 15 ++++++++++++++-
 include/display.h              | 10 ++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/video/display-uclass.c b/drivers/video/display-uclass.c
index 99ef5e76f5..1a29ce5d85 100644
--- a/drivers/video/display-uclass.c
+++ b/drivers/video/display-uclass.c
@@ -37,6 +37,17 @@ int display_enable(struct udevice *dev, int panel_bpp,
 	return 0;
 }
 
+static bool display_mode_valid(void *priv, const struct display_timing *timing)
+{
+	struct udevice *dev = priv;
+	struct dm_display_ops *ops = display_get_ops(dev);
+
+	if (ops && ops->mode_valid)
+		return ops->mode_valid(dev, timing);
+
+	return true;
+}
+
 int display_read_timing(struct udevice *dev, struct display_timing *timing)
 {
 	struct dm_display_ops *ops = display_get_ops(dev);
@@ -53,7 +64,9 @@ int display_read_timing(struct udevice *dev, struct display_timing *timing)
 	if (ret < 0)
 		return ret;
 
-	return edid_get_timing(buf, ret, timing, &panel_bits_per_colour);
+	return edid_get_timing_validate(buf, ret, timing,
+					&panel_bits_per_colour,
+					display_mode_valid, dev);
 }
 
 bool display_in_use(struct udevice *dev)
diff --git a/include/display.h b/include/display.h
index 16f317c9c8..66294616ea 100644
--- a/include/display.h
+++ b/include/display.h
@@ -80,6 +80,16 @@ struct dm_display_ops {
 	 */
 	int (*enable)(struct udevice *dev, int panel_bpp,
 		      const struct display_timing *timing);
+
+	/**
+	 * mode_valid() - Check if mode is supported
+	 *
+	 * @dev:	Device to enable
+	 * @timing:	Display timings
+	 * @return true if supported, false if not
+	 */
+	bool (*mode_valid)(struct udevice *dev,
+			   const struct display_timing *timing);
 };
 
 #define display_get_ops(dev)	((struct dm_display_ops *)(dev)->driver->ops)
-- 
2.21.0

[U-Boot] [PATCH 3/3] video: meson: dw-hdmi: add EDID mode filtering to only select supported modes

[Neil Armstrong]

Add support for the new mode_valid() display op to filter out unsupported
display DMT timings.

This is useful when connected to 4k displays, since we only support DMT
monitors up to 1920x1080, the 4k native timings are discarded to select
supported timings.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/video/meson/meson_dw_hdmi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/video/meson/meson_dw_hdmi.c b/drivers/video/meson/meson_dw_hdmi.c
index 483c93f6b6..617f75724b 100644
--- a/drivers/video/meson/meson_dw_hdmi.c
+++ b/drivers/video/meson/meson_dw_hdmi.c
@@ -426,9 +426,16 @@ static int meson_dw_hdmi_probe(struct udevice *dev)
 	return ret;
 }
 
+static bool meson_dw_hdmi_mode_valid(struct udevice *dev,
+				     const struct display_timing *timing)
+{
+	return meson_venc_hdmi_supported_mode(timing);
+}
+
 static const struct dm_display_ops meson_dw_hdmi_ops = {
 	.read_edid = meson_dw_hdmi_read_edid,
 	.enable = meson_dw_hdmi_enable,
+	.mode_valid = meson_dw_hdmi_mode_valid,
 };
 
 static const struct udevice_id meson_dw_hdmi_ids[] = {
-- 
2.21.0

[U-Boot] [PATCH 0/3] video: add support for EDID timings validation

[Anatolij Gustschin]

On Thu,  4 Jul 2019 15:52:05 +0200
Neil Armstrong narmstrong at baylibre.com wrote:
...
>  common/edid.c                       | 22 +++++++++++++++++++---
>  drivers/video/display-uclass.c      | 15 ++++++++++++++-
>  drivers/video/meson/meson_dw_hdmi.c |  7 +++++++
>  include/display.h                   | 10 ++++++++++
>  include/edid.h                      | 22 ++++++++++++++++++++++
>  5 files changed, 72 insertions(+), 4 deletions(-)

Series applied to u-boot-video/master, thanks!

--
Anatolij