Re: [PATCH 2/4] drm/i915: Reject modes where hdisplay or vdisplay is too small

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 01, 2013 at 05:08:15PM -0300, Rodrigo Vivi wrote:
> On Tue, Oct 1, 2013 at 10:13 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Check the adjusted_mode hdisplay/vdisplay against the limits from BSpec.
> >
> > Move the ctg+ zero front porch check to the same function, and change it
> > to use the crtc_ values as well.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 44 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 38 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a695888..fa02677 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4197,11 +4197,46 @@ static void hsw_compute_ips_config(struct intel_crtc *crtc,
> >                                    pipe_config->pipe_bpp <= 24;
> >  }
> >
> > +static int intel_check_mode(struct drm_crtc *crtc,
> > +                           const struct drm_display_mode *mode)
> > +{
> > +       struct drm_device *dev = crtc->dev;
> > +
> > +       /*
> > +        * Cantiga+ cannot handle modes with a hsync front porch of 0.
> > +        * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
> > +        */
> > +       if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
> > +           mode->crtc_hsync_start == mode->crtc_hdisplay)
> > +               return -EINVAL;
> > +
> > +       if (IS_HASWELL(dev)) {
> > +               if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI) &&
> > +                   mode->crtc_hdisplay < 256)
> > +                       return -EINVAL;
> > +
> > +               if (mode->crtc_hdisplay < 64 || mode->crtc_vdisplay < 7)
> > +                       return -EINVAL;
> > +       } else if (INTEL_INFO(dev)->gen >= 5) {
> > +               if (mode->crtc_hdisplay < 64 || mode->crtc_vdisplay < 7)
> > +                       return -EINVAL;
> > +       } else if (INTEL_INFO(dev)->gen >= 4) {
> > +               if (mode->crtc_hdisplay < 3 || mode->crtc_vdisplay < 3)
> > +                       return -EINVAL;
> > +       } else {
> 
> I checked all specs and all values above are ok.
> 
> > +               if (mode->crtc_hdisplay < 2 || mode->crtc_vdisplay < 2)
> 
> Just couldn't understand why this 2 lines as minimun here.
> Even on 965_g35 they are 3.

965 is gen4. G35 (BLB) is gen3(ish). The internal bspec lists the minimum
size as 2 for both gen2 and gen3.

> 
> But maybe I'm missing something so, with this fixed or explained feel
> free to use:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> 
> > +                       return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >                                      struct intel_crtc_config *pipe_config)
> >  {
> >         struct drm_device *dev = crtc->base.dev;
> >         struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > +       int ret;
> >
> >         /* FIXME should check pixel clock limits on all platforms */
> >         if (INTEL_INFO(dev)->gen < 4) {
> > @@ -4236,12 +4271,9 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >              intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
> >                 pipe_config->pipe_src_w &= ~1;
> >
> > -       /* Cantiga+ cannot handle modes with a hsync front porch of 0.
> > -        * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
> > -        */
> > -       if ((INTEL_INFO(dev)->gen > 4 || IS_G4X(dev)) &&
> > -               adjusted_mode->hsync_start == adjusted_mode->hdisplay)
> > -               return -EINVAL;
> > +       ret = intel_check_mode(&crtc->base, adjusted_mode);
> > +       if (ret)
> > +               return ret;
> >
> >         if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) && pipe_config->pipe_bpp > 10*3) {
> >                 pipe_config->pipe_bpp = 10*3; /* 12bpc is gen5+ */
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC