diff mbox

[PATCHv3] drm/sun4i: validate modes for HDMI

Message ID 162854cb-c7bd-d9ce-9fa0-9a6cd89c621b@xs4all.nl (mailing list archive)
State New, archived
Headers show

Commit Message

Hans Verkuil Dec. 15, 2017, 3:21 p.m. UTC
When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some
digging found that there is no check against the upper pixelclock limit of the HDMI
output, so X selects a 4kp60 format at 594 MHz, which obviously won't work.

The patch below adds a check for the upper bound of what this hardware can do, and
it checks if the requested tmds clock can be obtained.

It also allows for the +/- 0.5% pixel clock variation that the HDMI spec permits.

That code is based on commit 22d0be2a557e53a22feb484e8fce255fe09e6ad5 from
Jose Abreu for drm/arc.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Thanks-to: Jose Abreu <Jose.Abreu@synopsys.com>
---
Changes since v2:
- Allow for the 0.5% variation.
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Daniel Vetter Dec. 15, 2017, 4:01 p.m. UTC | #1
On Fri, Dec 15, 2017 at 04:21:50PM +0100, Hans Verkuil wrote:
> When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some
> digging found that there is no check against the upper pixelclock limit of the HDMI
> output, so X selects a 4kp60 format at 594 MHz, which obviously won't work.
> 
> The patch below adds a check for the upper bound of what this hardware can do, and
> it checks if the requested tmds clock can be obtained.
> 
> It also allows for the +/- 0.5% pixel clock variation that the HDMI spec permits.
> 
> That code is based on commit 22d0be2a557e53a22feb484e8fce255fe09e6ad5 from
> Jose Abreu for drm/arc.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Thanks-to: Jose Abreu <Jose.Abreu@synopsys.com>
> ---
> Changes since v2:
> - Allow for the 0.5% variation.
> ---
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> index dda904ec0534..a7e9eb93c378 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
> 
> +static int sun4i_hdmi_mode_valid(struct drm_connector *connector,
> +				 struct drm_display_mode *mode)
> +{

This only catches bad modes in the probe code, userspace can still request
bad modes directly through atomic or SETCRTC ioctls.

Please read the kerneldoc for drm_connector_helper_funcs->mode_valid and
put your shiny mode_valid callback into the right vtable.

Cheers, Daniel

> +	struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
> +	unsigned long rate = mode->clock * 1000;
> +	long diff = rate / 200; /* +-0.5% allowed by HDMI spec */
> +	long rounded_rate;
> +
> +	/* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */
> +	if (rate > 165000000)
> +		return MODE_CLOCK_HIGH;
> +	rounded_rate = clk_round_rate(hdmi->tmds_clk, rate);
> +	if (max(rounded_rate, rate) - min(rounded_rate, rate) < diff &&
> +	    rounded_rate > 0)
> +		return MODE_OK;
> +	return MODE_NOCLOCK;
> +}
> +
>  static const struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
>  	.get_modes	= sun4i_hdmi_get_modes,
> +	.mode_valid	= sun4i_hdmi_mode_valid,
>  };
> 
>  static enum drm_connector_status
> -- 
> 2.15.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Maxime Ripard Dec. 15, 2017, 9:25 p.m. UTC | #2
Hi,

On Fri, Dec 15, 2017 at 04:21:50PM +0100, Hans Verkuil wrote:
> When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some
> digging found that there is no check against the upper pixelclock limit of the HDMI
> output, so X selects a 4kp60 format at 594 MHz, which obviously won't work.
> 
> The patch below adds a check for the upper bound of what this hardware can do, and
> it checks if the requested tmds clock can be obtained.
> 
> It also allows for the +/- 0.5% pixel clock variation that the HDMI spec permits.
> 
> That code is based on commit 22d0be2a557e53a22feb484e8fce255fe09e6ad5 from
> Jose Abreu for drm/arc.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Thanks-to: Jose Abreu <Jose.Abreu@synopsys.com>

I've fixed the checkpatch warnings, and the compilation warning about
the type mismatch (between long and unsigned long), and
applied. Thanks!

Maxime
diff mbox

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index dda904ec0534..a7e9eb93c378 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -208,8 +208,27 @@  static int sun4i_hdmi_get_modes(struct drm_connector *connector)
 	return ret;
 }

+static int sun4i_hdmi_mode_valid(struct drm_connector *connector,
+				 struct drm_display_mode *mode)
+{
+	struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
+	unsigned long rate = mode->clock * 1000;
+	long diff = rate / 200; /* +-0.5% allowed by HDMI spec */
+	long rounded_rate;
+
+	/* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */
+	if (rate > 165000000)
+		return MODE_CLOCK_HIGH;
+	rounded_rate = clk_round_rate(hdmi->tmds_clk, rate);
+	if (max(rounded_rate, rate) - min(rounded_rate, rate) < diff &&
+	    rounded_rate > 0)
+		return MODE_OK;
+	return MODE_NOCLOCK;
+}
+
 static const struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
 	.get_modes	= sun4i_hdmi_get_modes,
+	.mode_valid	= sun4i_hdmi_mode_valid,
 };

 static enum drm_connector_status