Re: [PATCH 03/12] drm/i915: Stage scaler request for NV12 as src format

Konduru, Chandra
2015-05-21T16:24:02Z

> > --- 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));
> >