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

Wed Sep 6 22:53:41 UTC 2017

On Wed, Sep 6, 2017 at 3:02 PM, Matt Roper <matthew.d.roper at intel.com> wrote:
> On Wed, Sep 06, 2017 at 02:49:07PM -0700, Kristian Høgsberg wrote:
>> On Wed, Sep 6, 2017 at 1:17 PM, Matt Roper <matthew.d.roper at intel.com> wrote:
>> > On Wed, Sep 06, 2017 at 12:12:10PM -0700, Rodrigo Vivi wrote:
>> >> 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 at 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 at intel.com>
>> >> > > Reviewed-by: Clinton Taylor <clinton.a.taylor at intel.com>
>> >> > > Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
>> >> > > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti at intel.com>
>> >> > > Signed-off-by: Vidya Srinivas <vidya.srinivas at 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?
>> >
>> > Unfortunately the bspec WA database still hasn't been updated to
>> > indicate that *any* SKU of can properly support NV12.  So the workaround
>> > database still just gives a general "don't use NV12 at all" statement
>> > (entry 0870 in the display WA database which is listed for "BXT:ALL").
>
> Woops, this is actually 0826, not 0870 (0870 is a different NV12 entry
> specifically for y-tile).
>
>
>> > I tried unravel what the internal communication channels are for this
>> > kind of update a few months ago, but didn't make much headway.
>>
>> What about KBL support?
>>
>
> The only NV12 workaround I see for KBL is that KBL A-step and B-step
> shouldn't do NV12 + ytile.

But does this series actually enable KBL NV12 overlays? I only see
IS_BROXTON() in the conditional - that doesn't include KBL, does it?

> Matt
>
>
>> >>
>> >> But also, is there any team there still using anything older than D0? yet?
>> >
>> > Yes, definitely.  Pre-E0 SoC's (and thus pre-D0 graphics) is what a lot
>> > of our embedded customers have already gone to production with.
>> >
>> >
>> > Matt
>> >
>> >
>> >>
>> >> 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 at 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 at 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 at 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