Op 07-05-18 om 10:34 schreef Srinivas, Vidya: > >> -----Original Message----- >> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] >> Sent: Monday, May 7, 2018 1:59 PM >> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel- >> gfx@xxxxxxxxxxxxxxxxxxxxx >> Subject: Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported >> format for sprite plane >> >> Op 07-05-18 om 10:29 schreef Srinivas, Vidya: >>>> -----Original Message----- >>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] >>>> Sent: Monday, May 7, 2018 1:55 PM >>>> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel- >>>> gfx@xxxxxxxxxxxxxxxxxxxxx >>>> Subject: Re: [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' <maarten.lankhorst@xxxxxxxxxxxxxxx>; intel- >>>>>> gfx@xxxxxxxxxxxxxxxxxxxxx >>>>>> Subject: RE: [PATCH v6 6/6] drm/i915: Add NV12 as >>>>>> supported format for sprite plane >>>>>> >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] >>>>>>> Sent: Monday, May 7, 2018 1:44 PM >>>>>>> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel- >>>>>>> gfx@xxxxxxxxxxxxxxxxxxxxx >>>>>>> Subject: Re: [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:maarten.lankhorst@xxxxxxxxxxxxxxx] >>>>>>>>> Sent: Monday, May 7, 2018 1:38 PM >>>>>>>>> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel- >>>>>>>>> gfx@xxxxxxxxxxxxxxxxxxxxx >>>>>>>>> Subject: Re: [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 <chandra.konduru@xxxxxxxxx> >>>>>>>>>> >>>>>>>>>> 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 <hoegsberg@xxxxxxxxx> >>>>>>>>>> >>>>>>>>>> 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 <megha.aggarwal@xxxxxxxxx> >>>>>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Maarten Lankhorst >>>>>> <maarten.lankhorst@xxxxxxxxxxxxxxx> >>>>>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Juha-Pekka Heikkila >> <juhapekka.heikkila@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx> >>>>>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx> >>>>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@xxxxxxxxx> >>>>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx> >>>>>>>>>> --- >>>>>>>>>> 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. >> Yeah, in general that's what the 'fall through' comment is for, to make it >> clear that the lack of break is intentional and not a bug. :) > Oh okay, thank you so much. I did not know about that. I have made the change > and refloated the series. Kindly have a check. https://patchwork.freedesktop.org/series/41674/ > Rev 5. > > Regards > Vidya Ack, looking good. Now we only have to wait for the drm-misc-next backmerge, then I will commit this. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
