| Message ID | 1416276639-30297-5-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 17, 2014 at 06:10:38PM -0800, Matt Roper wrote: > When using the universal plane interface, the source rectangle > coordinates define the panning offset for the primary plane, which needs > to be stored in crtc->{x,y}. The original universal plane code > negelected to set these panning offset fields, which was partially > remedied in: > > commit ccc759dc2a0214fd8b65ed4ebe78050874a67f94 > Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > Date: Wed Sep 24 14:20:22 2014 -0300 > > drm/i915: Merge of visible and !visible paths for primary planes > > However the plane source coordinates are provided in 16.16 fixed point > format and the above commit forgot to convert back to integer > coordinates before saving the values. When we replace > intel_pipe_set_base() with plane->funcs->update_plane() in a future > patch, this bug becomes visible via the set_config entrypoint as well as > update_plane. > > Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > Testcase: igt/kms_plane > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8dbc0e9..1c1886e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11668,8 +11668,8 @@ intel_commit_primary_plane(struct drm_plane *plane, > struct drm_rect *src = &state->src; > > crtc->primary->fb = fb; > - crtc->x = src->x1; > - crtc->y = src->y1; > + crtc->x = src->x1 >> 16; > + crtc->y = src->y1 >> 16; Oh I had the notion that we shift these down during the check phase. But I see we only do that for sprites. We should pick one approach and use it for all planes. I suppose keeping to the 16.16 format is the better option in anticipation of the day when we have planes which can do sub-pixel panning. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > intel_plane->crtc_x = state->orig_dst.x1; > intel_plane->crtc_y = state->orig_dst.y1; > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8dbc0e9..1c1886e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11668,8 +11668,8 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_rect *src = &state->src; crtc->primary->fb = fb; - crtc->x = src->x1; - crtc->y = src->y1; + crtc->x = src->x1 >> 16; + crtc->y = src->y1 >> 16; intel_plane->crtc_x = state->orig_dst.x1; intel_plane->crtc_y = state->orig_dst.y1;
When using the universal plane interface, the source rectangle coordinates define the panning offset for the primary plane, which needs to be stored in crtc->{x,y}. The original universal plane code negelected to set these panning offset fields, which was partially remedied in: commit ccc759dc2a0214fd8b65ed4ebe78050874a67f94 Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk> Date: Wed Sep 24 14:20:22 2014 -0300 drm/i915: Merge of visible and !visible paths for primary planes However the plane source coordinates are provided in 16.16 fixed point format and the above commit forgot to convert back to integer coordinates before saving the values. When we replace intel_pipe_set_base() with plane->funcs->update_plane() in a future patch, this bug becomes visible via the set_config entrypoint as well as update_plane. Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk> Testcase: igt/kms_plane Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)