lkml.org 
[lkml]   [2017]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Xinliang Liu <xinliang.liu@linaro.org>
> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> Cc: Rongrong Zou <zourongrong@gmail.com>
> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> Cc: Chen Feng <puck.chen@hisilicon.com>
> Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.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

\
 
 \ /
  Last update: 2017-07-10 23:18    [W:0.085 / U:1.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site