On Fri, Aug 24, 2018 at 10:27:09PM +0000, Pandiyan, Dhinakaran wrote:
>
>
> On Fri, 2018-08-24 at 15:15 -0700, Rodrigo Vivi wrote:
> > On Fri, Aug 24, 2018 at 01:38:55PM -0700, Dhinakaran Pandiyan wrote:
> > > skl_plane_has_planar is hard to read, simplify the logic by
> > > checking for
> > > support in the order of platform, pipe and plane.
> > >
> > > No change in functionality intended.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 27 +++++++++---------------
> > > ---
> > > 1 file changed, 9 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 30fdfd1a3037..7e18bd8b21b8 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13622,24 +13622,15 @@ static bool skl_plane_has_fbc(struct
> > > drm_i915_private *dev_priv,
> > > bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
> > > enum pipe pipe, enum plane_id plane_id)
> > > {
> > > - if (plane_id == PLANE_PRIMARY) {
> > > - if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > - return false;
> > > - else if ((INTEL_GEN(dev_priv) == 9 && pipe ==
> > > PIPE_C) &&
> > > - !IS_GEMINILAKE(dev_priv))
> > > - return false;
> > > - } else if (plane_id >= PLANE_SPRITE0) {
> > > - if (plane_id == PLANE_CURSOR)
> > > - return false;
> > > - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
> > > == 10) {
> > > - if (plane_id != PLANE_SPRITE0)
> > > - return false;
> > > - } else {
> > > - if (plane_id != PLANE_SPRITE0 || pipe ==
> > > PIPE_C ||
> > > - IS_SKYLAKE(dev_priv) ||
> > > IS_BROXTON(dev_priv))
> > > - return false;
> ^ Here it does, or am I not seeing the paranthesis correctly.
dam... it is so bad that I got confused when drawing a
table here...
nip: The name of the function seems wrong since "skl_" always
return false....
but I don't have a better suggestion ... maybe just "intel_"
and can be in a follow up patch...
so,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> > > - }
> > > - }
> > > + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
> > > + return false;
> >
> > The current code is indeed ugly, but I'm afraid it doesn't always
> > return
> > false for these platforms.
> >
> > for instance plane_id = PLANE_SPRITE0
> >
> > > +
> > > + if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)
> > > && pipe == PIPE_C)
> > > + return false;
> > > +
> > > + if (plane_id == PLANE_CURSOR || plane_id != PLANE_SPRITE0)
> > > + return false;
> > > +
> > > return true;
> > > }
> > >
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
