diff mbox

[3/5] drm/i915: Enable vblanks after verifying power domain states.

Message ID 20180103204001.3008-3-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan Jan. 3, 2018, 8:39 p.m. UTC
Since we want to allow for a non-blocking power domain for vblanks,
the power domain use count and power well use count will not be updated
atomically inside the power domain mutex (see next patch). This affects
verifying if sum(power_domain_use_count) == power_well_use_count at
init time. So do not enable vblanks until this verification is done.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Chris Wilson Jan. 3, 2018, 8:57 p.m. UTC | #1
Quoting Dhinakaran Pandiyan (2018-01-03 20:39:59)
> Since we want to allow for a non-blocking power domain for vblanks,
> the power domain use count and power well use count will not be updated
> atomically inside the power domain mutex (see next patch). This affects
> verifying if sum(power_domain_use_count) == power_well_use_count at
> init time. So do not enable vblanks until this verification is done.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0cd355978ab4..7bc874b8dac7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv,
>                 (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
>  }
>  
> +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)
> +{
> +       enum pipe pipe;
> +
> +       for_each_pipe(dev_priv, pipe) {

for_each_intel_crtc()
-Chris
Maarten Lankhorst Jan. 4, 2018, 11:35 a.m. UTC | #2
Op 03-01-18 om 21:39 schreef Dhinakaran Pandiyan:
> Since we want to allow for a non-blocking power domain for vblanks,
> the power domain use count and power well use count will not be updated
> atomically inside the power domain mutex (see next patch). This affects
> verifying if sum(power_domain_use_count) == power_well_use_count at
> init time. So do not enable vblanks until this verification is done.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0cd355978ab4..7bc874b8dac7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv,
>  		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
>  }
>  
> +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)
> +{
> +	enum pipe pipe;
> +
> +	for_each_pipe(dev_priv, pipe) {
> +		struct intel_crtc *crtc;
> +
> +		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +
> +		/* restore vblank interrupts to correct state */
> +		drm_crtc_vblank_reset(&crtc->base);
> +
> +		if (crtc->active)
> +			drm_crtc_vblank_on(&crtc->base);
> +	}
> +}
> +
> +
>  static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  				struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -14754,13 +14772,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>  	}
>  
> -	/* restore vblank interrupts to correct state */
> -	drm_crtc_vblank_reset(&crtc->base);
>  	if (crtc->active) {
>  		struct intel_plane *plane;
>  
> -		drm_crtc_vblank_on(&crtc->base);
> -
>  		/* Disable everything but the primary plane */
>  		for_each_intel_plane_on_crtc(dev, crtc, plane) {
>  			const struct intel_plane_state *plane_state =
> @@ -15147,6 +15161,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  
>  	intel_power_domains_verify_state(dev_priv);
>  
> +	modeset_enable_vblanks(dev_priv);
> +
>  	intel_fbc_init_pipe_state(dev_priv);
>  }
>  

intel_pre_disable_primary_noatomic calls wait_for_vblank, which will fail with this patch.
Wouldn't it be better to make intel_power_domains_verify_state work correctly with the vblank irq?

~Maarten
Dhinakaran Pandiyan Jan. 4, 2018, 9:25 p.m. UTC | #3
On Wed, 2018-01-03 at 20:57 +0000, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-01-03 20:39:59)

> > Since we want to allow for a non-blocking power domain for vblanks,

> > the power domain use count and power well use count will not be updated

> > atomically inside the power domain mutex (see next patch). This affects

> > verifying if sum(power_domain_use_count) == power_well_use_count at

> > init time. So do not enable vblanks until this verification is done.

> > 

> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----

> >  1 file changed, 20 insertions(+), 4 deletions(-)

> > 

> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c

> > index 0cd355978ab4..7bc874b8dac7 100644

> > --- a/drivers/gpu/drm/i915/intel_display.c

> > +++ b/drivers/gpu/drm/i915/intel_display.c

> > @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv,

> >                 (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);

> >  }

> >  

> > +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)

> > +{

> > +       enum pipe pipe;

> > +

> > +       for_each_pipe(dev_priv, pipe) {

> 

> for_each_intel_crtc()

> -Chris




Thanks for you comments, I'll fix them up in the next version if the
overall approach (disabling DC off) is Acked.
-DK



> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Jan. 4, 2018, 9:51 p.m. UTC | #4
On Thu, 2018-01-04 at 12:35 +0100, Maarten Lankhorst wrote:
> Wouldn't it be better to make intel_power_domains_verify_state work

> correctly with the vblank irq?


I tried to :) Since I changed the domain_use_count to atomic_t and moved
it outside of the locks, verify_state became racy. Let me take another
look.

-DK
Maarten Lankhorst Jan. 5, 2018, 11:23 a.m. UTC | #5
Op 04-01-18 om 22:51 schreef Pandiyan, Dhinakaran:
> On Thu, 2018-01-04 at 12:35 +0100, Maarten Lankhorst wrote:
>> Wouldn't it be better to make intel_power_domains_verify_state work
>> correctly with the vblank irq?
> I tried to :) Since I changed the domain_use_count to atomic_t and moved
> it outside of the locks, verify_state became racy. Let me take another
> look.
>
> -DK

It sucks that we end up with 2 ways to handle power domains..

I'm trying to think of a cleaner way, coming up with none. :(

It would have been nice if we could do something like a seqlock for
the refcounts, but that would prevent us from blocking too..

Is it only the DC off power well we care about?

~Maarten
Rodrigo Vivi Jan. 5, 2018, 6:09 p.m. UTC | #6
On Fri, Jan 05, 2018 at 11:23:54AM +0000, Maarten Lankhorst wrote:
> Op 04-01-18 om 22:51 schreef Pandiyan, Dhinakaran:
> > On Thu, 2018-01-04 at 12:35 +0100, Maarten Lankhorst wrote:
> >> Wouldn't it be better to make intel_power_domains_verify_state work
> >> correctly with the vblank irq?
> > I tried to :) Since I changed the domain_use_count to atomic_t and moved
> > it outside of the locks, verify_state became racy. Let me take another
> > look.
> >
> > -DK
> 
> It sucks that we end up with 2 ways to handle power domains..

I also don't like that, but if we need to go with that I believe
we need to go with a generic approach.

> 
> I'm trying to think of a cleaner way, coming up with none. :(

me neither :(

> 
> It would have been nice if we could do something like a seqlock for
> the refcounts, but that would prevent us from blocking too..

reader does't block and writer doesn't wait for the reader so not
sure we could use this.

> 
> Is it only the DC off power well we care about?

It is the only call to any power well that comes from an spin-locked
region. So we can't sleep.

I think we looked to the option of changing the entire pw to spin locks
instead of mutexs, but we concluded it wasn't a viable option as well.
I just can't remember why right now.

Thanks,
Rodrigo.

> 
> ~Maarten
>
Dhinakaran Pandiyan Jan. 5, 2018, 6:45 p.m. UTC | #7
On Fri, 2018-01-05 at 10:09 -0800, Rodrigo Vivi wrote:
> On Fri, Jan 05, 2018 at 11:23:54AM +0000, Maarten Lankhorst wrote:

> > Op 04-01-18 om 22:51 schreef Pandiyan, Dhinakaran:

> > > On Thu, 2018-01-04 at 12:35 +0100, Maarten Lankhorst wrote:

> > >> Wouldn't it be better to make intel_power_domains_verify_state work

> > >> correctly with the vblank irq?

> > > I tried to :) Since I changed the domain_use_count to atomic_t and moved

> > > it outside of the locks, verify_state became racy. Let me take another

> > > look.

> > >

> > > -DK

> > 

> > It sucks that we end up with 2 ways to handle power domains..

> 

> I also don't like that, but if we need to go with that I believe

> we need to go with a generic approach.

> 

> > 

> > I'm trying to think of a cleaner way, coming up with none. :(

> 

> me neither :(

> 

> > 

> > It would have been nice if we could do something like a seqlock for

> > the refcounts, but that would prevent us from blocking too..

> 

> reader does't block and writer doesn't wait for the reader so not

> sure we could use this.

> 

> > 

> > Is it only the DC off power well we care about?

> 


Yeah, that is sufficient to deal with frame counter resets.

> It is the only call to any power well that comes from an spin-locked

> region. So we can't sleep.

> 

> I think we looked to the option of changing the entire pw to spin locks

> instead of mutexs, but we concluded it wasn't a viable option as well.

> I just can't remember why right now.


Enable/disable for other power wells have wait_for_register()

> 

> Thanks,

> Rodrigo.

> 

> > 

> > ~Maarten

> >
Dhinakaran Pandiyan Jan. 6, 2018, 9:51 a.m. UTC | #8
On Thursday, January 4, 2018 12:35:48 PM PST Maarten Lankhorst wrote:
> Op 03-01-18 om 21:39 schreef Dhinakaran Pandiyan:
> > Since we want to allow for a non-blocking power domain for vblanks,
> > the power domain use count and power well use count will not be updated
> > atomically inside the power domain mutex (see next patch). This affects
> > verifying if sum(power_domain_use_count) == power_well_use_count at
> > init time. So do not enable vblanks until this verification is done.
> > 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> > 
> >  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index 0cd355978ab4..7bc874b8dac7
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct
> > drm_i915_private *dev_priv,> 
> >  		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
> >  
> >  }
> > 
> > +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)
> > +{
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe(dev_priv, pipe) {
> > +		struct intel_crtc *crtc;
> > +
> > +		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > +
> > +		/* restore vblank interrupts to correct state */
> > +		drm_crtc_vblank_reset(&crtc->base);
> > +
> > +		if (crtc->active)
> > +			drm_crtc_vblank_on(&crtc->base);
> > +	}
> > +}
> > +
> > +
> > 
> >  static void intel_sanitize_crtc(struct intel_crtc *crtc,
> >  
> >  				struct drm_modeset_acquire_ctx *ctx)
> >  
> >  {
> > 
> > @@ -14754,13 +14772,9 @@ static void intel_sanitize_crtc(struct intel_crtc
> > *crtc,> 
> >  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> >  	
> >  	}
> > 
> > -	/* restore vblank interrupts to correct state */
> > -	drm_crtc_vblank_reset(&crtc->base);
> > 
> >  	if (crtc->active) {
> >  	
> >  		struct intel_plane *plane;
> > 
> > -		drm_crtc_vblank_on(&crtc->base);
> > -
> > 
> >  		/* Disable everything but the primary plane */
> >  		for_each_intel_plane_on_crtc(dev, crtc, plane) {
> >  		
> >  			const struct intel_plane_state *plane_state =
> > 
> > @@ -15147,6 +15161,8 @@ intel_modeset_setup_hw_state(struct drm_device
> > *dev,> 
> >  	intel_power_domains_verify_state(dev_priv);
> > 
> > +	modeset_enable_vblanks(dev_priv);
> > +
> > 
> >  	intel_fbc_init_pipe_state(dev_priv);
> >  
> >  }
> 
> intel_pre_disable_primary_noatomic calls wait_for_vblank, which will fail
> with this patch. Wouldn't it be better to make
> intel_power_domains_verify_state work correctly with the vblank irq?

How about disabling vblanks just before power_domains_verify_state and 
enabling it back again?

Another option is to ignore the check in power_domains_verify_state  for 
DC_OFF. 

-DK

> 
> ~Maarten
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst Jan. 8, 2018, 2:43 p.m. UTC | #9
Op 06-01-18 om 10:51 schreef Dhinakaran Pandiyan:
> On Thursday, January 4, 2018 12:35:48 PM PST Maarten Lankhorst wrote:
>> Op 03-01-18 om 21:39 schreef Dhinakaran Pandiyan:
>>> Since we want to allow for a non-blocking power domain for vblanks,
>>> the power domain use count and power well use count will not be updated
>>> atomically inside the power domain mutex (see next patch). This affects
>>> verifying if sum(power_domain_use_count) == power_well_use_count at
>>> init time. So do not enable vblanks until this verification is done.
>>>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> ---
>>>
>>>  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----
>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c index 0cd355978ab4..7bc874b8dac7
>>> 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct
>>> drm_i915_private *dev_priv,> 
>>>  		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
>>>  
>>>  }
>>>
>>> +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)
>>> +{
>>> +	enum pipe pipe;
>>> +
>>> +	for_each_pipe(dev_priv, pipe) {
>>> +		struct intel_crtc *crtc;
>>> +
>>> +		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>>> +
>>> +		/* restore vblank interrupts to correct state */
>>> +		drm_crtc_vblank_reset(&crtc->base);
>>> +
>>> +		if (crtc->active)
>>> +			drm_crtc_vblank_on(&crtc->base);
>>> +	}
>>> +}
>>> +
>>> +
>>>
>>>  static void intel_sanitize_crtc(struct intel_crtc *crtc,
>>>  
>>>  				struct drm_modeset_acquire_ctx *ctx)
>>>  
>>>  {
>>>
>>> @@ -14754,13 +14772,9 @@ static void intel_sanitize_crtc(struct intel_crtc
>>> *crtc,> 
>>>  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>>>  	
>>>  	}
>>>
>>> -	/* restore vblank interrupts to correct state */
>>> -	drm_crtc_vblank_reset(&crtc->base);
>>>
>>>  	if (crtc->active) {
>>>  	
>>>  		struct intel_plane *plane;
>>>
>>> -		drm_crtc_vblank_on(&crtc->base);
>>> -
>>>
>>>  		/* Disable everything but the primary plane */
>>>  		for_each_intel_plane_on_crtc(dev, crtc, plane) {
>>>  		
>>>  			const struct intel_plane_state *plane_state =
>>>
>>> @@ -15147,6 +15161,8 @@ intel_modeset_setup_hw_state(struct drm_device
>>> *dev,> 
>>>  	intel_power_domains_verify_state(dev_priv);
>>>
>>> +	modeset_enable_vblanks(dev_priv);
>>> +
>>>
>>>  	intel_fbc_init_pipe_state(dev_priv);
>>>  
>>>  }
>> intel_pre_disable_primary_noatomic calls wait_for_vblank, which will fail
>> with this patch. Wouldn't it be better to make
>> intel_power_domains_verify_state work correctly with the vblank irq?
> How about disabling vblanks just before power_domains_verify_state and 
> enabling it back again?
>
> Another option is to ignore the check in power_domains_verify_state  for 
> DC_OFF. 
That would be better, or at least assume it's at most 1 more than calculated..

~Maarten
Dhinakaran Pandiyan Jan. 8, 2018, 7:29 p.m. UTC | #10
On Mon, 2018-01-08 at 15:43 +0100, Maarten Lankhorst wrote:
> Op 06-01-18 om 10:51 schreef Dhinakaran Pandiyan:

> > On Thursday, January 4, 2018 12:35:48 PM PST Maarten Lankhorst wrote:

> >> Op 03-01-18 om 21:39 schreef Dhinakaran Pandiyan:

> >>> Since we want to allow for a non-blocking power domain for vblanks,

> >>> the power domain use count and power well use count will not be updated

> >>> atomically inside the power domain mutex (see next patch). This affects

> >>> verifying if sum(power_domain_use_count) == power_well_use_count at

> >>> init time. So do not enable vblanks until this verification is done.

> >>>

> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> >>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> >>> ---

> >>>

> >>>  drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++++----

> >>>  1 file changed, 20 insertions(+), 4 deletions(-)

> >>>

> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c

> >>> b/drivers/gpu/drm/i915/intel_display.c index 0cd355978ab4..7bc874b8dac7

> >>> 100644

> >>> --- a/drivers/gpu/drm/i915/intel_display.c

> >>> +++ b/drivers/gpu/drm/i915/intel_display.c

> >>> @@ -14739,6 +14739,24 @@ static bool has_pch_trancoder(struct

> >>> drm_i915_private *dev_priv,> 

> >>>  		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);

> >>>  

> >>>  }

> >>>

> >>> +static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)

> >>> +{

> >>> +	enum pipe pipe;

> >>> +

> >>> +	for_each_pipe(dev_priv, pipe) {

> >>> +		struct intel_crtc *crtc;

> >>> +

> >>> +		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);

> >>> +

> >>> +		/* restore vblank interrupts to correct state */

> >>> +		drm_crtc_vblank_reset(&crtc->base);

> >>> +

> >>> +		if (crtc->active)

> >>> +			drm_crtc_vblank_on(&crtc->base);

> >>> +	}

> >>> +}

> >>> +

> >>> +

> >>>

> >>>  static void intel_sanitize_crtc(struct intel_crtc *crtc,

> >>>  

> >>>  				struct drm_modeset_acquire_ctx *ctx)

> >>>  

> >>>  {

> >>>

> >>> @@ -14754,13 +14772,9 @@ static void intel_sanitize_crtc(struct intel_crtc

> >>> *crtc,> 

> >>>  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);

> >>>  	

> >>>  	}

> >>>

> >>> -	/* restore vblank interrupts to correct state */

> >>> -	drm_crtc_vblank_reset(&crtc->base);

> >>>

> >>>  	if (crtc->active) {

> >>>  	

> >>>  		struct intel_plane *plane;

> >>>

> >>> -		drm_crtc_vblank_on(&crtc->base);

> >>> -

> >>>

> >>>  		/* Disable everything but the primary plane */

> >>>  		for_each_intel_plane_on_crtc(dev, crtc, plane) {

> >>>  		

> >>>  			const struct intel_plane_state *plane_state =

> >>>

> >>> @@ -15147,6 +15161,8 @@ intel_modeset_setup_hw_state(struct drm_device

> >>> *dev,> 

> >>>  	intel_power_domains_verify_state(dev_priv);

> >>>

> >>> +	modeset_enable_vblanks(dev_priv);

> >>> +

> >>>

> >>>  	intel_fbc_init_pipe_state(dev_priv);

> >>>  

> >>>  }

> >> intel_pre_disable_primary_noatomic calls wait_for_vblank, which will fail

> >> with this patch. Wouldn't it be better to make

> >> intel_power_domains_verify_state work correctly with the vblank irq?

> > How about disabling vblanks just before power_domains_verify_state and 

> > enabling it back again?

> >

> > Another option is to ignore the check in power_domains_verify_state  for 

> > DC_OFF. 

> That would be better, or at least assume it's at most 1 more than calculated..


Sounds good.
-DK

> 

> ~Maarten
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0cd355978ab4..7bc874b8dac7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14739,6 +14739,24 @@  static bool has_pch_trancoder(struct drm_i915_private *dev_priv,
 		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
 }
 
+static void modeset_enable_vblanks(struct drm_i915_private *dev_priv)
+{
+	enum pipe pipe;
+
+	for_each_pipe(dev_priv, pipe) {
+		struct intel_crtc *crtc;
+
+		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+
+		/* restore vblank interrupts to correct state */
+		drm_crtc_vblank_reset(&crtc->base);
+
+		if (crtc->active)
+			drm_crtc_vblank_on(&crtc->base);
+	}
+}
+
+
 static void intel_sanitize_crtc(struct intel_crtc *crtc,
 				struct drm_modeset_acquire_ctx *ctx)
 {
@@ -14754,13 +14772,9 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc,
 			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
 	}
 
-	/* restore vblank interrupts to correct state */
-	drm_crtc_vblank_reset(&crtc->base);
 	if (crtc->active) {
 		struct intel_plane *plane;
 
-		drm_crtc_vblank_on(&crtc->base);
-
 		/* Disable everything but the primary plane */
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
 			const struct intel_plane_state *plane_state =
@@ -15147,6 +15161,8 @@  intel_modeset_setup_hw_state(struct drm_device *dev,
 
 	intel_power_domains_verify_state(dev_priv);
 
+	modeset_enable_vblanks(dev_priv);
+
 	intel_fbc_init_pipe_state(dev_priv);
 }