linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix screen flickering on X
@ 2015-04-11 22:40 Ismael Luceno
  2015-04-23  9:15 ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Ismael Luceno @ 2015-04-11 22:40 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ismael Luceno

A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
introduced the issue.

The issue starts as soon as X takes control of the screen, even if just
a plain X doing nothing, so based on the code touched by the commit I
thought it had to be related to the so called "hardware cursor". I
confirmed it when hiding the cursor made the flickering go away.

The aforementioned commit removed some suspicious code, and the
Programmer's Reference Manual confirmed my suspicion:

"Incorrectly programmed watermark values can result in screen corruption.

The watermarks should be calculated and programmed when any of the
watermark calculation inputs change. This includes planes enabling or
disabling, plane source format or size changing, etc."

So I'm re-adding the few lines that update the watermarks after a cursor
size change.

Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f75173c..e23f062 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+	unsigned old_width;
 	uint32_t addr;
 
 	crtc = crtc ? crtc : plane->crtc;
@@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	intel_crtc->cursor_addr = addr;
 	intel_crtc->cursor_bo = obj;
 update:
+	old_width = intel_crtc->cursor_width;
 	intel_crtc->cursor_width = state->base.crtc_w;
 	intel_crtc->cursor_height = state->base.crtc_h;
 
-	if (intel_crtc->active)
+	if (intel_crtc->active) {
+		if (old_width != intel_crtc->cursor_width)
+			intel_update_watermarks(crtc);
 		intel_crtc_update_cursor(crtc, state->visible);
+	}
 }
 
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
-- 
2.3.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Fix screen flickering on X
  2015-04-11 22:40 [PATCH] drm/i915: Fix screen flickering on X Ismael Luceno
@ 2015-04-23  9:15 ` Chris Wilson
  2015-05-07  9:12   ` [Intel-gfx] " Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2015-04-23  9:15 UTC (permalink / raw)
  To: Matt Roper; +Cc: linux-kernel, intel-gfx

[cc'ing the authors]

On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote:
> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
> introduced the issue.
> 
> The issue starts as soon as X takes control of the screen, even if just
> a plain X doing nothing, so based on the code touched by the commit I
> thought it had to be related to the so called "hardware cursor". I
> confirmed it when hiding the cursor made the flickering go away.
> 
> The aforementioned commit removed some suspicious code, and the
> Programmer's Reference Manual confirmed my suspicion:
> 
> "Incorrectly programmed watermark values can result in screen corruption.
> 
> The watermarks should be calculated and programmed when any of the
> watermark calculation inputs change. This includes planes enabling or
> disabling, plane source format or size changing, etc."
> 
> So I'm re-adding the few lines that update the watermarks after a cursor
> size change.
> 
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f75173c..e23f062 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  	struct intel_crtc *intel_crtc;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> +	unsigned old_width;
>  	uint32_t addr;
>  
>  	crtc = crtc ? crtc : plane->crtc;
> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  	intel_crtc->cursor_addr = addr;
>  	intel_crtc->cursor_bo = obj;
>  update:
> +	old_width = intel_crtc->cursor_width;
>  	intel_crtc->cursor_width = state->base.crtc_w;
>  	intel_crtc->cursor_height = state->base.crtc_h;
>  
> -	if (intel_crtc->active)
> +	if (intel_crtc->active) {
> +		if (old_width != intel_crtc->cursor_width)
> +			intel_update_watermarks(crtc);
>  		intel_crtc_update_cursor(crtc, state->visible);
> +	}
>  }
>  
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> -- 
> 2.3.4

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-04-23  9:15 ` Chris Wilson
@ 2015-05-07  9:12   ` Jani Nikula
  2015-05-07 13:27     ` Matt Roper
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2015-05-07  9:12 UTC (permalink / raw)
  To: Chris Wilson, Matt Roper; +Cc: intel-gfx, linux-kernel

On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> [cc'ing the authors]

This has been posted earlier [1] and it has review to be addressed [2].

BR,
Jani.


[1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ismael@iodev.co.uk
[2] http://mid.gmane.org/1428928418.2654.8.camel@gmail.com


>
> On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote:
>> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
>> introduced the issue.
>> 
>> The issue starts as soon as X takes control of the screen, even if just
>> a plain X doing nothing, so based on the code touched by the commit I
>> thought it had to be related to the so called "hardware cursor". I
>> confirmed it when hiding the cursor made the flickering go away.
>> 
>> The aforementioned commit removed some suspicious code, and the
>> Programmer's Reference Manual confirmed my suspicion:
>> 
>> "Incorrectly programmed watermark values can result in screen corruption.
>> 
>> The watermarks should be calculated and programmed when any of the
>> watermark calculation inputs change. This includes planes enabling or
>> disabling, plane source format or size changing, etc."
>> 
>> So I'm re-adding the few lines that update the watermarks after a cursor
>> size change.
>> 
>> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index f75173c..e23f062 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>>  	struct intel_crtc *intel_crtc;
>>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>> +	unsigned old_width;
>>  	uint32_t addr;
>>  
>>  	crtc = crtc ? crtc : plane->crtc;
>> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>>  	intel_crtc->cursor_addr = addr;
>>  	intel_crtc->cursor_bo = obj;
>>  update:
>> +	old_width = intel_crtc->cursor_width;
>>  	intel_crtc->cursor_width = state->base.crtc_w;
>>  	intel_crtc->cursor_height = state->base.crtc_h;
>>  
>> -	if (intel_crtc->active)
>> +	if (intel_crtc->active) {
>> +		if (old_width != intel_crtc->cursor_width)
>> +			intel_update_watermarks(crtc);
>>  		intel_crtc_update_cursor(crtc, state->visible);
>> +	}
>>  }
>>  
>>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>> -- 
>> 2.3.4
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-07  9:12   ` [Intel-gfx] " Jani Nikula
@ 2015-05-07 13:27     ` Matt Roper
  2015-05-07 13:41       ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Matt Roper @ 2015-05-07 13:27 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Chris Wilson, intel-gfx, linux-kernel

On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > [cc'ing the authors]
> 
> This has been posted earlier [1] and it has review to be addressed [2].
> 
> BR,
> Jani.

I agree with Ander's response in [2]...we can't call
intel_update_watermarks() in the commit function because we're under
vblank evasion.  We should already be flagging the transaction as
needing a watermark update in intel_check_cursor_plane(), and that flag
will be acted upon immediately after the commit functions are done
running, once we've re-enabled interrupts.

Note that our current codebase looks a bit different since we've dropped
intel_crtc->cursor_{width,height}.  So the relevant check in
intel_check_cursor_plane() now looks like:

        if (plane->state->crtc_w != state->base.crtc_w)
                intel_crtc->atomic.update_wm = true;

Is there a bugzilla open on this issue with more details?


Matt

> 
> 
> [1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ismael@iodev.co.uk
> [2] http://mid.gmane.org/1428928418.2654.8.camel@gmail.com
> 
> 
> >
> > On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote:
> >> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
> >> introduced the issue.
> >> 
> >> The issue starts as soon as X takes control of the screen, even if just
> >> a plain X doing nothing, so based on the code touched by the commit I
> >> thought it had to be related to the so called "hardware cursor". I
> >> confirmed it when hiding the cursor made the flickering go away.
> >> 
> >> The aforementioned commit removed some suspicious code, and the
> >> Programmer's Reference Manual confirmed my suspicion:
> >> 
> >> "Incorrectly programmed watermark values can result in screen corruption.
> >> 
> >> The watermarks should be calculated and programmed when any of the
> >> watermark calculation inputs change. This includes planes enabling or
> >> disabling, plane source format or size changing, etc."
> >> 
> >> So I'm re-adding the few lines that update the watermarks after a cursor
> >> size change.
> >> 
> >> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index f75173c..e23f062 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> >>  	struct intel_crtc *intel_crtc;
> >>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> >>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> >> +	unsigned old_width;
> >>  	uint32_t addr;
> >>  
> >>  	crtc = crtc ? crtc : plane->crtc;
> >> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> >>  	intel_crtc->cursor_addr = addr;
> >>  	intel_crtc->cursor_bo = obj;
> >>  update:
> >> +	old_width = intel_crtc->cursor_width;
> >>  	intel_crtc->cursor_width = state->base.crtc_w;
> >>  	intel_crtc->cursor_height = state->base.crtc_h;
> >>  
> >> -	if (intel_crtc->active)
> >> +	if (intel_crtc->active) {
> >> +		if (old_width != intel_crtc->cursor_width)
> >> +			intel_update_watermarks(crtc);
> >>  		intel_crtc_update_cursor(crtc, state->visible);
> >> +	}
> >>  }
> >>  
> >>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> >> -- 
> >> 2.3.4
> >
> > -- 
> > Chris Wilson, Intel Open Source Technology Centre
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-07 13:27     ` Matt Roper
@ 2015-05-07 13:41       ` Jani Nikula
  2015-05-07 14:18         ` Chris Wilson
  2015-05-08 15:10         ` Ismael Luceno
  0 siblings, 2 replies; 9+ messages in thread
From: Jani Nikula @ 2015-05-07 13:41 UTC (permalink / raw)
  To: Matt Roper, Ismael Luceno; +Cc: Chris Wilson, intel-gfx, linux-kernel

On Thu, 07 May 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
>> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > [cc'ing the authors]
>> 
>> This has been posted earlier [1] and it has review to be addressed [2].
>> 
>> BR,
>> Jani.
>
> I agree with Ander's response in [2]...we can't call
> intel_update_watermarks() in the commit function because we're under
> vblank evasion.  We should already be flagging the transaction as
> needing a watermark update in intel_check_cursor_plane(), and that flag
> will be acted upon immediately after the commit functions are done
> running, once we've re-enabled interrupts.
>
> Note that our current codebase looks a bit different since we've dropped
> intel_crtc->cursor_{width,height}.  So the relevant check in
> intel_check_cursor_plane() now looks like:
>
>         if (plane->state->crtc_w != state->base.crtc_w)
>                 intel_crtc->atomic.update_wm = true;
>
> Is there a bugzilla open on this issue with more details?

Not that I know of. Ismael?

BR,
Jani.


>
>
> Matt
>
>> 
>> 
>> [1] http://mid.gmane.org/1428790644-6812-1-git-send-email-ismael@iodev.co.uk
>> [2] http://mid.gmane.org/1428928418.2654.8.camel@gmail.com
>> 
>> 
>> >
>> > On Sat, Apr 11, 2015 at 07:40:34PM -0300, Ismael Luceno wrote:
>> >> A bisect showed that commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
>> >> introduced the issue.
>> >> 
>> >> The issue starts as soon as X takes control of the screen, even if just
>> >> a plain X doing nothing, so based on the code touched by the commit I
>> >> thought it had to be related to the so called "hardware cursor". I
>> >> confirmed it when hiding the cursor made the flickering go away.
>> >> 
>> >> The aforementioned commit removed some suspicious code, and the
>> >> Programmer's Reference Manual confirmed my suspicion:
>> >> 
>> >> "Incorrectly programmed watermark values can result in screen corruption.
>> >> 
>> >> The watermarks should be calculated and programmed when any of the
>> >> watermark calculation inputs change. This includes planes enabling or
>> >> disabling, plane source format or size changing, etc."
>> >> 
>> >> So I'm re-adding the few lines that update the watermarks after a cursor
>> >> size change.
>> >> 
>> >> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
>> >>  1 file changed, 6 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> >> index f75173c..e23f062 100644
>> >> --- a/drivers/gpu/drm/i915/intel_display.c
>> >> +++ b/drivers/gpu/drm/i915/intel_display.c
>> >> @@ -12258,6 +12258,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>> >>  	struct intel_crtc *intel_crtc;
>> >>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>> >>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>> >> +	unsigned old_width;
>> >>  	uint32_t addr;
>> >>  
>> >>  	crtc = crtc ? crtc : plane->crtc;
>> >> @@ -12282,11 +12283,15 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>> >>  	intel_crtc->cursor_addr = addr;
>> >>  	intel_crtc->cursor_bo = obj;
>> >>  update:
>> >> +	old_width = intel_crtc->cursor_width;
>> >>  	intel_crtc->cursor_width = state->base.crtc_w;
>> >>  	intel_crtc->cursor_height = state->base.crtc_h;
>> >>  
>> >> -	if (intel_crtc->active)
>> >> +	if (intel_crtc->active) {
>> >> +		if (old_width != intel_crtc->cursor_width)
>> >> +			intel_update_watermarks(crtc);
>> >>  		intel_crtc_update_cursor(crtc, state->visible);
>> >> +	}
>> >>  }
>> >>  
>> >>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>> >> -- 
>> >> 2.3.4
>> >
>> > -- 
>> > Chris Wilson, Intel Open Source Technology Centre
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-07 13:41       ` Jani Nikula
@ 2015-05-07 14:18         ` Chris Wilson
  2015-05-07 21:23           ` Ismael Luceno
  2015-05-08 15:10         ` Ismael Luceno
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2015-05-07 14:18 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Matt Roper, Ismael Luceno, intel-gfx, linux-kernel

On Thu, May 07, 2015 at 04:41:48PM +0300, Jani Nikula wrote:
> On Thu, 07 May 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
> >> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > [cc'ing the authors]
> >> 
> >> This has been posted earlier [1] and it has review to be addressed [2].
> >> 
> >> BR,
> >> Jani.
> >
> > I agree with Ander's response in [2]...we can't call
> > intel_update_watermarks() in the commit function because we're under
> > vblank evasion.  We should already be flagging the transaction as
> > needing a watermark update in intel_check_cursor_plane(), and that flag
> > will be acted upon immediately after the commit functions are done
> > running, once we've re-enabled interrupts.
> >
> > Note that our current codebase looks a bit different since we've dropped
> > intel_crtc->cursor_{width,height}.  So the relevant check in
> > intel_check_cursor_plane() now looks like:
> >
> >         if (plane->state->crtc_w != state->base.crtc_w)
> >                 intel_crtc->atomic.update_wm = true;
> >
> > Is there a bugzilla open on this issue with more details?
> 
> Not that I know of. Ismael?
> 

Probably:
https://bugs.freedesktop.org/show_bug.cgi?id=88944
https://bugzilla.redhat.com/show_bug.cgi?id=1199890
are related.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-07 14:18         ` Chris Wilson
@ 2015-05-07 21:23           ` Ismael Luceno
  0 siblings, 0 replies; 9+ messages in thread
From: Ismael Luceno @ 2015-05-07 21:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, Matt Roper, intel-gfx, linux-kernel

On Thu, 7 May 2015 15:18:27 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, May 07, 2015 at 04:41:48PM +0300, Jani Nikula wrote:
> > On Thu, 07 May 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
> > >> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk>
> > >> wrote:
> > >> > [cc'ing the authors]
> > >> 
> > >> This has been posted earlier [1] and it has review to be
> > >> addressed [2].
> > >> 
> > >> BR,
> > >> Jani.
> > >
> > > I agree with Ander's response in [2]...we can't call
> > > intel_update_watermarks() in the commit function because we're
> > > under vblank evasion.  We should already be flagging the
> > > transaction as needing a watermark update in
> > > intel_check_cursor_plane(), and that flag will be acted upon
> > > immediately after the commit functions are done running, once
> > > we've re-enabled interrupts.
> > >
> > > Note that our current codebase looks a bit different since we've
> > > dropped intel_crtc->cursor_{width,height}.  So the relevant check
> > > in intel_check_cursor_plane() now looks like:
> > >
> > >         if (plane->state->crtc_w != state->base.crtc_w)
> > >                 intel_crtc->atomic.update_wm = true;
> > >
> > > Is there a bugzilla open on this issue with more details?
> > 
> > Not that I know of. Ismael?
> > 
> 
> Probably:
> https://bugs.freedesktop.org/show_bug.cgi?id=88944
> https://bugzilla.redhat.com/show_bug.cgi?id=1199890
> are related.
> -Chris
> 

I am experiencing neither of those things.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-07 13:41       ` Jani Nikula
  2015-05-07 14:18         ` Chris Wilson
@ 2015-05-08 15:10         ` Ismael Luceno
  2015-05-08 15:16           ` Ismael Luceno
  1 sibling, 1 reply; 9+ messages in thread
From: Ismael Luceno @ 2015-05-08 15:10 UTC (permalink / raw)
  To: Jani Nikula, Chris Wilson; +Cc: Matt Roper, intel-gfx, linux-kernel

On Thu, 07 May 2015 16:41:48 +0300
Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 07 May 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
> >> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > [cc'ing the authors]
> >> 
> >> This has been posted earlier [1] and it has review to be addressed
> >> [2].
> >> 
> >> BR,
> >> Jani.
> >
> > I agree with Ander's response in [2]...we can't call
> > intel_update_watermarks() in the commit function because we're under
> > vblank evasion.  We should already be flagging the transaction as
> > needing a watermark update in intel_check_cursor_plane(), and that
> > flag will be acted upon immediately after the commit functions are
> > done running, once we've re-enabled interrupts.
> >
> > Note that our current codebase looks a bit different since we've
> > dropped intel_crtc->cursor_{width,height}.  So the relevant check in
> > intel_check_cursor_plane() now looks like:
> >
> >         if (plane->state->crtc_w != state->base.crtc_w)
> >                 intel_crtc->atomic.update_wm = true;
> >
> > Is there a bugzilla open on this issue with more details?
> 
> Not that I know of. Ismael?

Didn't found one at the time.

I apologize for the lack of communication, have been too busy job
hunting these weeks.

Chris comments prompted me to double-check. It seems one of Matt's
commits solves the issue [0], it just didn't hit mainline until April
20 [1], long after v4.0.

[0] 3dd512fbda0d87d1c3fb44bf878b262baee98fb6 
[1] 14aa02449064541217836b9f3d3295e241d5ae9c

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Fix screen flickering on X
  2015-05-08 15:10         ` Ismael Luceno
@ 2015-05-08 15:16           ` Ismael Luceno
  0 siblings, 0 replies; 9+ messages in thread
From: Ismael Luceno @ 2015-05-08 15:16 UTC (permalink / raw)
  To: Jani Nikula, Chris Wilson; +Cc: Matt Roper, intel-gfx, linux-kernel

On Fri, 8 May 2015 12:10:15 -0300
Ismael Luceno <ismael@iodev.co.uk> wrote:
> On Thu, 07 May 2015 16:41:48 +0300
> Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Thu, 07 May 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > > On Thu, May 07, 2015 at 12:12:18PM +0300, Jani Nikula wrote:
> > >> On Thu, 23 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk>
> > >> wrote:
> > >> > [cc'ing the authors]
> > >> 
> > >> This has been posted earlier [1] and it has review to be
> > >> addressed [2].
> > >> 
> > >> BR,
> > >> Jani.
> > >
> > > I agree with Ander's response in [2]...we can't call
> > > intel_update_watermarks() in the commit function because we're
> > > under vblank evasion.  We should already be flagging the
> > > transaction as needing a watermark update in
> > > intel_check_cursor_plane(), and that flag will be acted upon
> > > immediately after the commit functions are done running, once
> > > we've re-enabled interrupts.
> > >
> > > Note that our current codebase looks a bit different since we've
> > > dropped intel_crtc->cursor_{width,height}.  So the relevant check
> > > in intel_check_cursor_plane() now looks like:
> > >
> > >         if (plane->state->crtc_w != state->base.crtc_w)
> > >                 intel_crtc->atomic.update_wm = true;
> > >
> > > Is there a bugzilla open on this issue with more details?
> > 
> > Not that I know of. Ismael?
> 
> Didn't found one at the time.
> 
> I apologize for the lack of communication, have been too busy job
> hunting these weeks.
> 
> Chris comments prompted me to double-check. It seems one of Matt's
> commits solves the issue [0], it just didn't hit mainline until April
> 20 [1], long after v4.0.
> 
> [0] 3dd512fbda0d87d1c3fb44bf878b262baee98fb6 
> [1] 14aa02449064541217836b9f3d3295e241d5ae9c

Sorry, meant Matt's comment about the current codebase; my brain seems
not to be working well today.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-05-08 15:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-11 22:40 [PATCH] drm/i915: Fix screen flickering on X Ismael Luceno
2015-04-23  9:15 ` Chris Wilson
2015-05-07  9:12   ` [Intel-gfx] " Jani Nikula
2015-05-07 13:27     ` Matt Roper
2015-05-07 13:41       ` Jani Nikula
2015-05-07 14:18         ` Chris Wilson
2015-05-07 21:23           ` Ismael Luceno
2015-05-08 15:10         ` Ismael Luceno
2015-05-08 15:16           ` Ismael Luceno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).