From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 4/4] drm/i915: Do some basic sanity adjustments on the user provided mode Date: Tue, 1 Oct 2013 22:12:35 +0300 Message-ID: <20131001191235.GR9395@intel.com> References: <1380633211-16138-1-git-send-email-ville.syrjala@linux.intel.com> <1380633211-16138-5-git-send-email-ville.syrjala@linux.intel.com> <20131001190610.GS26592@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id A65CCE788F for ; Tue, 1 Oct 2013 12:12:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20131001190610.GS26592@phenom.ffwll.local> 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: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Oct 01, 2013 at 09:06:10PM +0200, Daniel Vetter wrote: > On Tue, Oct 01, 2013 at 04:13:31PM +0300, ville.syrjala@linux.intel.com w= rote: > > 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 adjustme= nts > > 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 ... The user is free to feed us any kind of garbage. Doesn't matter which modes we list in the connector's mode list. > -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/i91= 5/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_crt= c *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_FRA= ME_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 sour= ce > > * 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 -- = Ville Syrj=E4l=E4 Intel OTC