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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170710211833.bprcrq5tfpzxtpeg@art_vandelay>
Date:   Mon, 10 Jul 2017 17:18:33 -0400
From:   Sean Paul <seanpaul@...omium.org>
To:     John Stultz <john.stultz@...aro.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Sean Paul <seanpaul@...omium.org>,
        David Airlie <airlied@...ux.ie>,
        Rob Clark <robdclark@...il.com>,
        Xinliang Liu <xinliang.liu@...aro.org>,
        Xinliang Liu <z.liuxinliang@...ilicon.com>,
        Rongrong Zou <zourongrong@...il.com>,
        Xinwei Kong <kong.kongxinwei@...ilicon.com>,
        Chen Feng <puck.chen@...ilicon.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>,
        Archit Taneja <architt@...eaurora.org>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [RFC][PATCH] drm: kirin: Restrict modes to known good mode clocks

On Mon, Jul 10, 2017 at 01:48:02PM -0700, John Stultz wrote:
> Currently the hikey dsi logic cannot generate accurate byte
> clocks values for all pixel clock values. Thus if a mode clock
> is selected that cannot match the calculated byte clock, the
> device will boot with a blank screen.
> 
> This patch uses the new mode_valid callback (many thanks to
> Jose Abreu for upstreaming it!) to enforces known good mode
> clocks for well known resolutions, which should allow the
> display to work from given EDID options, and ensures for a given
> resolution & refresh, the right mode clock is selected.
> 
> Cc: Daniel Vetter <daniel.vetter@...el.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Sean Paul <seanpaul@...omium.org>
> Cc: David Airlie <airlied@...ux.ie>
> Cc: Rob Clark <robdclark@...il.com>
> Cc: Xinliang Liu <xinliang.liu@...aro.org>
> Cc: Xinliang Liu <z.liuxinliang@...ilicon.com>
> Cc: Rongrong Zou <zourongrong@...il.com>
> Cc: Xinwei Kong <kong.kongxinwei@...ilicon.com>
> Cc: Chen Feng <puck.chen@...ilicon.com>
> Cc: Jose Abreu <Jose.Abreu@...opsys.com>
> Cc: Archit Taneja <architt@...eaurora.org>
> Cc: dri-devel@...ts.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 37 ++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> index f77dcfa..a84f4bb 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> @@ -603,6 +603,42 @@ static void dsi_encoder_enable(struct drm_encoder *encoder)
>  	dsi->enable = true;
>  }
>  
> +static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *crtc,
> +					const struct drm_display_mode *mode)
> +{
> +	/*
> +	 * kirin cannot generate all modes, so use the whitelist below
> +	 */
> +	DRM_DEBUG("%s: Checking mode %ix%i@%i clock: %i...", __func__,
> +		mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode), mode->clock);
> +	if ((mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 148500) ||
> +	    (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 80192)  ||
> +	    (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 74250)  ||
> +	    (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock == 61855)  ||
> +	    (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 147116) ||
> +	    (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 146250) ||
> +	    (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock == 144589) ||
> +	    (mode->hdisplay == 1600 && mode->vdisplay == 1200 && mode->clock == 160961) ||
> +	    (mode->hdisplay == 1600 && mode->vdisplay == 900  && mode->clock == 118963) ||
> +	    (mode->hdisplay == 1440 && mode->vdisplay == 900  && mode->clock == 126991) ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock == 128946) ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock == 98619)  ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 960  && mode->clock == 102081) ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 800  && mode->clock == 83496)  ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 720  && mode->clock == 74440)  ||
> +	    (mode->hdisplay == 1280 && mode->vdisplay == 720  && mode->clock == 74250)  ||
> +	    (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 78800)  ||
> +	    (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 75000)  ||
> +	    (mode->hdisplay == 1024 && mode->vdisplay == 768  && mode->clock == 81833)  ||
> +	    (mode->hdisplay == 800  && mode->vdisplay == 600  && mode->clock == 48907)  ||
> +	    (mode->hdisplay == 800  && mode->vdisplay == 600  && mode->clock == 40000)) {

Bikeshed incoming:

Can you break this out into a lookup table? It's kind of long-winded as-is. It'd
be even better if you could calculate whether the mode is valid, but I didn't
spend enough time to figure out if this is possible.

Sean


> +		DRM_DEBUG("OK\n");
> +		return MODE_OK;
> +	}
> +	DRM_DEBUG("BAD\n");
> +	return MODE_BAD;
> +}
> +
>  static void dsi_encoder_mode_set(struct drm_encoder *encoder,
>  				 struct drm_display_mode *mode,
>  				 struct drm_display_mode *adj_mode)
> @@ -622,6 +658,7 @@ static int dsi_encoder_atomic_check(struct drm_encoder *encoder,
>  
>  static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = {
>  	.atomic_check	= dsi_encoder_atomic_check,
> +	.mode_valid	= dsi_encoder_mode_valid,
>  	.mode_set	= dsi_encoder_mode_set,
>  	.enable		= dsi_encoder_enable,
>  	.disable	= dsi_encoder_disable
> -- 
> 2.7.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ