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:
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
