diff mbox

[7/8] drm/i915: Add NV12 as supported format for sprite plane

Message ID 1499782256-25664-8-git-send-email-vidya.srinivas@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Srinivas, Vidya July 11, 2017, 2:10 p.m. UTC 
From: Chandra Konduru <chandra.konduru@intel.com>

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.

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_sprite.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Clint Taylor July 11, 2017, 3:04 p.m. UTC | #1 
On 07/11/2017 07:10 AM, Vidya Srinivas wrote:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> 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.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
I had not actually given the reviewed-by for patch 7/8 yet. Make sure 
that you only add the reviewed-by to the commit message for patches that 
have been marked that way by the reviewers.

Of course now the patch looks good and we can leave the RB in the commit 
message.

Thanks for getting this done,
-Clint

> 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_sprite.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9a6b011..f1d65d9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1078,6 +1078,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>   	DRM_FORMAT_YVYU,
>   	DRM_FORMAT_UYVY,
>   	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
>   };
>   
>   struct intel_plane *
Ville Syrjälä July 11, 2017, 4:12 p.m. UTC | #2 
On Tue, Jul 11, 2017 at 07:40:55PM +0530, Vidya Srinivas wrote:
> From: Chandra Konduru <chandra.konduru@intel.com>
> 
> 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.
> 
> 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_sprite.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9a6b011..f1d65d9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1078,6 +1078,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  	DRM_FORMAT_YVYU,
>  	DRM_FORMAT_UYVY,
>  	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
>  };

That's not correct for pipe C sprites on SKL/KBL/BXT.

Also like I mentioned earlier at least SKL shouldn't be
advertizing NV12 at all. IIRC you were going to find out what
the situation is with the other platforms.

>  
>  struct intel_plane *
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Srinivas, Vidya July 20, 2017, 10:05 a.m. UTC | #3 
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Tuesday, July 11, 2017 9:42 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 7/8] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> On Tue, Jul 11, 2017 at 07:40:55PM +0530, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > 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.
> >
> > 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_sprite.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 9a6b011..f1d65d9 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1078,6 +1078,7 @@ int intel_sprite_set_colorkey(struct drm_device
> *dev, void *data,
> >  	DRM_FORMAT_YVYU,
> >  	DRM_FORMAT_UYVY,
> >  	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> >  };
> 
> That's not correct for pipe C sprites on SKL/KBL/BXT.
> 
> Also like I mentioned earlier at least SKL shouldn't be advertizing NV12 at
> all. IIRC you were going to find out what the situation is with the other
> platforms.

Apologies for the delay. I shall find this and fix before sending the next series with other
review comments addressed.

Regards
Vidya
> 
> >
> >  struct intel_plane *
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
Srinivas, Vidya July 24, 2017, 4:24 a.m. UTC | #4 
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Tuesday, July 11, 2017 9:42 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 7/8] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> On Tue, Jul 11, 2017 at 07:40:55PM +0530, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > 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.
> >
> > 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_sprite.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 9a6b011..f1d65d9 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1078,6 +1078,7 @@ int intel_sprite_set_colorkey(struct drm_device
> *dev, void *data,
> >  	DRM_FORMAT_YVYU,
> >  	DRM_FORMAT_UYVY,
> >  	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> >  };
> 
> That's not correct for pipe C sprites on SKL/KBL/BXT.
> 
> Also like I mentioned earlier at least SKL shouldn't be advertizing NV12 at
> all. IIRC you were going to find out what the situation is with the other
> platforms.
> 

I checked for the platform support we need.
For now, we wanted to enable it on Apollo Lake platforms (BXT).I have changed
the patch and addressed the review comments. Kindly have a check.
Thank you.

Regards
Vidya

> >
> >  struct intel_plane *
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9a6b011..f1d65d9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1078,6 +1078,7 @@  int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
 };
 
 struct intel_plane *