On Tue, Apr 16, 2013 at 03:49:58PM +0100, Chris Wilson wrote: > On Tue, Apr 16, 2013 at 05:14:14PM +0300, Ville Syrj?l? wrote: > > On Tue, Apr 16, 2013 at 02:42:34PM +0100, Chris Wilson wrote: > > > On Tue, Apr 16, 2013 at 01:47:20PM +0300, ville.syrjala at linux.intel.com wrote: > > > > diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h > > > > index 2b7278c..de24f16 100644 > > > > --- a/include/drm/drm_rect.h > > > > +++ b/include/drm/drm_rect.h > > > > @@ -128,5 +128,17 @@ bool drm_rect_intersect(struct drm_rect *r, const struct drm_rect *clip); > > > > bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst, > > > > const struct drm_rect *clip, > > > > int hscale, int vscale); > > > > +int drm_rect_calc_hscale(const struct drm_rect *src, > > > > + const struct drm_rect *dst, > > > > + int min_hscale, int max_hscale); > > > > +int drm_rect_calc_vscale(const struct drm_rect *src, > > > > + const struct drm_rect *dst, > > > > + int min_vscale, int max_vscale); > > > > +int drm_rect_calc_hscale_relaxed(struct drm_rect *src, > > > > + struct drm_rect *dst, > > > > + int min_hscale, int max_hscale); > > > > +int drm_rect_calc_vscale_relaxed(struct drm_rect *src, > > > > + struct drm_rect *dst, > > > > + int min_vscale, int max_vscale); > > > > > > These struct drm_rect *src should be const so it is clear that dst is > > > being manipulated. > > > > Actually they can manipulate either src or dst, depending on whether > > we're trying upscale or downscale too much. The idea being that we > > can only decrease the size of either rect, never increase. > > Hmm, ofc you are right. I guess I'm just not that comfortable with the > concept of relaxed scaling. Yeah it's a bit of an open question how we should do things. The hardware constraints can be very complicated to describe, so I don't see any simple/sane way to report them to userspace. Which means writing simple hardware agnostic userspace code is pretty much impossible unless the kernel is very relaxed about what it accepts. OTOH if you want to be sure that the final picture will look exactly as specified, then you'd probably want an error instead. But I haven't really made up my mind on how we should handle those two cases. Some kind of control knob might be nice, but I've not yet figured out where the knob should live, and what kind of granularity it should have. And then we also have the problem that our errno based error reporting isn't really adequate for figuring out why something failed. Usually you just have to enable drm debugs, try again, and read through the log. > Dare I ask you to split patch 4 so that you > can convince me with a solid changelog? So you'd like me to implement strict checks first, and then relax them in a follow up patch? > > s/calculcated/calculated. Fixed. Thanks. -- Ville Syrj?l? Intel OTC
