On Thu, 9 May 2013 01:25:59 +0300 Ville Syrj?l? <ville.syrjala at linux.intel.com> wrote: > On Wed, May 08, 2013 at 02:01:25PM -0700, Jesse Barnes wrote: > > Requested-by: Ville Syrj?l? <ville.syrjala at linux.intel.com> > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org> > > --- > > drivers/gpu/drm/drm_crtc.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 792c3e3..72ae33a 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -1318,6 +1318,18 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out, > > if (in->clock > INT_MAX || in->vrefresh > INT_MAX) > > return -ERANGE; > > > > + /* Reject modes with invalid h/vsync */ > > + if (!(in->flags & (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC))) > > + return -EINVAL; > > + if ((in->flags & DRM_MODE_FLAG_PHSYNC) && > > + (in->flags & DRM_MODE_FLAG_NHSYNC)) > > + return -EINVAL; > > + if (!(in->flags & (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC))) > > + return -EINVAL; > > + if ((in->flags & DRM_MODE_FLAG_PVSYNC) && > > + (in->flags & DRM_MODE_FLAG_NVSYNC)) > > + return -EINVAL; > > That might be a bit too drastic. Well I suppose making sure that both > flags are not enabled at the same time could be OK. But having neither > flag set could be perfectly legal (the user could be asking for composite > sync instead for example). > > > So my less drastic suggestion would be doing something like this in i915 > specific code: > > adjusted_mode->flags = 0; > if (requested_mode->flags & NHSYNC) > adjusted_mode->flags |= NHSYNC; > else > adjusted_mode->flags |= PHSYNC; > > It would gurantee that we end up picking exactly one of the flags in > every case. If both are set, we pick -, of neither is set we pick +. Ah yeah I knew I was forgetting something... I'll drop the checks for no flags. You really think it would be better to do this in i915? I guess it's probably safe, but it seems nicer to filter this out where it might occur (the EDID quirks should already deal with bogus flags for kernel generated mode lists). -- Jesse Barnes, Intel Open Source Technology Center
