linux-kernel - Re: [v2] drm/sun4i: add lvds mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Apr 2018 15:34:21 +0200
From:   Ondřej Jirman <doudahwezomiechahtah@....cz>
To:     Giulio Benetti <giulio.benetti@...ronovasrl.com>
Cc:     Maxime Ripard <maxime.ripard@...e-electrons.com>,
        David Airlie <airlied@...ux.ie>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, Chen-Yu Tsai <wens@...e.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [v2] drm/sun4i: add lvds mode_valid function

Hello Giulio,

this patch breaks LVDS output on A83T. Without it, modesetting works,
with it there's no output.

Some more info below...

On Tue, Mar 13, 2018 at 12:20:19PM +0100, Giulio Benetti wrote:
> mode_valid function is missing for lvds.
> 
> Add it making it pointed by encoder helper functions.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@...ronovasrl.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_lvds.c | 55 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> index be3f14d..bffff4c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> @@ -94,9 +94,64 @@ static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder)
>  	}
>  }
>  
> +static enum drm_mode_status sun4i_lvds_encoder_mode_valid(struct drm_encoder *crtc,
> +							  const struct drm_display_mode *mode)
> +{
> +	struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(crtc);
> +	struct sun4i_tcon *tcon = lvds->tcon;
> +	u32 hsync = mode->hsync_end - mode->hsync_start;
> +	u32 vsync = mode->vsync_end - mode->vsync_start;
> +	unsigned long rate = mode->clock * 1000;
> +	long rounded_rate;
> +
> +	DRM_DEBUG_DRIVER("Validating modes...\n");
> +
> +	if (hsync < 1)
> +		return MODE_HSYNC_NARROW;
> +
> +	if (hsync > 0x3ff)
> +		return MODE_HSYNC_WIDE;
> +
> +	if ((mode->hdisplay < 1) || (mode->htotal < 1))
> +		return MODE_H_ILLEGAL;
> +
> +	if ((mode->hdisplay > 0x7ff) || (mode->htotal > 0xfff))
> +		return MODE_BAD_HVALUE;
> +
> +	DRM_DEBUG_DRIVER("Horizontal parameters OK\n");
> +
> +	if (vsync < 1)
> +		return MODE_VSYNC_NARROW;
> +
> +	if (vsync > 0x3ff)
> +		return MODE_VSYNC_WIDE;
> +
> +	if ((mode->vdisplay < 1) || (mode->vtotal < 1))
> +		return MODE_V_ILLEGAL;
> +
> +	if ((mode->vdisplay > 0x7ff) || (mode->vtotal > 0xfff))
> +		return MODE_BAD_VVALUE;
> +
> +	DRM_DEBUG_DRIVER("Vertical parameters OK\n");
> +
> +	tcon->dclk_min_div = 7;
> +	tcon->dclk_max_div = 7;

Why would validation function change any state anywhere?

> +	rounded_rate = clk_round_rate(tcon->dclk, rate);

The issue is here, on A83T TBS A711 tablet, I get...

sun4i-tcon 1c0c000.lcd-controller: XXX: hsync=20 hdisplay=1024 htotal=1384
  vsync=5 vdisplay=600 vtotal=640 rate=52000000 rounded_rate=51857142

> +	if (rounded_rate < rate)
> +		return MODE_CLOCK_LOW;
> +
> +	if (rounded_rate > rate)
> +		return MODE_CLOCK_HIGH;

... while the previous conditions require an exact match for some reason.

But HW works fine without an exact rate match. Why is exact match required here?

thank you,
  Ondrej

> +	DRM_DEBUG_DRIVER("Clock rate OK\n");
> +
> +	return MODE_OK;
> +}
> +
>  static const struct drm_encoder_helper_funcs sun4i_lvds_enc_helper_funcs = {
>  	.disable	= sun4i_lvds_encoder_disable,
>  	.enable		= sun4i_lvds_encoder_enable,
> +	.mode_valid	= sun4i_lvds_encoder_mode_valid,
>  };
>  
>  static const struct drm_encoder_funcs sun4i_lvds_enc_funcs = {