> -----Original Message----- > From: daniel.vetter@xxxxxxxx [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > Daniel Vetter > Sent: Wednesday, March 18, 2015 1:16 AM > To: Konduru, Chandra > Cc: intel-gfx; Conselvan De Oliveira, Ander; Vetter, Daniel > Subject: Re: [PATCH 08/21] drm/i915: Add helper function to update > scaler_users in crtc_state > > On Sun, Mar 15, 2015 at 6:55 AM, Chandra Konduru > <chandra.konduru@xxxxxxxxx> 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? I agree that some of limits are fixed, ratios aren't fixed. So kept them in state and updated during modeset. There isn't much complexity with current approach. > > 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. Agree. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
