Re: [PATCH 15/15] drm/i915: Add NV12 support to intel_framebuffer_init

Maarten Lankhorst
2018-01-18T14:50:00Z

Op 18-01-18 om 15:21 schreef Maarten Lankhorst:
> Op 20-01-18 om 22:45 schreef Vidya Srinivas:
>> From: Chandra Konduru <chandra.konduru@intel.com>
>>
>> This patch adds NV12 as supported format
>> to intel_framebuffer_init and performs various checks.
>>
>> v2:
>> -Fix an issue in checks added (Chandra Konduru)
>>
>> v3: rebased (me)
>>
>> v4: Review comments by Ville addressed
>> Added platform check for NV12 in intel_framebuffer_init
>> Removed offset checks for NV12 case
>>
>> v5: Addressed review comments by Clinton A Taylor
>> This NV12 support only correctly works on SKL.
>> Plane color space conversion is different on GLK and later platforms
>> causing the colors to display incorrectly.
>> Ville's plane color space property patch series
>> in review will fix this issue.
>> - Restricted the NV12 case in intel_framebuffer_init to
>> SKL and BXT only.
>>
>> v6: Rebased (me)
>>
>> v7: Addressed review comments by Ville
>> Restricting the NV12 to BXT for now.
>>
>> v8: Rebased (me)
>> Restricting the NV12 changes to BXT and KBL for now.
>>
>> v9: Rebased (me)
>>
>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 88bc750..db42448 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14005,6 +14005,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>> goto err;
>> }
>> break;
>> + case DRM_FORMAT_NV12:
>> + if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
>> + DRM_DEBUG_KMS("unsupported pixel format: %s\n",
>> + drm_get_format_name(mode_cmd->pixel_format,
>> + &format_name));
>> + goto err;
>> + }
>> + break;
> I started implementing the IGT side of this but as far as I can tell, the specification will allow you to set different handles for each plane. But intel doesn't support this right now..
>
> Could this assumption be fixed first? There's nothing stopping the drm core or applications from having separate buffer objects, and for blitting it would make sense to have a R8 plane for Y', and a R8G8 plane for Cb'Cr'
>
> Probably needs i915 to compile with something like below, then handling all corner cases..
Never mind, seems this is a hardware limitation that bo for plane2 must be higher mapped than plane1 so having separate bo's won't do much. And there was already a check for handles[i] == handles[0].