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)); > > > -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
