diff mbox series

[PROTO,05/10] drm/bridge: adv7511: Add max-clock, min-vrefresh options

Message ID 1534254604-24204-6-git-send-email-uli+renesas@fpond.eu (mailing list archive)
State RFC
Delegated to: Simon Horman
Headers show
Series R-Car D3 LVDS/HDMI support (with PLL) | expand

Commit Message

Ulrich Hecht Aug. 14, 2018, 1:49 p.m. UTC 
From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

This patch adds the option to specify a maximal clock and a minimal vertical
refresh rate.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
[uli: renamed properties, fixed transposed parsing]
Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     |  7 +++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Laurent Pinchart Aug. 20, 2018, 9:28 a.m. UTC | #1 
Hi Ulrich,

Thank you for the patch.

On Tuesday, 14 August 2018 16:49:59 EEST Ulrich Hecht wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> This patch adds the option to specify a maximal clock and a minimal vertical
> refresh rate.

What is this needed for ?

> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> [uli: renamed properties, fixed transposed parsing]
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  7 +++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 22 ++++++++++++++++++++++

You're missing updates to the DT bindings.

>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 73d8ccb..7f29d4f 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -271,6 +271,8 @@ enum adv7511_sync_polarity {
>   * @sync_pulse:			Select the sync pulse
>   * @vsync_polarity:		vsync input signal configuration
>   * @hsync_polarity:		hsync input signal configuration
> + * @min_vrefresh_option:	min vrefresh option
> + * @max_freq_option:		max frequency option
>   */
>  struct adv7511_link_config {
>  	unsigned int input_color_depth;
> @@ -285,6 +287,9 @@ struct adv7511_link_config {
>  	enum adv7511_input_sync_pulse sync_pulse;
>  	enum adv7511_sync_polarity vsync_polarity;
>  	enum adv7511_sync_polarity hsync_polarity;
> +
> +	unsigned int min_vrefresh_option;
> +	unsigned int max_freq_option;
>  };
> 
>  /**
> @@ -354,6 +359,8 @@ struct adv7511 {
>  	enum adv7511_sync_polarity vsync_polarity;
>  	enum adv7511_sync_polarity hsync_polarity;
>  	bool rgb;
> +	unsigned int min_vref;
> +	unsigned int max_freq;
> 
>  	struct gpio_desc *gpio_pd;
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 6437b87..2938b02
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -323,6 +323,8 @@ static void adv7511_set_link_config(struct adv7511
> *adv7511, adv7511->hsync_polarity = config->hsync_polarity;
>  	adv7511->vsync_polarity = config->vsync_polarity;
>  	adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
> +	adv7511->min_vref = config->min_vrefresh_option;
> +	adv7511->max_freq = config->max_freq_option;
>  }
> 
>  static void __adv7511_power_on(struct adv7511 *adv7511)
> @@ -660,6 +662,16 @@ static enum drm_mode_status adv7511_mode_valid(struct
> adv7511 *adv7511, if (mode->clock > 165000)
>  		return MODE_CLOCK_HIGH;
> 
> +	if (adv7511->max_freq) {
> +		if (mode->clock > (adv7511->max_freq / 1000))
> +			return MODE_CLOCK_HIGH;
> +	}
> +
> +	if (adv7511->min_vref) {
> +		if (drm_mode_vrefresh(mode) < adv7511->min_vref)
> +			return MODE_BAD;
> +	}
> +
>  	return MODE_OK;
>  }
> 
> @@ -1074,6 +1086,16 @@ static int adv7511_parse_dt(struct device_node *np,
>  	config->vsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH;
>  	config->hsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH;
> 
> +	ret = of_property_read_u32(np, "max-clock",
> +				   &config->max_freq_option);
> +	if (ret < 0)
> +		config->max_freq_option = 0;
> +
> +	ret = of_property_read_u32(np, "min-vrefresh",
> +				   &config->min_vrefresh_option);
> +	if (ret < 0)
> +		config->min_vrefresh_option = 0;
> +
>  	return 0;
>  }
Ulrich Hecht Aug. 21, 2018, 8:03 a.m. UTC | #2 
> On August 20, 2018 at 11:28 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> 
> 
> Hi Ulrich,
> 
> Thank you for the patch.
> 
> On Tuesday, 14 August 2018 16:49:59 EEST Ulrich Hecht wrote:
> > From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > 
> > This patch adds the option to specify a maximal clock and a minimal vertical
> > refresh rate.
> 
> What is this needed for ?

Somewhere in the chain there is a component that will not tolerate clocks in excess of 135 MHz; if you don't limit it, the default for a 1920x1200 display is somewhere along 148 MHz, and the HDMI signal output is invalid.

CU
Uli
Laurent Pinchart Aug. 21, 2018, 8:09 a.m. UTC | #3 
Hi Ulrich,

On Tuesday, 21 August 2018 11:03:45 EEST Ulrich Hecht wrote:
> On August 20, 2018 at 11:28 AM Laurent Pinchart wrote:
> > On Tuesday, 14 August 2018 16:49:59 EEST Ulrich Hecht wrote:
> >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >> 
> >> This patch adds the option to specify a maximal clock and a minimal
> >> vertical refresh rate.
> > 
> > What is this needed for ?
> 
> Somewhere in the chain there is a component that will not tolerate clocks in
> excess of 135 MHz; if you don't limit it, the default for a 1920x1200
> display is somewhere along 148 MHz, and the HDMI signal output is invalid.

Do you know which component that is ?
Ulrich Hecht Aug. 22, 2018, 9:13 a.m. UTC | #4 
> On August 21, 2018 at 10:09 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> 
> 
> Hi Ulrich,
> 
> On Tuesday, 21 August 2018 11:03:45 EEST Ulrich Hecht wrote:
> > On August 20, 2018 at 11:28 AM Laurent Pinchart wrote:
> > > On Tuesday, 14 August 2018 16:49:59 EEST Ulrich Hecht wrote:
> > >> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> > >> 
> > >> This patch adds the option to specify a maximal clock and a minimal
> > >> vertical refresh rate.
> > > 
> > > What is this needed for ?
> > 
> > Somewhere in the chain there is a component that will not tolerate clocks in
> > excess of 135 MHz; if you don't limit it, the default for a 1920x1200
> > display is somewhere along 148 MHz, and the HDMI signal output is invalid.
> 
> Do you know which component that is ?

Unfortunately not. It's not the ADV7511, though, that goes up to 225 MHz...

CU
Uli
Laurent Pinchart Aug. 22, 2018, 2 p.m. UTC | #5 
Hi Ulrich,

On Wednesday, 22 August 2018 12:13:59 EEST Ulrich Hecht wrote:
> On August 21, 2018 at 10:09 AM Laurent Pinchart wrote:
> > On Tuesday, 21 August 2018 11:03:45 EEST Ulrich Hecht wrote:
> >> On August 20, 2018 at 11:28 AM Laurent Pinchart wrote:
> >>> On Tuesday, 14 August 2018 16:49:59 EEST Ulrich Hecht wrote:
> >>>> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> >>>> 
> >>>> This patch adds the option to specify a maximal clock and a minimal
> >>>> vertical refresh rate.
> >>> 
> >>> What is this needed for ?
> >> 
> >> Somewhere in the chain there is a component that will not tolerate
> >> clocks in excess of 135 MHz; if you don't limit it, the default for a
> >> 1920x1200 display is somewhere along 148 MHz, and the HDMI signal
> >> output is invalid.
> > 
> > Do you know which component that is ?
> 
> Unfortunately not. It's not the ADV7511, though, that goes up to 225 MHz...

After investigation, it turns out that the THC63LVD1024 is the culprit, its 
datasheet reports a maximum operating frequency of 135 MHz.

Do you have any idea what the minimal refresh rate is for ?
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 73d8ccb..7f29d4f 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -271,6 +271,8 @@  enum adv7511_sync_polarity {
  * @sync_pulse:			Select the sync pulse
  * @vsync_polarity:		vsync input signal configuration
  * @hsync_polarity:		hsync input signal configuration
+ * @min_vrefresh_option:	min vrefresh option
+ * @max_freq_option:		max frequency option
  */
 struct adv7511_link_config {
 	unsigned int input_color_depth;
@@ -285,6 +287,9 @@  struct adv7511_link_config {
 	enum adv7511_input_sync_pulse sync_pulse;
 	enum adv7511_sync_polarity vsync_polarity;
 	enum adv7511_sync_polarity hsync_polarity;
+
+	unsigned int min_vrefresh_option;
+	unsigned int max_freq_option;
 };
 
 /**
@@ -354,6 +359,8 @@  struct adv7511 {
 	enum adv7511_sync_polarity vsync_polarity;
 	enum adv7511_sync_polarity hsync_polarity;
 	bool rgb;
+	unsigned int min_vref;
+	unsigned int max_freq;
 
 	struct gpio_desc *gpio_pd;
 
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6437b87..2938b02 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -323,6 +323,8 @@  static void adv7511_set_link_config(struct adv7511 *adv7511,
 	adv7511->hsync_polarity = config->hsync_polarity;
 	adv7511->vsync_polarity = config->vsync_polarity;
 	adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB;
+	adv7511->min_vref = config->min_vrefresh_option;
+	adv7511->max_freq = config->max_freq_option;
 }
 
 static void __adv7511_power_on(struct adv7511 *adv7511)
@@ -660,6 +662,16 @@  static enum drm_mode_status adv7511_mode_valid(struct adv7511 *adv7511,
 	if (mode->clock > 165000)
 		return MODE_CLOCK_HIGH;
 
+	if (adv7511->max_freq) {
+		if (mode->clock > (adv7511->max_freq / 1000))
+			return MODE_CLOCK_HIGH;
+	}
+
+	if (adv7511->min_vref) {
+		if (drm_mode_vrefresh(mode) < adv7511->min_vref)
+			return MODE_BAD;
+	}
+
 	return MODE_OK;
 }
 
@@ -1074,6 +1086,16 @@  static int adv7511_parse_dt(struct device_node *np,
 	config->vsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH;
 	config->hsync_polarity = ADV7511_SYNC_POLARITY_PASSTHROUGH;
 
+	ret = of_property_read_u32(np, "max-clock",
+				   &config->max_freq_option);
+	if (ret < 0)
+		config->max_freq_option = 0;
+
+	ret = of_property_read_u32(np, "min-vrefresh",
+				   &config->min_vrefresh_option);
+	if (ret < 0)
+		config->min_vrefresh_option = 0;
+
 	return 0;
 }