[PATCH 1/4] drm/i915: Remove unnecessary test on the gen in intel_do_mmio_flip()

[PATCH 1/4] drm/i915: Remove unnecessary test on the gen in intel_do_mmio_flip()

[Damien Lespiau]

use_mmio_flip() makes sure we only enable MMIO flips on gen5+. So we
don't need to take into account older devices.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7d891e5..5133ef9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9567,12 +9567,11 @@ static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
 	reg = DSPCNTR(intel_crtc->plane);
 	dspcntr = I915_READ(reg);
 
-	if (INTEL_INFO(dev)->gen >= 4) {
-		if (obj->tiling_mode != I915_TILING_NONE)
-			dspcntr |= DISPPLANE_TILED;
-		else
-			dspcntr &= ~DISPPLANE_TILED;
-	}
+	if (obj->tiling_mode != I915_TILING_NONE)
+		dspcntr |= DISPPLANE_TILED;
+	else
+		dspcntr &= ~DISPPLANE_TILED;
+
 	I915_WRITE(reg, dspcntr);
 
 	I915_WRITE(DSPSURF(intel_crtc->plane),
-- 
1.8.3.1

[PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Damien Lespiau]

SKL will specialize it.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 627b7e7..d979549 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -492,6 +492,7 @@ struct drm_i915_display_funcs {
 			  struct drm_i915_gem_object *obj,
 			  struct intel_engine_cs *ring,
 			  uint32_t flags);
+	void (*do_mmio_flip)(struct intel_crtc *crtc);
 	void (*update_primary_plane)(struct drm_crtc *crtc,
 				     struct drm_framebuffer *fb,
 				     int x, int y);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5133ef9..07440ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9552,7 +9552,7 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
 		return ring != obj->ring;
 }
 
-static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
+static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -9561,9 +9561,6 @@ static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	u32 dspcntr;
 	u32 reg;
-
-	intel_mark_page_flip_active(intel_crtc);
-
 	reg = DSPCNTR(intel_crtc->plane);
 	dspcntr = I915_READ(reg);
 
@@ -9579,6 +9576,15 @@ static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
 	POSTING_READ(DSPSURF(intel_crtc->plane));
 }
 
+static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	intel_mark_page_flip_active(intel_crtc);
+	dev_priv->display.do_mmio_flip(intel_crtc);
+}
+
 static int intel_postpone_flip(struct drm_i915_gem_object *obj)
 {
 	struct intel_engine_cs *ring;
@@ -12694,6 +12700,9 @@ static void intel_init_display(struct drm_device *dev)
 		break;
 	}
 
+	if (INTEL_INFO(dev)->gen >= 5)
+		dev_priv->display.do_mmio_flip = ilk_do_mmio_flip;
+
 	intel_panel_init_backlight_funcs(dev);
 
 	mutex_init(&dev_priv->pps_mutex);
-- 
1.8.3.1

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Daniel Vetter]

On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> SKL will specialize it.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

So with atomic I'd expect that we'd have one giant mmio_flip driver
function and that just calls into the relevant platform/plane update
hooks. Gustavo&Ville are working on that monster.

Now this here seems to add another incarnation. Can't I have just one
please?

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 627b7e7..d979549 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -492,6 +492,7 @@ struct drm_i915_display_funcs {
>  			  struct drm_i915_gem_object *obj,
>  			  struct intel_engine_cs *ring,
>  			  uint32_t flags);
> +	void (*do_mmio_flip)(struct intel_crtc *crtc);
>  	void (*update_primary_plane)(struct drm_crtc *crtc,
>  				     struct drm_framebuffer *fb,
>  				     int x, int y);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5133ef9..07440ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9552,7 +9552,7 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
>  		return ring != obj->ring;
>  }
>  
> -static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
> +static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -9561,9 +9561,6 @@ static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	u32 dspcntr;
>  	u32 reg;
> -
> -	intel_mark_page_flip_active(intel_crtc);
> -
>  	reg = DSPCNTR(intel_crtc->plane);
>  	dspcntr = I915_READ(reg);
>  
> @@ -9579,6 +9576,15 @@ static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	POSTING_READ(DSPSURF(intel_crtc->plane));
>  }
>  
> +static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_device *dev = intel_crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_mark_page_flip_active(intel_crtc);
> +	dev_priv->display.do_mmio_flip(intel_crtc);
> +}
> +
>  static int intel_postpone_flip(struct drm_i915_gem_object *obj)
>  {
>  	struct intel_engine_cs *ring;
> @@ -12694,6 +12700,9 @@ static void intel_init_display(struct drm_device *dev)
>  		break;
>  	}
>  
> +	if (INTEL_INFO(dev)->gen >= 5)
> +		dev_priv->display.do_mmio_flip = ilk_do_mmio_flip;
> +
>  	intel_panel_init_backlight_funcs(dev);
>  
>  	mutex_init(&dev_priv->pps_mutex);
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Damien Lespiau]

On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
> On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> > SKL will specialize it.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> So with atomic I'd expect that we'd have one giant mmio_flip driver
> function and that just calls into the relevant platform/plane update
> hooks. Gustavo&Ville are working on that monster.
> 
> Now this here seems to add another incarnation. Can't I have just one
> please?

I'd be happy to have that, but are we talking before 3.19? SKL doesn't
flip at all without MMIO flips, so we'll need to have of of those by
3.19.

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Daniel Vetter]

On Mon, Oct 27, 2014 at 11:08:20AM +0000, Damien Lespiau wrote:
> On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
> > On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> > > SKL will specialize it.
> > > 
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > So with atomic I'd expect that we'd have one giant mmio_flip driver
> > function and that just calls into the relevant platform/plane update
> > hooks. Gustavo&Ville are working on that monster.
> > 
> > Now this here seems to add another incarnation. Can't I have just one
> > please?
> 
> I'd be happy to have that, but are we talking before 3.19? SKL doesn't
> flip at all without MMIO flips, so we'll need to have of of those by
> 3.19.

It shouldn't be a lot of fuzz really since it will more or less amount to
calling the same low-level plane update functions for all of them. Or I'm
confused about all this. Also I think Ander is starting to rework
mmio_flip this week.

Ville, Ander?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Ander Conselvan de Oliveira]

On 10/27/2014 04:25 PM, Daniel Vetter wrote:
> On Mon, Oct 27, 2014 at 11:08:20AM +0000, Damien Lespiau wrote:
>> On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
>>> On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
>>>> SKL will specialize it.
>>>>
>>>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>>>
>>> So with atomic I'd expect that we'd have one giant mmio_flip driver
>>> function and that just calls into the relevant platform/plane update
>>> hooks. Gustavo&Ville are working on that monster.
>>>
>>> Now this here seems to add another incarnation. Can't I have just one
>>> please?
>>
>> I'd be happy to have that, but are we talking before 3.19? SKL doesn't
>> flip at all without MMIO flips, so we'll need to have of of those by
>> 3.19.
>
> It shouldn't be a lot of fuzz really since it will more or less amount to
> calling the same low-level plane update functions for all of them. Or I'm
> confused about all this. Also I think Ander is starting to rework
> mmio_flip this week.

I did send a patch [1] related to mmio flip last week, but I wasn't 
aware of the grand plan for that "giant mmio_flip driver function". I'm 
learning the details as I go, so I should probably look into that, but 
at least for now I wasn't planning to much more than that patch.

Paulo gave me some comments (off-list, due to the mailman issues) that I 
plan to fix and then send a v2.

  [1] 
http://lists.freedesktop.org/archives/intel-gfx/2014-October/053792.html

Cheers,
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Ville Syrjälä]

On Mon, Oct 27, 2014 at 04:38:04PM +0200, Ander Conselvan de Oliveira wrote:
> On 10/27/2014 04:25 PM, Daniel Vetter wrote:
> > On Mon, Oct 27, 2014 at 11:08:20AM +0000, Damien Lespiau wrote:
> >> On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
> >>> On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> >>>> SKL will specialize it.
> >>>>
> >>>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> >>>
> >>> So with atomic I'd expect that we'd have one giant mmio_flip driver
> >>> function and that just calls into the relevant platform/plane update
> >>> hooks. Gustavo&Ville are working on that monster.
> >>>
> >>> Now this here seems to add another incarnation. Can't I have just one
> >>> please?
> >>
> >> I'd be happy to have that, but are we talking before 3.19? SKL doesn't
> >> flip at all without MMIO flips, so we'll need to have of of those by
> >> 3.19.
> >
> > It shouldn't be a lot of fuzz really since it will more or less amount to
> > calling the same low-level plane update functions for all of them. Or I'm
> > confused about all this. Also I think Ander is starting to rework
> > mmio_flip this week.
> 
> I did send a patch [1] related to mmio flip last week, but I wasn't 
> aware of the grand plan for that "giant mmio_flip driver function". I'm 
> learning the details as I go, so I should probably look into that, but 
> at least for now I wasn't planning to much more than that patch.
> 
> Paulo gave me some comments (off-list, due to the mailman issues) that I 
> plan to fix and then send a v2.
> 
>   [1] 
> http://lists.freedesktop.org/archives/intel-gfx/2014-October/053792.html

Yeah so this a step in the direction Daniel mentioned. The other part is
making the primary plane update func more sane, and split it to
check/commit phases, which is what Gustavo is working on.

Once all that's done we can add the vblank evade to the primary plane
update commit hook. And then there shouldn't be much difference in the mmio
flip vs. plane update anymore so we should be able to just call the primary
plane funcs for mmio flip, with the commit done from the wq.

And the end result is then quite close to what we want for nuclear flip.
Then we "just" expand it to cover multiple planes in one go and we're
almost there :)

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Damien Lespiau]

On Mon, Oct 27, 2014 at 04:47:53PM +0200, Ville Syrjälä wrote:
> On Mon, Oct 27, 2014 at 04:38:04PM +0200, Ander Conselvan de Oliveira wrote:
> > On 10/27/2014 04:25 PM, Daniel Vetter wrote:
> > > On Mon, Oct 27, 2014 at 11:08:20AM +0000, Damien Lespiau wrote:
> > >> On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
> > >>> On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> > >>>> SKL will specialize it.
> > >>>>
> > >>>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > >>>
> > >>> So with atomic I'd expect that we'd have one giant mmio_flip driver
> > >>> function and that just calls into the relevant platform/plane update
> > >>> hooks. Gustavo&Ville are working on that monster.
> > >>>
> > >>> Now this here seems to add another incarnation. Can't I have just one
> > >>> please?
> > >>
> > >> I'd be happy to have that, but are we talking before 3.19? SKL doesn't
> > >> flip at all without MMIO flips, so we'll need to have of of those by
> > >> 3.19.
> > >
> > > It shouldn't be a lot of fuzz really since it will more or less amount to
> > > calling the same low-level plane update functions for all of them. Or I'm
> > > confused about all this. Also I think Ander is starting to rework
> > > mmio_flip this week.
> > 
> > I did send a patch [1] related to mmio flip last week, but I wasn't 
> > aware of the grand plan for that "giant mmio_flip driver function". I'm 
> > learning the details as I go, so I should probably look into that, but 
> > at least for now I wasn't planning to much more than that patch.
> > 
> > Paulo gave me some comments (off-list, due to the mailman issues) that I 
> > plan to fix and then send a v2.
> > 
> >   [1] 
> > http://lists.freedesktop.org/archives/intel-gfx/2014-October/053792.html
> 
> Yeah so this a step in the direction Daniel mentioned. The other part is
> making the primary plane update func more sane, and split it to
> check/commit phases, which is what Gustavo is working on.
> 
> Once all that's done we can add the vblank evade to the primary plane
> update commit hook. And then there shouldn't be much difference in the mmio
> flip vs. plane update anymore so we should be able to just call the primary
> plane funcs for mmio flip, with the commit done from the wq.
> 
> And the end result is then quite close to what we want for nuclear flip.
> Then we "just" expand it to cover multiple planes in one go and we're
> almost there :)

Sounds like a wonderful future, but in the meantime, it'd be nice to
have flips working on SKL. Maybe with a comment on top of the vfunc
explaning it should be removed once we have MMIO updates for flips built
on top the plane vfuncs?

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Daniel Vetter]

On Mon, Oct 27, 2014 at 02:53:49PM +0000, Damien Lespiau wrote:
> On Mon, Oct 27, 2014 at 04:47:53PM +0200, Ville Syrjälä wrote:
> > On Mon, Oct 27, 2014 at 04:38:04PM +0200, Ander Conselvan de Oliveira wrote:
> > > On 10/27/2014 04:25 PM, Daniel Vetter wrote:
> > > > On Mon, Oct 27, 2014 at 11:08:20AM +0000, Damien Lespiau wrote:
> > > >> On Mon, Oct 27, 2014 at 10:16:06AM +0100, Daniel Vetter wrote:
> > > >>> On Sat, Oct 25, 2014 at 12:11:12AM +0100, Damien Lespiau wrote:
> > > >>>> SKL will specialize it.
> > > >>>>
> > > >>>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > > >>>
> > > >>> So with atomic I'd expect that we'd have one giant mmio_flip driver
> > > >>> function and that just calls into the relevant platform/plane update
> > > >>> hooks. Gustavo&Ville are working on that monster.
> > > >>>
> > > >>> Now this here seems to add another incarnation. Can't I have just one
> > > >>> please?
> > > >>
> > > >> I'd be happy to have that, but are we talking before 3.19? SKL doesn't
> > > >> flip at all without MMIO flips, so we'll need to have of of those by
> > > >> 3.19.
> > > >
> > > > It shouldn't be a lot of fuzz really since it will more or less amount to
> > > > calling the same low-level plane update functions for all of them. Or I'm
> > > > confused about all this. Also I think Ander is starting to rework
> > > > mmio_flip this week.
> > > 
> > > I did send a patch [1] related to mmio flip last week, but I wasn't 
> > > aware of the grand plan for that "giant mmio_flip driver function". I'm 
> > > learning the details as I go, so I should probably look into that, but 
> > > at least for now I wasn't planning to much more than that patch.
> > > 
> > > Paulo gave me some comments (off-list, due to the mailman issues) that I 
> > > plan to fix and then send a v2.
> > > 
> > >   [1] 
> > > http://lists.freedesktop.org/archives/intel-gfx/2014-October/053792.html
> > 
> > Yeah so this a step in the direction Daniel mentioned. The other part is
> > making the primary plane update func more sane, and split it to
> > check/commit phases, which is what Gustavo is working on.
> > 
> > Once all that's done we can add the vblank evade to the primary plane
> > update commit hook. And then there shouldn't be much difference in the mmio
> > flip vs. plane update anymore so we should be able to just call the primary
> > plane funcs for mmio flip, with the commit done from the wq.
> > 
> > And the end result is then quite close to what we want for nuclear flip.
> > Then we "just" expand it to cover multiple planes in one go and we're
> > almost there :)
> 
> Sounds like a wonderful future, but in the meantime, it'd be nice to
> have flips working on SKL. Maybe with a comment on top of the vfunc
> explaning it should be removed once we have MMIO updates for flips built
> on top the plane vfuncs?

Can't we copypaste the mmioflip for skl for now with a big comment stating
that this should all disappear? The current mmio flip is for gen4+ too
after all ...

I don't some temporary hacks too much, but going overboard with the design
with vfuncs and stuff seems like too much.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Introduce a vfunc for platform-specfic MMIO flip code

[Damien Lespiau]

On Tue, Oct 28, 2014 at 08:25:38AM +0100, Daniel Vetter wrote:
> Can't we copypaste the mmioflip for skl for now with a big comment stating
> that this should all disappear? The current mmio flip is for gen4+ too
> after all ...

We totally can!

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: Add space between variable declarations and code

[Damien Lespiau]

OCD kicks in again...

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 07440ad..dd071c6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9561,6 +9561,7 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	u32 dspcntr;
 	u32 reg;
+
 	reg = DSPCNTR(intel_crtc->plane);
 	dspcntr = I915_READ(reg);
 
-- 
1.8.3.1

Re: [PATCH 3/4] drm/i915: Add space between variable declarations and code

[Chris Wilson]

On Sat, Oct 25, 2014 at 12:11:13AM +0100, Damien Lespiau wrote:
> OCD kicks in again...
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 07440ad..dd071c6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9561,6 +9561,7 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	u32 dspcntr;
>  	u32 reg;
> +
>  	reg = DSPCNTR(intel_crtc->plane);

ocd would have moved it into the variable block and made it const. ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 3/4 v2] drm/i915: Add space between variable declarations and code

[Damien Lespiau]

OCD kicks in again...

v2: Chris out-OCDed me and asked for a const.

Signed-off-by: Damien Lespiau <damien.lespiau@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 07440ad..052c6ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9559,9 +9559,9 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
 	struct intel_framebuffer *intel_fb =
 		to_intel_framebuffer(intel_crtc->base.primary->fb);
 	struct drm_i915_gem_object *obj = intel_fb->obj;
+	const u32 reg = DSPCNTR(intel_crtc->plane);
 	u32 dspcntr;
-	u32 reg;
-	reg = DSPCNTR(intel_crtc->plane);
+
 	dspcntr = I915_READ(reg);
 
 	if (obj->tiling_mode != I915_TILING_NONE)
-- 
1.8.3.1

[PATCH 4/4] drm/i915/skl: Implement do_mmio_flip for SKL

[Damien Lespiau]

Nothing special to note. We mirror what is done for other platforms, but
using the SKL plane registers.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dd071c6..c09d009 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9552,6 +9552,28 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
 		return ring != obj->ring;
 }
 
+static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_framebuffer *intel_fb =
+		to_intel_framebuffer(intel_crtc->base.primary->fb);
+	struct drm_i915_gem_object *obj = intel_fb->obj;
+	const int pipe = intel_crtc->pipe;
+	u32 val;
+
+	val = I915_READ(PLANE_CTL(pipe, 0));
+
+	val &= ~PLANE_CTL_TILED_MASK;
+	if (obj->tiling_mode == I915_TILING_X)
+		val |= PLANE_CTL_TILED_X;
+
+	I915_WRITE(PLANE_CTL(pipe, 0), val);
+
+	I915_WRITE(PLANE_SURF(pipe, 0), intel_crtc->unpin_work->gtt_offset);
+	POSTING_READ(PLANE_SURF(pipe, 0));
+}
+
 static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -12701,7 +12723,9 @@ static void intel_init_display(struct drm_device *dev)
 		break;
 	}
 
-	if (INTEL_INFO(dev)->gen >= 5)
+	if (INTEL_INFO(dev)->gen >= 9)
+		dev_priv->display.do_mmio_flip = skl_do_mmio_flip;
+	else if (INTEL_INFO(dev)->gen >= 5)
 		dev_priv->display.do_mmio_flip = ilk_do_mmio_flip;
 
 	intel_panel_init_backlight_funcs(dev);
-- 
1.8.3.1

Re: [PATCH 4/4] drm/i915/skl: Implement do_mmio_flip for SKL

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=273/273->273/273
PNV: pass/total=271/271->271/271
ILK: pass/total=269/271->271/271
IVB: pass/total=271/271->271/271
SNB: pass/total=271/271->271/271
HSW: pass/total=271/271->271/271
BDW: pass/total=271/271->271/271
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly->result_with_patch_applied
ILK: Intel_gpu_tools, igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-forked, TIMEOUT->PASS
ILK: Intel_gpu_tools, igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-forked, TIMEOUT->PASS

Re: [PATCH 1/4] drm/i915: Remove unnecessary test on the gen in intel_do_mmio_flip()

[Chris Wilson]

On Sat, Oct 25, 2014 at 12:11:11AM +0100, Damien Lespiau wrote:
> use_mmio_flip() makes sure we only enable MMIO flips on gen5+. So we
> don't need to take into account older devices.

Honestly, I prefer to keep the gen test here (around touching gen
specifc portions of common registers). It stops it later biting
back if we relax mmioflip to run on older gen.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/4] drm/i915: Remove unnecessary test on the gen in intel_do_mmio_flip()

[Chris Wilson]

On Sat, Oct 25, 2014 at 10:01:12AM +0100, Chris Wilson wrote:
> On Sat, Oct 25, 2014 at 12:11:11AM +0100, Damien Lespiau wrote:
> > use_mmio_flip() makes sure we only enable MMIO flips on gen5+. So we
> > don't need to take into account older devices.
> 
> Honestly, I prefer to keep the gen test here (around touching gen
> specifc portions of common registers). It stops it later biting
> back if we relax mmioflip to run on older gen.

On the other hand, DSPSURF is gen4+. Ok, might as put a comment in there
that it is gen4+ specific and remove the check.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/4] drm/i915: Remove unnecessary test on the gen in intel_do_mmio_flip()

[Chris Wilson]

On Sat, Oct 25, 2014 at 12:11:11AM +0100, Damien Lespiau wrote:
> use_mmio_flip() makes sure we only enable MMIO flips on gen5+. So we
> don't need to take into account older devices.

Since the series accomplishes everything I just whined about in the
first patch, I guess I have to:

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre