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. :) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
