From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 4/4] drm/i915: Do some basic sanity adjustments on the user provided mode Date: Tue, 1 Oct 2013 21:06:10 +0200 Message-ID: <20131001190610.GS26592@phenom.ffwll.local> References: <1380633211-16138-1-git-send-email-ville.syrjala@linux.intel.com> <1380633211-16138-5-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f53.google.com (mail-ee0-f53.google.com [74.125.83.53]) by gabe.freedesktop.org (Postfix) with ESMTP id DF11DE787E for ; Tue, 1 Oct 2013 12:05:52 -0700 (PDT) Received: by mail-ee0-f53.google.com with SMTP id b15so3653488eek.40 for ; Tue, 01 Oct 2013 12:05:51 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1380633211-16138-5-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Oct 01, 2013 at 04:13:31PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > Make sure the active, blanking and sync portions of the timings are in > the proper order. The contract with userspace basically is that we must > not increase the active portion. For the rest we're more or less free to > do as we please. A good rul IMO is that we only increase the non-active > portions of the timings. > = > We could do a lot more adjustment to make sure the timings meet all > the minimum requirements of the hardware. But as the mimimums may depend > on the output type and other details, we should perhaps do the adjustments > at some later point. > = > Signed-off-by: Ville Syrj=E4l=E4 Do we actually get such modes? I'd vote to just filter them from our ->get_modes callbacks and reject them here if at all possible ... -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++++++++++++++++++= ++++-- > 1 file changed, 58 insertions(+), 2 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 7f61cfb..48cad99 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8458,6 +8458,55 @@ static bool check_encoder_cloning(struct drm_crtc = *crtc) > return !(num_encoders > 1 && uncloneable_encoders); > } > = > +static void adjust_timings(struct drm_display_mode *mode, > + int *val, int min, int clocks_per_unit) > +{ > + if (*val >=3D min) > + return; > + > + /* maintain the same refresh rate */ > + mode->clock +=3D (min - *val) * clocks_per_unit; > + > + *val =3D min; > +} > + > +static int intel_adjust_mode(struct drm_display_mode *mode) > +{ > + if (mode->clock =3D=3D 0) { > + DRM_DEBUG_KMS("bad pixel clock\n"); > + drm_mode_debug_printmodeline(mode); > + return -EINVAL; > + } > + > + if (mode->hdisplay =3D=3D 0 || mode->vdisplay =3D=3D 0) { > + DRM_DEBUG_KMS("bad hdisplay or vdisplay\n"); > + drm_mode_debug_printmodeline(mode); > + return -EINVAL; > + } > + > + adjust_timings(mode, &mode->hsync_start, mode->hdisplay, 0); > + adjust_timings(mode, &mode->hsync_end, mode->hsync_start, 0); > + adjust_timings(mode, &mode->htotal, mode->hsync_end, mode->vtotal); > + > + adjust_timings(mode, &mode->vsync_start, mode->vdisplay, 0); > + adjust_timings(mode, &mode->vsync_end, mode->vsync_start, 0); > + > + /* > + * We must not increase the size of the active > + * space area for frame packing modes. > + */ > + if ((mode->flags & DRM_MODE_FLAG_3D_MASK) =3D=3D DRM_MODE_FLAG_3D_FRAME= _PACKING) { > + if (mode->vtotal < mode->vsync_end) { > + DRM_DEBUG_KMS("refusing to adjust vtotal for frame packing mode\n"); > + drm_mode_debug_printmodeline(mode); > + return -EINVAL; > + } > + } else > + adjust_timings(mode, &mode->vtotal, mode->vsync_end, mode->htotal); > + > + return 0; > +} > + > static struct intel_crtc_config * > intel_modeset_pipe_config(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > @@ -8466,7 +8515,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, > struct drm_device *dev =3D crtc->dev; > struct intel_encoder *encoder; > struct intel_crtc_config *pipe_config; > - int plane_bpp, ret =3D -EINVAL; > + int plane_bpp, ret; > bool retry =3D true; > = > if (!check_encoder_cloning(crtc)) { > @@ -8498,14 +8547,20 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, > (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC))) > pipe_config->adjusted_mode.flags |=3D DRM_MODE_FLAG_NVSYNC; > = > + ret =3D intel_adjust_mode(&pipe_config->adjusted_mode); > + if (ret) > + goto fail; > + > /* Compute a starting value for pipe_config->pipe_bpp taking the source > * plane pixel format and any sink constraints into account. Returns the > * source plane bpp so that dithering can be selected on mismatches > * after encoders and crtc also have had their say. */ > plane_bpp =3D compute_baseline_pipe_bpp(to_intel_crtc(crtc), > fb, pipe_config); > - if (plane_bpp < 0) > + if (plane_bpp < 0) { > + ret =3D -EINVAL; > goto fail; > + } > = > /* Determine the real pipe dimensions */ > drm_mode_set_crtcinfo(&pipe_config->adjusted_mode, CRTC_STEREO_DOUBLE); > @@ -8532,6 +8587,7 @@ encoder_retry: > = > if (!(encoder->compute_config(encoder, pipe_config))) { > DRM_DEBUG_KMS("Encoder config failure\n"); > + ret =3D -EINVAL; > goto fail; > } > } > -- = > 1.8.1.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch