| Message ID | 1431925865-7637-4-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, May 17, 2015 at 10:10:56PM -0700, Chandra Konduru wrote: > This patch stages a scaler request when input format > is NV12. The same scaler does both chroma-upsampling > and resolution scaling as needed. > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0a2e883..1ad7d13 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4499,9 +4499,11 @@ skl_update_scaler_users( > rotation = DRM_ROTATE_0; > } > > - need_scaling = intel_rotation_90_or_270(rotation) ? > - (src_h != dst_w || src_w != dst_h): > - (src_w != dst_w || src_h != dst_h); > + /* scaling is required when src dst sizes doesn't match or format is NV12 */ > + need_scaling = (src_w != dst_w || src_h != dst_h || > + (intel_rotation_90_or_270(rotation) && > + (src_h != dst_w || src_w != dst_h)) || That doesn't look right. Maybe add a small helper function that has these scaling checks so that we don't need to have them all in the same if statement. > + (fb && fb->pixel_format == DRM_FORMAT_NV12)); > > /* > * if plane is being disabled or scaler is no more required or force detach > @@ -4567,6 +4569,7 @@ skl_update_scaler_users( > case DRM_FORMAT_YVYU: > case DRM_FORMAT_UYVY: > case DRM_FORMAT_VYUY: > + case DRM_FORMAT_NV12: > break; > default: > DRM_DEBUG_KMS("PLANE:%d FB:%d unsupported scaling format 0x%x\n", > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4499,9 +4499,11 @@ skl_update_scaler_users( > > rotation = DRM_ROTATE_0; > > } > > > > - need_scaling = intel_rotation_90_or_270(rotation) ? > > - (src_h != dst_w || src_w != dst_h): > > - (src_w != dst_w || src_h != dst_h); > > + /* scaling is required when src dst sizes doesn't match or format is NV12 > */ > > + need_scaling = (src_w != dst_w || src_h != dst_h || > > + (intel_rotation_90_or_270(rotation) && > > + (src_h != dst_w || src_w != dst_h)) || > > That doesn't look right. It is evaluating scaling needed by comparing 1) src != dst 2) format == nv12 Can you pls point what doesn't look right here? > Maybe add a small helper function that has these scaling > checks so that we don't need to have them all in the same if statement. Thought about doing that but have to pass around 6 params to helper and do the same evaluation there which seems unnecessary. > > > + (fb && fb->pixel_format == DRM_FORMAT_NV12)); > >
On Thu, May 21, 2015 at 04:24:00PM +0000, Konduru, Chandra wrote: > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -4499,9 +4499,11 @@ skl_update_scaler_users( > > > rotation = DRM_ROTATE_0; > > > } > > > > > > - need_scaling = intel_rotation_90_or_270(rotation) ? > > > - (src_h != dst_w || src_w != dst_h): > > > - (src_w != dst_w || src_h != dst_h); > > > + /* scaling is required when src dst sizes doesn't match or format is NV12 > > */ > > > + need_scaling = (src_w != dst_w || src_h != dst_h || > > > + (intel_rotation_90_or_270(rotation) && > > > + (src_h != dst_w || src_w != dst_h)) || > > > > That doesn't look right. > It is evaluating scaling needed by comparing > 1) src != dst > 2) format == nv12 > Can you pls point what doesn't look right here? It'll do these checks before even checking the rotation: src_w != dst_w || src_h != dst_h > > > Maybe add a small helper function that has these scaling > > checks so that we don't need to have them all in the same if statement. > > Thought about doing that but have to pass around 6 params to helper > and do the same evaluation there which seems unnecessary. Just figured it'll look a bit less convoluted if you split it up into a few separate if statements. And doing that via a helper avoids polluting the main codepath with the details. I tend to like small helper functions like that (maybe a bit too much sometimes :) > > > > > > + (fb && fb->pixel_format == DRM_FORMAT_NV12)); > > >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0a2e883..1ad7d13 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4499,9 +4499,11 @@ skl_update_scaler_users( rotation = DRM_ROTATE_0; } - need_scaling = intel_rotation_90_or_270(rotation) ? - (src_h != dst_w || src_w != dst_h): - (src_w != dst_w || src_h != dst_h); + /* scaling is required when src dst sizes doesn't match or format is NV12 */ + need_scaling = (src_w != dst_w || src_h != dst_h || + (intel_rotation_90_or_270(rotation) && + (src_h != dst_w || src_w != dst_h)) || + (fb && fb->pixel_format == DRM_FORMAT_NV12)); /* * if plane is being disabled or scaler is no more required or force detach @@ -4567,6 +4569,7 @@ skl_update_scaler_users( case DRM_FORMAT_YVYU: case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: + case DRM_FORMAT_NV12: break; default: DRM_DEBUG_KMS("PLANE:%d FB:%d unsupported scaling format 0x%x\n",
This patch stages a scaler request when input format is NV12. The same scaler does both chroma-upsampling and resolution scaling as needed. Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)