On Thu, Mar 19, 2015 at 05:43:24PM +0000, Konduru, Chandra wrote: > > > > -----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. Hm, where are the ratios? I haven't found that part in your series ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
