> -----Original Message-----
> From: Maarten Lankhorst [mailto:[email protected]]
> Sent: Monday, May 7, 2018 1:55 PM
> To: Srinivas, Vidya <[email protected]>; intel-
> [email protected]
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
>
> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Srinivas, Vidya
> >> Sent: Monday, May 7, 2018 1:46 PM
> >> To: 'Maarten Lankhorst' <[email protected]>; intel-
> >> [email protected]
> >> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >> supported format for sprite plane
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Maarten Lankhorst [mailto:[email protected]]
> >>> Sent: Monday, May 7, 2018 1:44 PM
> >>> To: Srinivas, Vidya <[email protected]>; intel-
> >>> [email protected]
> >>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>> supported format for sprite plane
> >>>
> >>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> >>>>> -----Original Message-----
> >>>>> From: Maarten Lankhorst [mailto:[email protected]]
> >>>>> Sent: Monday, May 7, 2018 1:38 PM
> >>>>> To: Srinivas, Vidya <[email protected]>; intel-
> >>>>> [email protected]
> >>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>> supported format for sprite plane
> >>>>>
> >>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> >>>>>> From: Chandra Konduru <[email protected]>
> >>>>>>
> >>>>>> This patch adds NV12 to list of supported formats for sprite plane.
> >>>>>>
> >>>>>> v2: Rebased (me)
> >>>>>>
> >>>>>> v3: Review comments by Ville addressed
> >>>>>> - Removed skl_plane_formats_with_nv12 and added
> >>>>>> NV12 case in existing skl_plane_formats
> >>>>>> - Added the 10bpc RGB formats
> >>>>>>
> >>>>>> v4: Addressed review comments from Clinton A Taylor "Why are we
> >>>>> adding
> >>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>> Trying to set XR30 or AB30 results in error returned even though
> >>>>>> the modes are advertised for the planes"
> >>>>>> - Removed 10bit RGB formats added previously with NV12 series
> >>>>>>
> >>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
> >>>>>> Adding the same to commit message in this version.
> >>>>>> Addressed review comments from Clinton A Taylor "Why are we
> >> adding
> >>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>> Trying to set XR30 or AB30 results in error returned even though
> >>>>>> the modes are advertised for the planes"
> >>>>>> - Previous version has 10bit RGB format removed from VLV formats
> >>>>>> by mistake. Fixing that in this version.
> >>>>>> Removed 10bit RGB formats added previously with NV12 series for
> >> SKL.
> >>>>>> v6: Addressed review comments by Ville Restricting the NV12 to
> >>>>>> BXT and PIPE A and B
> >>>>>>
> >>>>>> v7: Rebased (me)
> >>>>>>
> >>>>>> v8: Rebased (me)
> >>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes
> >>>>>> for plane 0 (overlay)
> >>>>>>
> >>>>>> v9: Rebased (me)
> >>>>>>
> >>>>>> v10: Addressed review comments from Maarten.
> >>>>>> Adding NV12 to skl_plane_formats itself.
> >>>>>>
> >>>>>> v11: Addressed review comments from Shashank Sharma
> >>>>>>
> >>>>>> v12: Addressed review comments from Shashank Sharma Made the
> >>>>> condition
> >>>>>> in intel_sprite_plane_create simple and easy to read as suggested.
> >>>>>>
> >>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
> review
> >>>>>> comments from Juha-Pekka Heikkila
> >>>>>> "NV12 not to be supported by SKL"
> >>>>>>
> >>>>>> v14: Addressed review comments from Ville Added
> >> skl_planar_formats
> >>>>>> to include NV12 and a check skl_plane_has_planar in sprite create
> >>>>>> Added
> >>>>>> NV12 format to skl_mod_supported. These were review comments
> >>> from
> >>>>>> Kristian Høgsberg <[email protected]>
> >>>>>>
> >>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
> >>>>>>
> >>>>>> v16: Rebased the series
> >>>>>>
> >>>>>> v17: Added all tiling under mod supported for NV12 Credits to
> >>>>>> Megha Aggarwal
> >>>>>>
> >>>>>> v18: Added RB by Maarten and Kristian
> >>>>>>
> >>>>>> Credits-to: Megha Aggarwal <[email protected]>
> >>>>>> Credits-to: Kristian Høgsberg <[email protected]>
> >>>>>> Reviewed-by: Kristian Høgsberg <[email protected]>
> >>>>>> Reviewed-by: Maarten Lankhorst
> >> <[email protected]>
> >>>>>> Tested-by: Clinton Taylor <[email protected]>
> >>>>>> Reviewed-by: Juha-Pekka Heikkila <[email protected]>
> >>>>>> Reviewed-by: Shashank Sharma <[email protected]>
> >>>>>> Reviewed-by: Clinton Taylor <[email protected]>
> >>>>>> Signed-off-by: Chandra Konduru <[email protected]>
> >>>>>> Signed-off-by: Nabendu Maiti <[email protected]>
> >>>>>> Signed-off-by: Vidya Srinivas <[email protected]>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/i915/intel_sprite.c | 29
> >>>>>> +++++++++++++++++++++++++++--
> >>>>>> 1 file changed, 27 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> index c73553a..cdcae9e 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
> >>>>>> DRM_FORMAT_VYUY,
> >>>>>> };
> >>>>>>
> >>>>>> +static uint32_t skl_planar_formats[] = {
> >>>>>> + DRM_FORMAT_RGB565,
> >>>>>> + DRM_FORMAT_ABGR8888,
> >>>>>> + DRM_FORMAT_ARGB8888,
> >>>>>> + DRM_FORMAT_XBGR8888,
> >>>>>> + DRM_FORMAT_XRGB8888,
> >>>>>> + DRM_FORMAT_YUYV,
> >>>>>> + DRM_FORMAT_YVYU,
> >>>>>> + DRM_FORMAT_UYVY,
> >>>>>> + DRM_FORMAT_VYUY,
> >>>>>> + DRM_FORMAT_NV12,
> >>>>>> +};
> >>>>>> +
> >>>>>> static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >>>>>> I915_FORMAT_MOD_Yf_TILED,
> >>>>>> I915_FORMAT_MOD_Y_TILED,
> >>>>>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
> >>>>> format, uint64_t modifier)
> >>>>>> if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>> return true;
> >>>>>> /* fall through */
> >>>>>> + case DRM_FORMAT_NV12:
> >>>>>> + if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >>>>>> + modifier == I915_FORMAT_MOD_X_TILED ||
> >>>>>> + modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>>>> + modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>> + return true;
> >>>>> On patch 5 and 6: this is a tad overkill, just put it below
> >>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
> >>>>> the other formats. :) Otherwise looks good, I'll wait until the
> >>>>> changes from drm-misc- next are merged into drm-intel-next-queued
> >>>>> then this series can be applied with the minor fixup.
> >>>> Sure, thank you. I'll make the change. I did this to fix a review
> >>>> comment - which said all modifiers aren’t Covered for NV12 - when
> >>>> we
> >>> earlier had it under VYUV. Would that be still okay?
> >>> Seems only CCS isn't covered, so putting it there should be fine. :)
> >> Oh okay - sure thank you. Will make the change and float the series.
> > Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need to
> > add the other modifiers like MOD_LINEAR, MOD_X_TILED and
> MOD_Y_TILED?
> >> Regards
> >> Vidya
>
> This is a C switch, unless there is a break it will continue, so it works as
> intended. :)
>
> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore all the
> lines with case and default, and continue to the end of the switch
> statement, or until it hits a 'break' so the below:
I am so sorry, I did not notice there was no break :( Got confused. Thank you.
Regards
Vidya
>
>
> case DRM_FORMAT_UYVY:
> case DRM_FORMAT_VYUY:
> if (modifier == I915_FORMAT_MOD_Yf_TILED)
> return true;
> /* fall through */
> case DRM_FORMAT_C8:
> if (modifier == DRM_FORMAT_MOD_LINEAR ||
> modifier == I915_FORMAT_MOD_X_TILED ||
> modifier == I915_FORMAT_MOD_Y_TILED)
> return true;
> /* fall through */
> default:
> return false;
> }
>
> for UYVY ends up being the same as:
>
> if (format == DRM_FORMAT_UYVY) {
> if (modifier == I915_FORMAT_MOD_Yf_TILED)
> return true;
> if (modifier == DRM_FORMAT_MOD_LINEAR ||
> modifier == I915_FORMAT_MOD_X_TILED ||
> modifier == I915_FORMAT_MOD_Y_TILED)
> return true;
> return false;
> }
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
