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

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Vidya Srinivas <vidya.srinivas@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 15/15] drm/i915: Add NV12 support to intel_framebuffer_init
Date: Thu, 18 Jan 2018 15:21:07 +0100	[thread overview]
Message-ID: <9ca3eb30-2c37-3f1c-0ef9-c355d4ff0d8d@linux.intel.com> (raw)
In-Reply-To: <1516484713-5837-16-git-send-email-vidya.srinivas@intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39c3bfa74d67..4eeb94fbfdd6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -186,7 +186,7 @@ enum intel_output_type {
 
 struct intel_framebuffer {
 	struct drm_framebuffer base;
-	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_object *objs[2];
 	struct intel_rotation_info rot_info;
 
 	/* for each plane in the normal GTT view */
@@ -996,7 +996,7 @@ struct cxsr_latency {
 #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
 #define to_intel_plane(x) container_of(x, struct intel_plane, base)
 #define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base)
-#define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
+#define intel_fb_obj(x, n) (x ? to_intel_framebuffer(x)->objs[n] : NULL)
 
 struct intel_hdmi {
 	i915_reg_t hdmi_reg;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
