Re: [PATCH 4/6] drm/i915: Add NV12 as supported format for primary plane

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
> On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> > 
> > This patch adds NV12 to list of supported formats for
> > primary plane
> > 
> > v2: Rebased (Chandra Konduru)
> > 
> > v3: Rebased (me)
> > 
> > v4: Review comments by Ville addressed
> > 	Removed the skl_primary_formats_with_nv12 and
> > 	added NV12 case in existing skl_primary_formats
> > 
> > v5: Rebased (me)
> > 
> > v6: Missed the Tested-by/Reviewed-by in the previous series
> > 	Adding the same to commit message in this version.
> > 
> > v7: Review comments by Ville addressed
> > 	Restricting the NV12 for BXT and on PIPE A and B
> > 	Rebased (me)
> > 
> > v8: Rebased (me)
> > 
> > Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--
> >  1 file changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4e73d88..6cf8806 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -106,6 +106,22 @@
> >  	DRM_FORMAT_MOD_INVALID
> >  };
> >  
> > +static const uint32_t nv12_primary_formats[] = {
> > +	DRM_FORMAT_C8,
> > +	DRM_FORMAT_RGB565,
> > +	DRM_FORMAT_XRGB8888,
> > +	DRM_FORMAT_XBGR8888,
> > +	DRM_FORMAT_ARGB8888,
> > +	DRM_FORMAT_ABGR8888,
> > +	DRM_FORMAT_XRGB2101010,
> > +	DRM_FORMAT_XBGR2101010,
> > +	DRM_FORMAT_YUYV,
> > +	DRM_FORMAT_YVYU,
> > +	DRM_FORMAT_UYVY,
> > +	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> > +};
> > +
> >  /* Cursor formats */
> >  static const uint32_t intel_cursor_formats[] = {
> >  	DRM_FORMAT_ARGB8888,
> > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
> >  		primary->update_plane = skylake_update_primary_plane;
> >  		primary->disable_plane = skylake_disable_primary_plane;
> >  	} else if (INTEL_GEN(dev_priv) >= 9) {
> > -		intel_primary_formats = skl_primary_formats;
> > -		num_formats = ARRAY_SIZE(skl_primary_formats);
> > +		if (IS_BROXTON(dev_priv) &&
> 
> I believe this needs to be
> 
>    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...

We usually use this stepping information for Workarounds. So usually
blocks around this are the non-expected default behaviour.
So I'd handle that from A0 to C0 and else the default behaviour or at least
![A0,C0]...

> 
> There were unavoidable flickering/underrun issues on the earlier
> steppings due to memory fetch issues for the second color plane.  Those
> issues were only fixed on the E0 SoC stepping (which incorporates the D0
> Display/GT).

Also we usuallly use this steppings checking with a documented W/a.
Is there one? Anything that would justify this?

But also, is there any team there still using anything older than D0? yet?

If we don't know anyone probably pure IS_BROXTON is the best option.

> 
> Same change for your sprite plane changes in the next patch.
> 
> 
> Matt
> 
> > +			((pipe == PIPE_A || pipe == PIPE_B))) {
> > +			intel_primary_formats = nv12_primary_formats;
> > +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> > +		} else {
> > +			intel_primary_formats = skl_primary_formats;
> > +			num_formats = ARRAY_SIZE(skl_primary_formats);
> > +		}
> >  		if (pipe < PIPE_C)
> >  			modifiers = skl_format_modifiers_ccs;
> >  		else
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx