| Message ID | 1409247613-14232-4-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Due to the upcoming atomic modesetting feature we need to separate > some update functions into a check step that can fail and a commit > step that should, ideally, never fail. > > This commit splits intel_update_plane() and its commit part can still > fail due to the fb pinning procedure. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++---------- > 1 file changed, 93 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 4cbe286..b1cb606 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) > return key.flags != I915_SET_COLORKEY_NONE; > } > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, get_visisble() is not a good name here IMO, also I think this split is at a slightly too low level. What we really want is to start creating some kind of plane config struct that can be passed to both check and commit, and then check can already store all the clipped coordinates etc. to the plane config and commit can just look them up w/o recomputing them. Initially you could just have one such struct on the stack in intel_update_plane() and pass it to both functions. Later we'll need to figure out how to pass around the plane configs for all planes during the nuclear flip, but there's no need to worry about such things quite yet. > + const struct drm_rect *clip, > + int min_scale, int max_scale) > +{ > + int hscale, vscale; > + > + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); > + BUG_ON(hscale < 0); > + > + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); > + BUG_ON(vscale < 0); > + > + return drm_rect_clip_scaled(src, dst, clip, hscale, vscale); > +} > + > static int > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > +intel_check_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, > struct drm_framebuffer *fb, int crtc_x, int crtc_y, > unsigned int crtc_w, unsigned int crtc_h, > uint32_t src_x, uint32_t src_y, > uint32_t src_w, uint32_t src_h) > { > - struct drm_device *dev = plane->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > - enum pipe pipe = intel_crtc->pipe; > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > struct drm_i915_gem_object *obj = intel_fb->obj; > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > - int ret; > - bool primary_enabled; > bool visible; > int hscale, vscale; > int max_scale, min_scale; > @@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, > .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, > }; > - const struct { > - int crtc_x, crtc_y; > - unsigned int crtc_w, crtc_h; > - uint32_t src_x, src_y, src_w, src_h; > - } orig = { > - .crtc_x = crtc_x, > - .crtc_y = crtc_y, > - .crtc_w = crtc_w, > - .crtc_h = crtc_h, > - .src_x = src_x, > - .src_y = src_y, > - .src_w = src_w, > - .src_h = src_h, > - }; > > /* Don't modify another pipe's plane */ > if (intel_plane->pipe != intel_crtc->pipe) { > @@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > drm_rect_rotate(&src, fb->width << 16, fb->height << 16, > intel_plane->rotation); > > - hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale); > - BUG_ON(hscale < 0); > - > - vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale); > - BUG_ON(vscale < 0); > - > - visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale); > + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); > > crtc_x = dst.x1; > crtc_y = dst.y1; > @@ -1027,6 +1017,54 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > } > } > > + return 0; > +} > + > +static int > +intel_commit_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, > + struct drm_framebuffer *fb, int crtc_x, int crtc_y, > + unsigned int crtc_w, unsigned int crtc_h, > + uint32_t src_x, uint32_t src_y, > + uint32_t src_w, uint32_t src_h) > +{ > + struct drm_device *dev = plane->dev; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_plane *intel_plane = to_intel_plane(plane); > + enum pipe pipe = intel_crtc->pipe; > + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > + struct drm_i915_gem_object *obj = intel_fb->obj; > + struct drm_i915_gem_object *old_obj = intel_plane->obj; > + int ret; > + bool primary_enabled; > + bool visible; > + int max_scale, min_scale; > + struct drm_rect src = { > + /* sample coordinates in 16.16 fixed point */ > + .x1 = src_x, > + .x2 = src_x + src_w, > + .y1 = src_y, > + .y2 = src_y + src_h, > + }; > + struct drm_rect dst = { > + /* integer pixels */ > + .x1 = crtc_x, > + .x2 = crtc_x + crtc_w, > + .y1 = crtc_y, > + .y2 = crtc_y + crtc_h, > + }; > + const struct drm_rect clip = { > + .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, > + .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, > + }; > + > + max_scale = intel_plane->max_downscale << 16; > + min_scale = intel_plane->can_scale ? 1 : (1 << 16); > + > + drm_rect_rotate(&src, fb->width << 16, fb->height << 16, > + intel_plane->rotation); > + > + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); > + > dst.x1 = crtc_x; > dst.x2 = crtc_x + crtc_w; > dst.y1 = crtc_y; > @@ -1055,14 +1093,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > if (ret) > return ret; > > - intel_plane->crtc_x = orig.crtc_x; > - intel_plane->crtc_y = orig.crtc_y; > - intel_plane->crtc_w = orig.crtc_w; > - intel_plane->crtc_h = orig.crtc_h; > - intel_plane->src_x = orig.src_x; > - intel_plane->src_y = orig.src_y; > - intel_plane->src_w = orig.src_w; > - intel_plane->src_h = orig.src_h; > + intel_plane->crtc_x = crtc_x; > + intel_plane->crtc_y = crtc_y; > + intel_plane->crtc_w = crtc_w; > + intel_plane->crtc_h = crtc_h; > + intel_plane->src_x = src_x; > + intel_plane->src_y = src_y; > + intel_plane->src_w = src_w; > + intel_plane->src_h = src_h; > intel_plane->obj = obj; > > if (intel_crtc->active) { > @@ -1109,6 +1147,26 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > } > > static int > +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > + struct drm_framebuffer *fb, int crtc_x, int crtc_y, > + unsigned int crtc_w, unsigned int crtc_h, > + uint32_t src_x, uint32_t src_y, > + uint32_t src_w, uint32_t src_h) > +{ > + int ret; > + > + ret = intel_check_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, > + crtc_w, crtc_h, src_x, src_y, > + src_w, src_h); > + if (ret) > + return ret; > + > + return intel_commit_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, > + crtc_w, crtc_h, src_x, src_y, > + src_w, src_h); > +} > + > +static int > intel_disable_plane(struct drm_plane *plane) > { > struct drm_device *dev = plane->dev; > -- > 1.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Aug 29, 2014 at 10:55:41AM +0300, Ville Syrjälä wrote: > On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > Due to the upcoming atomic modesetting feature we need to separate > > some update functions into a check step that can fail and a commit > > step that should, ideally, never fail. > > > > This commit splits intel_update_plane() and its commit part can still > > fail due to the fb pinning procedure. > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++---------- > > 1 file changed, 93 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 4cbe286..b1cb606 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) > > return key.flags != I915_SET_COLORKEY_NONE; > > } > > > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, > > get_visisble() is not a good name here IMO, also I think this split is at > a slightly too low level. What we really want is to start creating some > kind of plane config struct that can be passed to both check and commit, > and then check can already store all the clipped coordinates etc. to the > plane config and commit can just look them up w/o recomputing them. > > Initially you could just have one such struct on the stack in > intel_update_plane() and pass it to both functions. Later we'll need to > figure out how to pass around the plane configs for all planes during > the nuclear flip, but there's no need to worry about such things quite yet. To avoid too much rework I think it would be good to peak at the drm_plane_state structure in either Rob's or my atomic branch (iirc they're the same anyway), so that we can align as much as possible with the atomic interface. On the crtc side for full modesets we already have intel_crtc_config, which doesn't really fully align with drm_crtc_state. So lots of work required there. Otherwise I think Ville's plan to start out with an on-stack intel_plane_config sounds sane. -Daniel > > > + const struct drm_rect *clip, > > + int min_scale, int max_scale) > > +{ > > + int hscale, vscale; > > + > > + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); > > + BUG_ON(hscale < 0); > > + > > + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); > > + BUG_ON(vscale < 0); > > + > > + return drm_rect_clip_scaled(src, dst, clip, hscale, vscale); > > +} > > + > > static int > > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > +intel_check_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > struct drm_framebuffer *fb, int crtc_x, int crtc_y, > > unsigned int crtc_w, unsigned int crtc_h, > > uint32_t src_x, uint32_t src_y, > > uint32_t src_w, uint32_t src_h) > > { > > - struct drm_device *dev = plane->dev; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > struct intel_plane *intel_plane = to_intel_plane(plane); > > - enum pipe pipe = intel_crtc->pipe; > > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > > struct drm_i915_gem_object *obj = intel_fb->obj; > > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > > - int ret; > > - bool primary_enabled; > > bool visible; > > int hscale, vscale; > > int max_scale, min_scale; > > @@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, > > .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, > > }; > > - const struct { > > - int crtc_x, crtc_y; > > - unsigned int crtc_w, crtc_h; > > - uint32_t src_x, src_y, src_w, src_h; > > - } orig = { > > - .crtc_x = crtc_x, > > - .crtc_y = crtc_y, > > - .crtc_w = crtc_w, > > - .crtc_h = crtc_h, > > - .src_x = src_x, > > - .src_y = src_y, > > - .src_w = src_w, > > - .src_h = src_h, > > - }; > > > > /* Don't modify another pipe's plane */ > > if (intel_plane->pipe != intel_crtc->pipe) { > > @@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > drm_rect_rotate(&src, fb->width << 16, fb->height << 16, > > intel_plane->rotation); > > > > - hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale); > > - BUG_ON(hscale < 0); > > - > > - vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale); > > - BUG_ON(vscale < 0); > > - > > - visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale); > > + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); > > > > crtc_x = dst.x1; > > crtc_y = dst.y1; > > @@ -1027,6 +1017,54 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > } > > } > > > > + return 0; > > +} > > + > > +static int > > +intel_commit_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, int crtc_x, int crtc_y, > > + unsigned int crtc_w, unsigned int crtc_h, > > + uint32_t src_x, uint32_t src_y, > > + uint32_t src_w, uint32_t src_h) > > +{ > > + struct drm_device *dev = plane->dev; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + struct intel_plane *intel_plane = to_intel_plane(plane); > > + enum pipe pipe = intel_crtc->pipe; > > + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > > + struct drm_i915_gem_object *obj = intel_fb->obj; > > + struct drm_i915_gem_object *old_obj = intel_plane->obj; > > + int ret; > > + bool primary_enabled; > > + bool visible; > > + int max_scale, min_scale; > > + struct drm_rect src = { > > + /* sample coordinates in 16.16 fixed point */ > > + .x1 = src_x, > > + .x2 = src_x + src_w, > > + .y1 = src_y, > > + .y2 = src_y + src_h, > > + }; > > + struct drm_rect dst = { > > + /* integer pixels */ > > + .x1 = crtc_x, > > + .x2 = crtc_x + crtc_w, > > + .y1 = crtc_y, > > + .y2 = crtc_y + crtc_h, > > + }; > > + const struct drm_rect clip = { > > + .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, > > + .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, > > + }; > > + > > + max_scale = intel_plane->max_downscale << 16; > > + min_scale = intel_plane->can_scale ? 1 : (1 << 16); > > + > > + drm_rect_rotate(&src, fb->width << 16, fb->height << 16, > > + intel_plane->rotation); > > + > > + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); > > + > > dst.x1 = crtc_x; > > dst.x2 = crtc_x + crtc_w; > > dst.y1 = crtc_y; > > @@ -1055,14 +1093,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > if (ret) > > return ret; > > > > - intel_plane->crtc_x = orig.crtc_x; > > - intel_plane->crtc_y = orig.crtc_y; > > - intel_plane->crtc_w = orig.crtc_w; > > - intel_plane->crtc_h = orig.crtc_h; > > - intel_plane->src_x = orig.src_x; > > - intel_plane->src_y = orig.src_y; > > - intel_plane->src_w = orig.src_w; > > - intel_plane->src_h = orig.src_h; > > + intel_plane->crtc_x = crtc_x; > > + intel_plane->crtc_y = crtc_y; > > + intel_plane->crtc_w = crtc_w; > > + intel_plane->crtc_h = crtc_h; > > + intel_plane->src_x = src_x; > > + intel_plane->src_y = src_y; > > + intel_plane->src_w = src_w; > > + intel_plane->src_h = src_h; > > intel_plane->obj = obj; > > > > if (intel_crtc->active) { > > @@ -1109,6 +1147,26 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > } > > > > static int > > +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, int crtc_x, int crtc_y, > > + unsigned int crtc_w, unsigned int crtc_h, > > + uint32_t src_x, uint32_t src_y, > > + uint32_t src_w, uint32_t src_h) > > +{ > > + int ret; > > + > > + ret = intel_check_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, > > + crtc_w, crtc_h, src_x, src_y, > > + src_w, src_h); > > + if (ret) > > + return ret; > > + > > + return intel_commit_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, > > + crtc_w, crtc_h, src_x, src_y, > > + src_w, src_h); > > +} > > + > > +static int > > intel_disable_plane(struct drm_plane *plane) > > { > > struct drm_device *dev = plane->dev; > > -- > > 1.9.3 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2014-08-29 Ville Syrjälä <ville.syrjala@linux.intel.com>: > On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > Due to the upcoming atomic modesetting feature we need to separate > > some update functions into a check step that can fail and a commit > > step that should, ideally, never fail. > > > > This commit splits intel_update_plane() and its commit part can still > > fail due to the fb pinning procedure. > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++---------- > > 1 file changed, 93 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 4cbe286..b1cb606 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) > > return key.flags != I915_SET_COLORKEY_NONE; > > } > > > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, > > get_visisble() is not a good name here IMO, also I think this split is at > a slightly too low level. What we really want is to start creating some > kind of plane config struct that can be passed to both check and commit, > and then check can already store all the clipped coordinates etc. to the > plane config and commit can just look them up w/o recomputing them. What do you really mean by too low level here? Would grabbing struct drm_plane_state from the atomic branch fix this for you? I'll get a v2 of these patches working with struct drm_plane_state. Gustavo
On Fri, Aug 29, 2014 at 12:09:39PM -0300, Gustavo Padovan wrote: > 2014-08-29 Ville Syrjälä <ville.syrjala@linux.intel.com>: > > > On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote: > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > > > Due to the upcoming atomic modesetting feature we need to separate > > > some update functions into a check step that can fail and a commit > > > step that should, ideally, never fail. > > > > > > This commit splits intel_update_plane() and its commit part can still > > > fail due to the fb pinning procedure. > > > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > --- > > > drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++---------- > > > 1 file changed, 93 insertions(+), 35 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index 4cbe286..b1cb606 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) > > > return key.flags != I915_SET_COLORKEY_NONE; > > > } > > > > > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, > > > > get_visisble() is not a good name here IMO, also I think this split is at > > a slightly too low level. What we really want is to start creating some > > kind of plane config struct that can be passed to both check and commit, > > and then check can already store all the clipped coordinates etc. to the > > plane config and commit can just look them up w/o recomputing them. > > What do you really mean by too low level here? I mean you just roughtly cut it in half from the middle and duplicated a bit of the clipping logic in both halves. That's actually fairly broken since you seem to have left the extra src coordinate frobbing (align) that we do after clipping only to the check() part. So you'd need to yank all that extra code to the "get_visible" function as well. But with the plane config you can avoid doing all that work twice since check will do it and just pass the adjusted coordinates to commit. > Would grabbing > struct drm_plane_state from the atomic branch fix this for you? That looks like it's meant to house the user requested coordinates rather than the clipped ones. What you could do is just shovel the drm_rects we use during the clipping into a new i915 specific plane config struct and pass that to both check and commit. We can later look into moving that stuff into some core struct if seems like a win for more than one driver. > > I'll get a v2 of these patches working with struct drm_plane_state. > > Gustavo
On Fri, Aug 29, 2014 at 5:38 PM, Ville Syrjälä < ville.syrjala@linux.intel.com> wrote: > > Would grabbing > > struct drm_plane_state from the atomic branch fix this for you? > > That looks like it's meant to house the user requested coordinates > rather than the clipped ones. What you could do is just shovel the > drm_rects we use during the clipping into a new i915 specific plane > config struct and pass that to both check and commit. We can later > look into moving that stuff into some core struct if seems like a win > for more than one driver. Creating a new intel_plane_config will also be better for merging, since if you depend upon drm_plane_state directly your patch will be blocked. Ofc that means we need to do a bit of refactoring once atomic has landed, but that shouldn't be too onerous really. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 4cbe286..b1cb606 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) return key.flags != I915_SET_COLORKEY_NONE; } +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, + const struct drm_rect *clip, + int min_scale, int max_scale) +{ + int hscale, vscale; + + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); + BUG_ON(hscale < 0); + + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); + BUG_ON(vscale < 0); + + return drm_rect_clip_scaled(src, dst, clip, hscale, vscale); +} + static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, +intel_check_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, int crtc_x, int crtc_y, unsigned int crtc_w, unsigned int crtc_h, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h) { - struct drm_device *dev = plane->dev; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); - enum pipe pipe = intel_crtc->pipe; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; - struct drm_i915_gem_object *old_obj = intel_plane->obj; - int ret; - bool primary_enabled; bool visible; int hscale, vscale; int max_scale, min_scale; @@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, }; - const struct { - int crtc_x, crtc_y; - unsigned int crtc_w, crtc_h; - uint32_t src_x, src_y, src_w, src_h; - } orig = { - .crtc_x = crtc_x, - .crtc_y = crtc_y, - .crtc_w = crtc_w, - .crtc_h = crtc_h, - .src_x = src_x, - .src_y = src_y, - .src_w = src_w, - .src_h = src_h, - }; /* Don't modify another pipe's plane */ if (intel_plane->pipe != intel_crtc->pipe) { @@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, drm_rect_rotate(&src, fb->width << 16, fb->height << 16, intel_plane->rotation); - hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale); - BUG_ON(hscale < 0); - - vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale); - BUG_ON(vscale < 0); - - visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale); + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); crtc_x = dst.x1; crtc_y = dst.y1; @@ -1027,6 +1017,54 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, } } + return 0; +} + +static int +intel_commit_sprite_plane(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_device *dev = plane->dev; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_plane *intel_plane = to_intel_plane(plane); + enum pipe pipe = intel_crtc->pipe; + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); + struct drm_i915_gem_object *obj = intel_fb->obj; + struct drm_i915_gem_object *old_obj = intel_plane->obj; + int ret; + bool primary_enabled; + bool visible; + int max_scale, min_scale; + struct drm_rect src = { + /* sample coordinates in 16.16 fixed point */ + .x1 = src_x, + .x2 = src_x + src_w, + .y1 = src_y, + .y2 = src_y + src_h, + }; + struct drm_rect dst = { + /* integer pixels */ + .x1 = crtc_x, + .x2 = crtc_x + crtc_w, + .y1 = crtc_y, + .y2 = crtc_y + crtc_h, + }; + const struct drm_rect clip = { + .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, + .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, + }; + + max_scale = intel_plane->max_downscale << 16; + min_scale = intel_plane->can_scale ? 1 : (1 << 16); + + drm_rect_rotate(&src, fb->width << 16, fb->height << 16, + intel_plane->rotation); + + visible = get_visible(&src, &dst, &clip, max_scale, min_scale); + dst.x1 = crtc_x; dst.x2 = crtc_x + crtc_w; dst.y1 = crtc_y; @@ -1055,14 +1093,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret; - intel_plane->crtc_x = orig.crtc_x; - intel_plane->crtc_y = orig.crtc_y; - intel_plane->crtc_w = orig.crtc_w; - intel_plane->crtc_h = orig.crtc_h; - intel_plane->src_x = orig.src_x; - intel_plane->src_y = orig.src_y; - intel_plane->src_w = orig.src_w; - intel_plane->src_h = orig.src_h; + intel_plane->crtc_x = crtc_x; + intel_plane->crtc_y = crtc_y; + intel_plane->crtc_w = crtc_w; + intel_plane->crtc_h = crtc_h; + intel_plane->src_x = src_x; + intel_plane->src_y = src_y; + intel_plane->src_w = src_w; + intel_plane->src_h = src_h; intel_plane->obj = obj; if (intel_crtc->active) { @@ -1109,6 +1147,26 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, } static int +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + int ret; + + ret = intel_check_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, + crtc_w, crtc_h, src_x, src_y, + src_w, src_h); + if (ret) + return ret; + + return intel_commit_sprite_plane(plane, crtc, fb, crtc_x, crtc_y, + crtc_w, crtc_h, src_x, src_y, + src_w, src_h); +} + +static int intel_disable_plane(struct drm_plane *plane) { struct drm_device *dev = plane->dev;