On Fri, Aug 24, 2018 at 07:56:34PM +0000, Souza, Jose wrote:
> On Thu, 2018-07-19 at 21:22 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > 
> > Move the skl+ specific framebuffer related checks from
> > intel_plane_atomic_check_with_state() into a new function
> > (skl_plane_check_fb()) which we'll simply call from the skl
> > plane->check() hook.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c | 42 --------------------
> >  drivers/gpu/drm/i915/intel_display.c      | 12 ------
> >  drivers/gpu/drm/i915/intel_sprite.c       | 66
> > +++++++++++++++++++++++++++++++
> >  3 files changed, 66 insertions(+), 54 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index eddcdd6e4b3b..7c0873060934 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -116,40 +116,11 @@ int intel_plane_atomic_check_with_state(const
> > struct intel_crtc_state *old_crtc_
> >     struct drm_i915_private *dev_priv = to_i915(plane->dev);
> >     struct drm_plane_state *state = &intel_state->base;
> >     struct intel_plane *intel_plane = to_intel_plane(plane);
> > -   const struct drm_display_mode *adjusted_mode =
> > -           &crtc_state->base.adjusted_mode;
> >     int ret;
> >  
> >     if (!intel_state->base.crtc && !old_plane_state->base.crtc)
> >             return 0;
> >  
> > -   if (state->fb && drm_rotation_90_or_270(state->rotation)) {
> > -           struct drm_format_name_buf format_name;
> > -
> > -           if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> > -               state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> > -                   DRM_DEBUG_KMS("Y/Yf tiling required for
> > 90/270!\n");
> > -                   return -EINVAL;
> > -           }
> > -
> > -           /*
> > -            * 90/270 is not allowed with RGB64 16:16:16:16,
> > -            * RGB 16-bit 5:6:5, and Indexed 8-bit.
> > -            * TBD: Add RGB64 case once its added in supported
> > format list.
> > -            */
> > -           switch (state->fb->format->format) {
> > -           case DRM_FORMAT_C8:
> > -           case DRM_FORMAT_RGB565:
> > -                   DRM_DEBUG_KMS("Unsupported pixel format %s for
> > 90/270!\n",
> > -                                 drm_get_format_name(state->fb-
> > >format->format,
> > -                                                     &format_name)
> > );
> > -                   return -EINVAL;
> > -
> > -           default:
> > -                   break;
> > -           }
> > -   }
> > -
> >     /* CHV ignores the mirror bit when the rotate bit is set :( */
> >     if (IS_CHERRYVIEW(dev_priv) &&
> >         state->rotation & DRM_MODE_ROTATE_180 &&
> > @@ -163,19 +134,6 @@ int intel_plane_atomic_check_with_state(const
> > struct intel_crtc_state *old_crtc_
> >     if (ret)
> >             return ret;
> >  
> > -   /*
> > -    * Y-tiling is not supported in IF-ID Interlace mode in
> > -    * GEN9 and above.
> > -    */
> > -   if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state-
> > >base.enable &&
> > -       adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> > -           if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> > -               state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> > -                   DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-
> > ID mode\n");
> > -                   return -EINVAL;
> > -           }
> > -   }
> > -
> >     /* FIXME pre-g4x don't work like this */
> >     if (state->visible)
> >             crtc_state->active_planes |= BIT(intel_plane->id);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 9b9eaeda15dd..2381b762d109 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3151,12 +3151,6 @@ static int skl_check_ccs_aux_surface(struct
> > intel_plane_state *plane_state)
> >     int y = src_y / vsub;
> >     u32 offset;
> >  
> > -   if (plane_state->base.rotation & ~(DRM_MODE_ROTATE_0 |
> > DRM_MODE_ROTATE_180)) {
> > -           DRM_DEBUG_KMS("RC support only with 0/180 degree
> > rotation %x\n",
> > -                         plane_state->base.rotation);
> > -           return -EINVAL;
> > -   }
> > -
> >     intel_add_fb_offsets(&x, &y, plane_state, 1);
> >     offset = intel_plane_compute_aligned_offset(&x, &y,
> > plane_state, 1);
> >  
> > @@ -3178,12 +3172,6 @@ int skl_check_plane_surface(const struct
> > intel_crtc_state *crtc_state,
> >     plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> > rotation);
> >     plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> > rotation);
> >  
> > -   if (rotation & DRM_MODE_REFLECT_X &&
> > -       fb->modifier == DRM_FORMAT_MOD_LINEAR) {
> > -           DRM_DEBUG_KMS("horizontal flip is not supported with
> > linear surface formats\n");
> > -           return -EINVAL;
> > -   }
> > -
> >     if (!plane_state->base.visible)
> >             return 0;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 36e150885897..041b8921f4fe 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1156,6 +1156,68 @@ vlv_sprite_check(struct intel_crtc_state
> > *crtc_state,
> >     return 0;
> >  }
> >  
> > +static int skl_plane_check_fb(const struct intel_crtc_state
> > *crtc_state,
> > +                         const struct intel_plane_state
> > *plane_state)
> > +{
> > +   const struct drm_framebuffer *fb = plane_state->base.fb;
> > +   unsigned int rotation = plane_state->base.rotation;
> > +   struct drm_format_name_buf format_name;
> > +
> > +   if (!fb)
> > +           return 0;
> > +
> > +   if (rotation & ~(DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180) &&
> > +       (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS &&
> > +        fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS)) {
> > +           DRM_DEBUG_KMS("RC support only with 0/180 degree
> > rotation (%x)\n",
> > +                         rotation);
> > +           return -EINVAL;
> > +   }
> > +
> > +   if (rotation & DRM_MODE_REFLECT_X &&
> > +       fb->modifier == DRM_FORMAT_MOD_LINEAR) {
> > +           DRM_DEBUG_KMS("horizontal flip is not supported with
> > linear surface formats\n");
> > +           return -EINVAL;
> > +   }
> > +
> > +   if (drm_rotation_90_or_270(rotation)) {
> > +           if (fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> > +               fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> > +                   DRM_DEBUG_KMS("Y/Yf tiling required for
> > 90/270!\n");
> > +                   return -EINVAL;
> > +           }
> > +
> > +           /*
> > +            * 90/270 is not allowed with RGB64 16:16:16:16,
> > +            * RGB 16-bit 5:6:5, and Indexed 8-bit.
> > +            * TBD: Add RGB64 case once its added in supported
> > format list.
> > +            */
> > +           switch (fb->format->format) {
> > +           case DRM_FORMAT_C8:
> > +           case DRM_FORMAT_RGB565:
> > +                   DRM_DEBUG_KMS("Unsupported pixel format %s for
> > 90/270!\n",
> > +                                 drm_get_format_name(fb->format-
> > >format,
> > +                                                     &format_name)
> > );
> > +                   return -EINVAL;
> > +           default:
> > +                   break;
> > +           }
> > +   }
> > +
> > +   /* Y-tiling is not supported in IF-ID Interlace mode */
> > +   if (crtc_state->base.enable &&
> > +       crtc_state->base.adjusted_mode.flags &
> > DRM_MODE_FLAG_INTERLACE &&
> > +       (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> > +        fb->modifier == I915_FORMAT_MOD_Yf_TILED ||
> 
> I guess this 2 checks bellow are new? It was not being handled in
> skl_check_ccs_aux_surface() or else where.

Right. The old code missed the compressed Y tiled cases. Should have
really split that change into a separate patch.

> 
> Other than this everything else looks good.
> 
> Reviewed-by: José Roberto de Souza <jose.so...@intel.com>
> 
> > +        fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > +        fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS)) {
> > +           DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID
> > mode\n");
> > +           return -EINVAL;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> >  int skl_plane_check(struct intel_crtc_state *crtc_state,
> >                 struct intel_plane_state *plane_state)
> >  {
> > @@ -1164,6 +1226,10 @@ int skl_plane_check(struct intel_crtc_state
> > *crtc_state,
> >     int max_scale, min_scale;
> >     int ret;
> >  
> > +   ret = skl_plane_check_fb(crtc_state, plane_state);
> > +   if (ret)
> > +           return ret;
> > +
> >     /* use scaler when colorkey is not required */
> >     if (!plane_state->ckey.flags) {
> >             const struct drm_framebuffer *fb = plane_state-
> > >base.fb;

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to