Re: [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers

Konduru, Chandra
2015-04-27T05:13:54Z

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Thursday, April 23, 2015 1:20 PM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander
> Subject: Re: [Intel-gfx] [PATCH 13/14] drm/i915: skylake primary plane scaling
> using shared scalers
>
> On Wed, Apr 15, 2015 at 03:14:38PM -0700, Chandra Konduru wrote:
> > This patch enables skylake primary plane scaling using shared
> > scalers atomic desgin.
> >
> > v2:
> > -use single copy of scaler limits (Matt)
> >
> > v3:
> > -move detach_scalers to crtc commit path (Matt)
> > -use values in plane_state->src as regular integers (me)
> >
> > v4:
> > -changes to align with updated scaler structures (Matt, me)
> > -keep plane src rect in 16.16 format (Matt, Daniel)
> >
> > v5:
> > -Rebased on top of 90/270 rotation changes (me)
> > -Fixed an issue introduced by 90/270 changes where plane programming
> > is using drm_plane->state rect instead of intel_plane->state rect.
> > This change also required for scaling to work properly. (me)
> > -With 90/270, updated limits to cover both portrait and landscape usages (me)
> > -Refactored skylake_update_primary_plane to reduce its size (Daniel)
> > Added helper functions for refactoring are comprehended enough to be
> > used for skylake_update_plane (for sprite) too. One stop towards
> > having single function for all planes.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > Testcase: kms_plane_scaling
>
> I wanted to pull this in but then spotted an issue. Since this needs one
> more round can you perhaps use the older version as a baseline again
> (without the refactoring)? Since skl planes aren't converted to universal
> planes yet it might be better to wait with refactoring until that's done
> actually. Two more comments below.
Hi Daniel,
Sorry for delay due to osts/f2f travel.
Per last discussion at osts, you were ok to merge the changes made to
skl update_plane functions to reduce its size. To undo these
changes I have to redo all testing and also have to redo changes
to upcoming nv12 changes.
skl planes are already universal planes, Matt can comment more
on this. But irrespective of whether skl planes universal or not
with below changes, function sizes are more manageable.

Regarding the FIXME and lock issue, will send out updated
patch shortly.

-Chandra