Re: [Intel-gfx] [PATCH 11/16] drm/i915: Update format_is_yuv() to include NV12
Sorry about this. Will fix it in the next push. Thanks.
Vidya
> -Original Message-
> From: Sharma, Shashank
> Sent: Thursday, February 15, 2018 12:05 PM
> To: Srinivas, Vidya ; intel-
> g...@lists.freedesktop.org
> Cc: maarten.lankho...@linux.intel.com; Kamath, Sunil
> ; Shankar, Uma ;
> Konduru, Chandra ; Maiti, Nabendu Bikash
>
> Subject: Re: [PATCH 11/16] drm/i915: Update format_is_yuv() to include
> NV12
>
> My previous review comments is neither acknowledged nor addressed.
>
> Regards
> Shashank
> On 2/14/2018 10:27 AM, Vidya Srinivas wrote:
> > From: Chandra Konduru
> >
> > This patch adds NV12 to format_is_yuv() function for sprite planes.
> >
> > v2:
> > -Use intel_ prefix for format_is_yuv (Ville)
> >
> > v3: Rebased (me)
> >
> > v4: Rebased and addressed review comments from Clinton A Taylor.
> > "static function in intel_sprite.c is not available to the primary
> > plane functions".
> > Changed commit message - function modified for sprite planes.
> >
> > v5: Missed the Tested-by/Reviewed-by in the previous series Adding the
> > same to commit message in this version.
> >
> > v6: Rebased (me)
> >
> > v7: Rebased (me)
> >
> > v8: Rebased (me)
> >
> > v9: Rebased (me)
> >
> > v10: Changed intel_format_is_yuv function from static to non-static.
> > We need to use it later from other files for check.
> >
> > Tested-by: Clinton Taylor
> > Reviewed-by: Clinton Taylor
> > Reviewed-by: Shashank Sharma
> > Signed-off-by: Chandra Konduru
> > Signed-off-by: Nabendu Maiti
> > Signed-off-by: Vidya Srinivas
> > ---
> > drivers/gpu/drm/i915/intel_drv.h| 1 +
> > drivers/gpu/drm/i915/intel_sprite.c | 8
> > 2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 6402d6d..8d4988b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -2034,6 +2034,7 @@ void skl_disable_plane(struct intel_plane
> *plane, struct intel_crtc *crtc);
> > bool skl_plane_get_hw_state(struct intel_plane *plane);
> > bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> >enum pipe pipe, enum plane_id plane_id);
> > +bool intel_format_is_yuv(uint32_t format);
> >
> > /* intel_tv.c */
> > void intel_tv_init(struct drm_i915_private *dev_priv); diff --git
> > a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index e098e4b..2c51d8a 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -41,14 +41,14 @@
> > #include
> > #include "i915_drv.h"
> >
> > -static bool
> > -format_is_yuv(uint32_t format)
> > +bool intel_format_is_yuv(uint32_t format)
> > {
> > switch (format) {
> > case DRM_FORMAT_YUYV:
> > case DRM_FORMAT_UYVY:
> > case DRM_FORMAT_VYUY:
> > case DRM_FORMAT_YVYU:
> > + case DRM_FORMAT_NV12:
> > return true;
> > default:
> > return false;
> > @@ -352,7 +352,7 @@ chv_update_csc(struct intel_plane *plane,
> uint32_t format)
> > enum plane_id plane_id = plane->id;
> >
> > /* Seems RGB data bypasses the CSC always */
> > - if (!format_is_yuv(format))
> > + if (!intel_format_is_yuv(format))
> > return;
> >
> > /*
> > @@ -979,7 +979,7 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> > src_y = src->y1 >> 16;
> > src_h = drm_rect_height(src) >> 16;
> >
> > - if (format_is_yuv(fb->format->format)) {
> > + if (intel_format_is_yuv(fb->format->format)) {
> > src_x &= ~1;
> > src_w &= ~1;
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/16] drm/i915: Update format_is_yuv() to include NV12
My previous review comments is neither acknowledged nor addressed.
Regards
Shashank
On 2/14/2018 10:27 AM, Vidya Srinivas wrote:
From: Chandra Konduru
This patch adds NV12 to format_is_yuv() function
for sprite planes.
v2:
-Use intel_ prefix for format_is_yuv (Ville)
v3: Rebased (me)
v4: Rebased and addressed review comments from Clinton A Taylor.
"static function in intel_sprite.c is not available
to the primary plane functions".
Changed commit message - function modified for
sprite planes.
v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.
v6: Rebased (me)
v7: Rebased (me)
v8: Rebased (me)
v9: Rebased (me)
v10: Changed intel_format_is_yuv function from
static to non-static. We need to use it later from
other files for check.
Tested-by: Clinton Taylor
Reviewed-by: Clinton Taylor
Reviewed-by: Shashank Sharma
Signed-off-by: Chandra Konduru
Signed-off-by: Nabendu Maiti
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/intel_drv.h| 1 +
drivers/gpu/drm/i915/intel_sprite.c | 8
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6402d6d..8d4988b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2034,6 +2034,7 @@ void skl_disable_plane(struct intel_plane *plane, struct
intel_crtc *crtc);
bool skl_plane_get_hw_state(struct intel_plane *plane);
bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
enum pipe pipe, enum plane_id plane_id);
+bool intel_format_is_yuv(uint32_t format);
/* intel_tv.c */
void intel_tv_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index e098e4b..2c51d8a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -41,14 +41,14 @@
#include
#include "i915_drv.h"
-static bool
-format_is_yuv(uint32_t format)
+bool intel_format_is_yuv(uint32_t format)
{
switch (format) {
case DRM_FORMAT_YUYV:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_NV12:
return true;
default:
return false;
@@ -352,7 +352,7 @@ chv_update_csc(struct intel_plane *plane, uint32_t format)
enum plane_id plane_id = plane->id;
/* Seems RGB data bypasses the CSC always */
- if (!format_is_yuv(format))
+ if (!intel_format_is_yuv(format))
return;
/*
@@ -979,7 +979,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
src_y = src->y1 >> 16;
src_h = drm_rect_height(src) >> 16;
- if (format_is_yuv(fb->format->format)) {
+ if (intel_format_is_yuv(fb->format->format)) {
src_x &= ~1;
src_w &= ~1;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/16] drm/i915: Update format_is_yuv() to include NV12
Regards
Shashank
On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
From: Chandra Konduru
This patch adds NV12 to format_is_yuv() function
for sprite planes.
v2:
-Use intel_ prefix for format_is_yuv (Ville)
v3: Rebased (me)
v4: Rebased and addressed review comments from Clinton A Taylor.
"static function in intel_sprite.c is not available
to the primary plane functions".
Changed commit message - function modified for
sprite planes.
v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.
v6: Rebased (me)
v7: Rebased (me)
v8: Rebased (me)
v9: Rebased (me)
v10: Changed intel_format_is_yuv function from
static to non-static. We need to use it later from
other files for check.
Tested-by: Clinton Taylor
Reviewed-by: Clinton Taylor
Signed-off-by: Chandra Konduru
Signed-off-by: Nabendu Maiti
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/intel_drv.h| 1 +
drivers/gpu/drm/i915/intel_sprite.c | 8
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 65dac21..d4c027a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2032,6 +2032,7 @@ void skl_disable_plane(struct intel_plane *plane, struct
intel_crtc *crtc);
bool skl_plane_get_hw_state(struct intel_plane *plane);
bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
enum pipe pipe, enum plane_id plane_id);
+bool intel_format_is_yuv(uint32_t format);
/* intel_tv.c */
void intel_tv_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 3be22c0..9e31be2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -41,14 +41,14 @@
#include
#include "i915_drv.h"
-static bool
-format_is_yuv(uint32_t format)
+bool intel_format_is_yuv(uint32_t format)
{
switch (format) {
case DRM_FORMAT_YUYV:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_YVYU:
+ case DRM_FORMAT_NV12:
return true;
default:
return false;
@@ -352,7 +352,7 @@ chv_update_csc(struct intel_plane *plane, uint32_t format)
enum plane_id plane_id = plane->id;
/* Seems RGB data bypasses the CSC always */
- if (!format_is_yuv(format))
+ if (!intel_format_is_yuv(format))
This assumption that CSC does only limited_range to full_range
conversion is only correct until we enable plane level gamut mapping /
generic color conversion. It would be great if we keep it like:
chv_update_csc() {
if (intel_format_is_yuv()) {
/* limited_range -> full_range code here */
}
/* So that generic CSC/GM code can be added here */
}
But this should not really a blocker for this patch,
return;
/*
@@ -979,7 +979,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
src_y = src->y1 >> 16;
src_h = drm_rect_height(src) >> 16;
- if (format_is_yuv(fb->format->format)) {
+ if (intel_format_is_yuv(fb->format->format)) {
src_x &= ~1;
src_w &= ~1;
so with or without above comment addressed:
Reviewed-by: Shashank Sharma
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
