[Intel-gfx] [PATCH 08/21] drm/i915: Add helper function to update scaler_users in crtc_state

[Daniel Vetter]

On Sun, Mar 15, 2015 at 6:55 AM, Chandra Konduru
<chandra.konduru at intel.com> wrote:
> +       /*
> +        * check for rect size:
> +        *     min sizes in case of scaling involved
> +        *     max sizes in all cases
> +        */
> +       if ((need_scaling &&
> +               (src_w < scaler->min_src_w || src_h < scaler->min_src_h ||
> +                dst_w < scaler->min_dst_w || dst_h < scaler->min_dst_h)) ||
> +
> +                src_w > scaler->max_src_w || src_h > scaler->max_src_h ||
> +                dst_w > scaler->max_dst_w || dst_h > scaler->max_dst_h) {

Ok let's hope I've traced all the min/max stuff in your patches
correctly. It looks like we only ever initialized them to fixed
values, never changed them and only use them here. Spreading out the
values like that without having a real need for this flexibility makes
review really hard imo.

What about instead adding a skl_check_scale_limits functions which
does all these checks here and uses hardcoded values? That way you
could move the commits about the various values (e.g. only 34% scaling
and the other easier-to-understand limits) right next to the code that
checks these limits?

There's also some confusion with the overly generic (imo) old sprite
code and its scaling limit checks. Imo we can look at that later on.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch