* [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
@ 2015-10-20 14:54 Tvrtko Ursulin
2015-10-20 15:08 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2015-10-20 14:54 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Previously rotation was ignored and wrong stride programmed
into the plane registers resulting in a corrupt image on screen.
v2: Do not access potentialy old plane state at flip time,
but store the rotation value at the time of queing the flip.
(Ville)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1fc1d240d3c1..9cb29086971a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
}
static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
+ unsigned int rotation,
struct intel_unpin_work *work)
{
struct drm_device *dev = intel_crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
const enum pipe pipe = intel_crtc->pipe;
- u32 ctl, stride;
+ u32 ctl, stride, tile_height;
ctl = I915_READ(PLANE_CTL(pipe, 0));
ctl &= ~PLANE_CTL_TILED_MASK;
@@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
* The stride is either expressed as a multiple of 64 bytes chunks for
* linear buffers or in number of tiles for tiled buffers.
*/
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev, fb->modifier[0],
- fb->pixel_format);
+ if (intel_rotation_90_or_270(rotation)) {
+ /* stride = Surface height in tiles */
+ tile_height = intel_tile_height(dev, fb->pixel_format,
+ fb->modifier[0], 0);
+ stride = DIV_ROUND_UP(fb->height, tile_height);
+ } else {
+ stride = fb->pitches[0] /
+ intel_fb_stride_alignment(dev, fb->modifier[0],
+ fb->pixel_format);
+ }
/*
* Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
@@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
intel_pipe_update_start(crtc);
if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
- skl_do_mmio_flip(crtc, work);
+ skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
else
/* use_mmio_flip() retricts MMIO flips to ilk+ */
ilk_do_mmio_flip(crtc, work);
@@ -11208,10 +11216,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
static int intel_queue_mmio_flip(struct drm_device *dev,
struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring,
- uint32_t flags)
+ unsigned int rotation,
+ struct drm_i915_gem_object *obj)
{
struct intel_mmio_flip *mmio_flip;
@@ -11222,6 +11228,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
mmio_flip->i915 = to_i915(dev);
mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
mmio_flip->crtc = to_intel_crtc(crtc);
+ mmio_flip->rotation = rotation;
INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
schedule_work(&mmio_flip->work);
@@ -11438,8 +11445,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->gtt_offset += intel_crtc->dspaddr_offset;
if (mmio_flip) {
- ret = intel_queue_mmio_flip(dev, crtc, fb, obj, ring,
- page_flip_flags);
+ ret = intel_queue_mmio_flip(dev, crtc,
+ crtc->primary->state->rotation,
+ obj);
if (ret)
goto cleanup_unpin;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 27dccf35f9bc..140f949302c7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -493,6 +493,7 @@ struct intel_mmio_flip {
struct drm_i915_private *i915;
struct drm_i915_gem_request *req;
struct intel_crtc *crtc;
+ unsigned int rotation;
};
struct skl_pipe_wm {
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
2015-10-20 14:54 [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip Tvrtko Ursulin
@ 2015-10-20 15:08 ` Ville Syrjälä
2015-10-20 15:11 ` Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2015-10-20 15:08 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Tue, Oct 20, 2015 at 03:54:11PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Previously rotation was ignored and wrong stride programmed
> into the plane registers resulting in a corrupt image on screen.
>
> v2: Do not access potentialy old plane state at flip time,
> but store the rotation value at the time of queing the flip.
> (Ville)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1fc1d240d3c1..9cb29086971a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
> }
>
> static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> + unsigned int rotation,
> struct intel_unpin_work *work)
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> const enum pipe pipe = intel_crtc->pipe;
> - u32 ctl, stride;
> + u32 ctl, stride, tile_height;
>
> ctl = I915_READ(PLANE_CTL(pipe, 0));
> ctl &= ~PLANE_CTL_TILED_MASK;
> @@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> * The stride is either expressed as a multiple of 64 bytes chunks for
> * linear buffers or in number of tiles for tiled buffers.
> */
> - stride = fb->pitches[0] /
> - intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + if (intel_rotation_90_or_270(rotation)) {
> + /* stride = Surface height in tiles */
> + tile_height = intel_tile_height(dev, fb->pixel_format,
> + fb->modifier[0], 0);
> + stride = DIV_ROUND_UP(fb->height, tile_height);
> + } else {
> + stride = fb->pitches[0] /
> + intel_fb_stride_alignment(dev, fb->modifier[0],
> + fb->pixel_format);
> + }
>
> /*
> * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> @@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> intel_pipe_update_start(crtc);
>
> if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> - skl_do_mmio_flip(crtc, work);
> + skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> else
> /* use_mmio_flip() retricts MMIO flips to ilk+ */
> ilk_do_mmio_flip(crtc, work);
> @@ -11208,10 +11216,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
>
> static int intel_queue_mmio_flip(struct drm_device *dev,
> struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - struct drm_i915_gem_object *obj,
> - struct intel_engine_cs *ring,
> - uint32_t flags)
> + unsigned int rotation,
> + struct drm_i915_gem_object *obj)
> {
> struct intel_mmio_flip *mmio_flip;
>
> @@ -11222,6 +11228,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> mmio_flip->i915 = to_i915(dev);
> mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> mmio_flip->crtc = to_intel_crtc(crtc);
> + mmio_flip->rotation = rotation;
Could just do the crtc->primary->state->rotation thing here and avoid
having to pass the rotation from the caller.
But the patch looks OK anyway:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
> schedule_work(&mmio_flip->work);
> @@ -11438,8 +11445,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->gtt_offset += intel_crtc->dspaddr_offset;
>
> if (mmio_flip) {
> - ret = intel_queue_mmio_flip(dev, crtc, fb, obj, ring,
> - page_flip_flags);
> + ret = intel_queue_mmio_flip(dev, crtc,
> + crtc->primary->state->rotation,
> + obj);
> if (ret)
> goto cleanup_unpin;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 27dccf35f9bc..140f949302c7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -493,6 +493,7 @@ struct intel_mmio_flip {
> struct drm_i915_private *i915;
> struct drm_i915_gem_request *req;
> struct intel_crtc *crtc;
> + unsigned int rotation;
> };
>
> struct skl_pipe_wm {
> --
> 1.9.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
2015-10-20 15:08 ` Ville Syrjälä
@ 2015-10-20 15:11 ` Tvrtko Ursulin
2015-10-20 15:17 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2015-10-20 15:11 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On 20/10/15 16:08, Ville Syrjälä wrote:
> On Tue, Oct 20, 2015 at 03:54:11PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Previously rotation was ignored and wrong stride programmed
>> into the plane registers resulting in a corrupt image on screen.
>>
>> v2: Do not access potentialy old plane state at flip time,
>> but store the rotation value at the time of queing the flip.
>> (Ville)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++-----------
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> 2 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1fc1d240d3c1..9cb29086971a 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
>> }
>>
>> static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
>> + unsigned int rotation,
>> struct intel_unpin_work *work)
>> {
>> struct drm_device *dev = intel_crtc->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
>> const enum pipe pipe = intel_crtc->pipe;
>> - u32 ctl, stride;
>> + u32 ctl, stride, tile_height;
>>
>> ctl = I915_READ(PLANE_CTL(pipe, 0));
>> ctl &= ~PLANE_CTL_TILED_MASK;
>> @@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
>> * The stride is either expressed as a multiple of 64 bytes chunks for
>> * linear buffers or in number of tiles for tiled buffers.
>> */
>> - stride = fb->pitches[0] /
>> - intel_fb_stride_alignment(dev, fb->modifier[0],
>> - fb->pixel_format);
>> + if (intel_rotation_90_or_270(rotation)) {
>> + /* stride = Surface height in tiles */
>> + tile_height = intel_tile_height(dev, fb->pixel_format,
>> + fb->modifier[0], 0);
>> + stride = DIV_ROUND_UP(fb->height, tile_height);
>> + } else {
>> + stride = fb->pitches[0] /
>> + intel_fb_stride_alignment(dev, fb->modifier[0],
>> + fb->pixel_format);
>> + }
>>
>> /*
>> * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
>> @@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
>> intel_pipe_update_start(crtc);
>>
>> if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
>> - skl_do_mmio_flip(crtc, work);
>> + skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
>> else
>> /* use_mmio_flip() retricts MMIO flips to ilk+ */
>> ilk_do_mmio_flip(crtc, work);
>> @@ -11208,10 +11216,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
>>
>> static int intel_queue_mmio_flip(struct drm_device *dev,
>> struct drm_crtc *crtc,
>> - struct drm_framebuffer *fb,
>> - struct drm_i915_gem_object *obj,
>> - struct intel_engine_cs *ring,
>> - uint32_t flags)
>> + unsigned int rotation,
>> + struct drm_i915_gem_object *obj)
>> {
>> struct intel_mmio_flip *mmio_flip;
>>
>> @@ -11222,6 +11228,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
>> mmio_flip->i915 = to_i915(dev);
>> mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
>> mmio_flip->crtc = to_intel_crtc(crtc);
>> + mmio_flip->rotation = rotation;
>
> Could just do the crtc->primary->state->rotation thing here and avoid
> having to pass the rotation from the caller.
Oh that would indeed be nicer. I'll do a v3.
> But the patch looks OK anyway:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Can I carry over the r-b to v3?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
2015-10-20 15:11 ` Tvrtko Ursulin
@ 2015-10-20 15:17 ` Ville Syrjälä
2015-10-20 15:20 ` [PATCH v3] " Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2015-10-20 15:17 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Tue, Oct 20, 2015 at 04:11:34PM +0100, Tvrtko Ursulin wrote:
>
> On 20/10/15 16:08, Ville Syrjälä wrote:
> > On Tue, Oct 20, 2015 at 03:54:11PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Previously rotation was ignored and wrong stride programmed
> >> into the plane registers resulting in a corrupt image on screen.
> >>
> >> v2: Do not access potentialy old plane state at flip time,
> >> but store the rotation value at the time of queing the flip.
> >> (Ville)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
> >> Cc: Sonika Jindal <sonika.jindal@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++-----------
> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
> >> 2 files changed, 20 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 1fc1d240d3c1..9cb29086971a 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
> >> }
> >>
> >> static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> >> + unsigned int rotation,
> >> struct intel_unpin_work *work)
> >> {
> >> struct drm_device *dev = intel_crtc->base.dev;
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> >> const enum pipe pipe = intel_crtc->pipe;
> >> - u32 ctl, stride;
> >> + u32 ctl, stride, tile_height;
> >>
> >> ctl = I915_READ(PLANE_CTL(pipe, 0));
> >> ctl &= ~PLANE_CTL_TILED_MASK;
> >> @@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> >> * The stride is either expressed as a multiple of 64 bytes chunks for
> >> * linear buffers or in number of tiles for tiled buffers.
> >> */
> >> - stride = fb->pitches[0] /
> >> - intel_fb_stride_alignment(dev, fb->modifier[0],
> >> - fb->pixel_format);
> >> + if (intel_rotation_90_or_270(rotation)) {
> >> + /* stride = Surface height in tiles */
> >> + tile_height = intel_tile_height(dev, fb->pixel_format,
> >> + fb->modifier[0], 0);
> >> + stride = DIV_ROUND_UP(fb->height, tile_height);
> >> + } else {
> >> + stride = fb->pitches[0] /
> >> + intel_fb_stride_alignment(dev, fb->modifier[0],
> >> + fb->pixel_format);
> >> + }
> >>
> >> /*
> >> * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> >> @@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> >> intel_pipe_update_start(crtc);
> >>
> >> if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> >> - skl_do_mmio_flip(crtc, work);
> >> + skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> >> else
> >> /* use_mmio_flip() retricts MMIO flips to ilk+ */
> >> ilk_do_mmio_flip(crtc, work);
> >> @@ -11208,10 +11216,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
> >>
> >> static int intel_queue_mmio_flip(struct drm_device *dev,
> >> struct drm_crtc *crtc,
> >> - struct drm_framebuffer *fb,
> >> - struct drm_i915_gem_object *obj,
> >> - struct intel_engine_cs *ring,
> >> - uint32_t flags)
> >> + unsigned int rotation,
> >> + struct drm_i915_gem_object *obj)
> >> {
> >> struct intel_mmio_flip *mmio_flip;
> >>
> >> @@ -11222,6 +11228,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> >> mmio_flip->i915 = to_i915(dev);
> >> mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> >> mmio_flip->crtc = to_intel_crtc(crtc);
> >> + mmio_flip->rotation = rotation;
> >
> > Could just do the crtc->primary->state->rotation thing here and avoid
> > having to pass the rotation from the caller.
>
> Oh that would indeed be nicer. I'll do a v3.
>
> > But the patch looks OK anyway:
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Can I carry over the r-b to v3?
Yes.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
2015-10-20 15:17 ` Ville Syrjälä
@ 2015-10-20 15:20 ` Tvrtko Ursulin
2015-10-21 6:51 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2015-10-20 15:20 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Previously rotation was ignored and wrong stride programmed
into the plane registers resulting in a corrupt image on screen.
v2: Do not access potentialy old plane state at flip time,
but store the rotation value at the time of queing the flip.
(Ville)
v3: No need to pass rotation to intel_queue_mmio_flip since it
is available in the crtc. (Ville)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1fc1d240d3c1..2981d09d5e4e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
}
static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
+ unsigned int rotation,
struct intel_unpin_work *work)
{
struct drm_device *dev = intel_crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
const enum pipe pipe = intel_crtc->pipe;
- u32 ctl, stride;
+ u32 ctl, stride, tile_height;
ctl = I915_READ(PLANE_CTL(pipe, 0));
ctl &= ~PLANE_CTL_TILED_MASK;
@@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
* The stride is either expressed as a multiple of 64 bytes chunks for
* linear buffers or in number of tiles for tiled buffers.
*/
- stride = fb->pitches[0] /
- intel_fb_stride_alignment(dev, fb->modifier[0],
- fb->pixel_format);
+ if (intel_rotation_90_or_270(rotation)) {
+ /* stride = Surface height in tiles */
+ tile_height = intel_tile_height(dev, fb->pixel_format,
+ fb->modifier[0], 0);
+ stride = DIV_ROUND_UP(fb->height, tile_height);
+ } else {
+ stride = fb->pitches[0] /
+ intel_fb_stride_alignment(dev, fb->modifier[0],
+ fb->pixel_format);
+ }
/*
* Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
@@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
intel_pipe_update_start(crtc);
if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
- skl_do_mmio_flip(crtc, work);
+ skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
else
/* use_mmio_flip() retricts MMIO flips to ilk+ */
ilk_do_mmio_flip(crtc, work);
@@ -11208,10 +11216,7 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
static int intel_queue_mmio_flip(struct drm_device *dev,
struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring,
- uint32_t flags)
+ struct drm_i915_gem_object *obj)
{
struct intel_mmio_flip *mmio_flip;
@@ -11222,6 +11227,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
mmio_flip->i915 = to_i915(dev);
mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
mmio_flip->crtc = to_intel_crtc(crtc);
+ mmio_flip->rotation = crtc->primary->state->rotation;
INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
schedule_work(&mmio_flip->work);
@@ -11438,8 +11444,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->gtt_offset += intel_crtc->dspaddr_offset;
if (mmio_flip) {
- ret = intel_queue_mmio_flip(dev, crtc, fb, obj, ring,
- page_flip_flags);
+ ret = intel_queue_mmio_flip(dev, crtc, obj);
if (ret)
goto cleanup_unpin;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 27dccf35f9bc..140f949302c7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -493,6 +493,7 @@ struct intel_mmio_flip {
struct drm_i915_private *i915;
struct drm_i915_gem_request *req;
struct intel_crtc *crtc;
+ unsigned int rotation;
};
struct skl_pipe_wm {
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip
2015-10-20 15:20 ` [PATCH v3] " Tvrtko Ursulin
@ 2015-10-21 6:51 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-10-21 6:51 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Tue, Oct 20, 2015 at 04:20:21PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Previously rotation was ignored and wrong stride programmed
> into the plane registers resulting in a corrupt image on screen.
>
> v2: Do not access potentialy old plane state at flip time,
> but store the rotation value at the time of queing the flip.
> (Ville)
>
> v3: No need to pass rotation to intel_queue_mmio_flip since it
> is available in the crtc. (Ville)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Testcase: igt/kms_rotation_crc/primary-rotation-90-flip-stress (SKL)
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1fc1d240d3c1..2981d09d5e4e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11091,13 +11091,14 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
> }
>
> static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> + unsigned int rotation,
> struct intel_unpin_work *work)
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> const enum pipe pipe = intel_crtc->pipe;
> - u32 ctl, stride;
> + u32 ctl, stride, tile_height;
>
> ctl = I915_READ(PLANE_CTL(pipe, 0));
> ctl &= ~PLANE_CTL_TILED_MASK;
> @@ -11121,9 +11122,16 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> * The stride is either expressed as a multiple of 64 bytes chunks for
> * linear buffers or in number of tiles for tiled buffers.
> */
> - stride = fb->pitches[0] /
> - intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + if (intel_rotation_90_or_270(rotation)) {
> + /* stride = Surface height in tiles */
> + tile_height = intel_tile_height(dev, fb->pixel_format,
> + fb->modifier[0], 0);
> + stride = DIV_ROUND_UP(fb->height, tile_height);
> + } else {
> + stride = fb->pitches[0] /
> + intel_fb_stride_alignment(dev, fb->modifier[0],
> + fb->pixel_format);
> + }
>
> /*
> * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> @@ -11181,7 +11189,7 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> intel_pipe_update_start(crtc);
>
> if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> - skl_do_mmio_flip(crtc, work);
> + skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> else
> /* use_mmio_flip() retricts MMIO flips to ilk+ */
> ilk_do_mmio_flip(crtc, work);
> @@ -11208,10 +11216,7 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
>
> static int intel_queue_mmio_flip(struct drm_device *dev,
> struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - struct drm_i915_gem_object *obj,
> - struct intel_engine_cs *ring,
> - uint32_t flags)
> + struct drm_i915_gem_object *obj)
> {
> struct intel_mmio_flip *mmio_flip;
>
> @@ -11222,6 +11227,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> mmio_flip->i915 = to_i915(dev);
> mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> mmio_flip->crtc = to_intel_crtc(crtc);
> + mmio_flip->rotation = crtc->primary->state->rotation;
>
> INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
> schedule_work(&mmio_flip->work);
> @@ -11438,8 +11444,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->gtt_offset += intel_crtc->dspaddr_offset;
>
> if (mmio_flip) {
> - ret = intel_queue_mmio_flip(dev, crtc, fb, obj, ring,
> - page_flip_flags);
> + ret = intel_queue_mmio_flip(dev, crtc, obj);
> if (ret)
> goto cleanup_unpin;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 27dccf35f9bc..140f949302c7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -493,6 +493,7 @@ struct intel_mmio_flip {
> struct drm_i915_private *i915;
> struct drm_i915_gem_request *req;
> struct intel_crtc *crtc;
> + unsigned int rotation;
> };
>
> struct skl_pipe_wm {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-21 6:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-20 14:54 [PATCH v2] drm/i915/skl: Consider plane rotation when calculating stride in skl_do_mmio_flip Tvrtko Ursulin
2015-10-20 15:08 ` Ville Syrjälä
2015-10-20 15:11 ` Tvrtko Ursulin
2015-10-20 15:17 ` Ville Syrjälä
2015-10-20 15:20 ` [PATCH v3] " Tvrtko Ursulin
2015-10-21 6:51 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox