[v4,8/8] drm/i915: Remove vblank wait from hsw_enable_ips.

Submitted by Maarten Lankhorst on Feb. 10, 2016, 12:49 p.m.

Details

Message ID 1455108583-29227-9-git-send-email-maarten.lankhorst@linux.intel.com
State New
Headers show
Series "Kill off intel_crtc->atomic!" ( rev: 10 9 8 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Maarten Lankhorst Feb. 10, 2016, 12:49 p.m.
intel_post_plane_update did an extra vblank wait that's no longer needed when enabling ips.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 1 file changed, 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6bb1f5dbc7a0..19a8d376d63e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4569,9 +4569,6 @@  void hsw_enable_ips(struct intel_crtc *crtc)
 	if (!crtc->config->ips_enabled)
 		return;
 
-	/* We can only enable IPS after we enable a plane and wait for a vblank */
-	intel_wait_for_vblank(dev, crtc->pipe);
-
 	assert_plane_enabled(dev_priv, crtc->plane);
 	if (IS_BROADWELL(dev)) {
 		mutex_lock(&dev_priv->rps.hw_lock);

Comments

Em Qua, 2016-02-10 às 13:49 +0100, Maarten Lankhorst escreveu:
> intel_post_plane_update did an extra vblank wait that's no longer

> needed when enabling ips.

> 

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/intel_display.c | 3 ---

>  1 file changed, 3 deletions(-)

> 

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

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

> index 6bb1f5dbc7a0..19a8d376d63e 100644

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

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

> @@ -4569,9 +4569,6 @@ void hsw_enable_ips(struct intel_crtc *crtc)

>  	if (!crtc->config->ips_enabled)

>  		return;

>  

> -	/* We can only enable IPS after we enable a plane and wait

> for a vblank */


Perhaps we can keep the comment, but change it into something like:

/* We can only enable IPS after we enable a plane and wait for a
vblank, but we don't need the wait here because of XYZ. */

> -	intel_wait_for_vblank(dev, crtc->pipe);

> -

>  	assert_plane_enabled(dev_priv, crtc->plane);

>  	if (IS_BROADWELL(dev)) {

>  		mutex_lock(&dev_priv->rps.hw_lock);
Hey,

Op 12-02-16 om 13:06 schreef Zanoni, Paulo R:
> Em Qua, 2016-02-10 às 13:49 +0100, Maarten Lankhorst escreveu:
>> intel_post_plane_update did an extra vblank wait that's no longer
>> needed when enabling ips.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 6bb1f5dbc7a0..19a8d376d63e 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4569,9 +4569,6 @@ void hsw_enable_ips(struct intel_crtc *crtc)
>>  	if (!crtc->config->ips_enabled)
>>  		return;
>>  
>> -	/* We can only enable IPS after we enable a plane and wait
>> for a vblank */
> Perhaps we can keep the comment, but change it into something like:
>
> /* We can only enable IPS after we enable a plane and wait for a
> vblank, but we don't need the wait here because of XYZ. */
>
post_plane_update only gets run after a vblank wait now, so I think adding a comment here is optional.
But yeah I'll make it more clear in the commit.

~Maarten
intel_post_plane_update did an extra vblank wait that's no longer needed when enabling ips.

Changes since v1:
- Add comment explaining why vblank wait is performed. (Paulo)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Noticed this one still in my branch, does this look ok?

~Maarten
Em Qua, 2016-03-23 às 14:33 +0100, Maarten Lankhorst escreveu:
> intel_post_plane_update did an extra vblank wait that's no longer

> needed when enabling ips.

> 

> Changes since v1:

> - Add comment explaining why vblank wait is performed. (Paulo)

> 

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> ---

> Noticed this one still in my branch, does this look ok?

> 

> ~Maarten

> 

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

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

> index dc920cfc3fd7..6dcc159ce6ac 100644

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

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

> @@ -4454,8 +4454,11 @@ void hsw_enable_ips(struct intel_crtc *crtc)

>  	if (!crtc->config->ips_enabled)

>  		return;

>  

> -	/* We can only enable IPS after we enable a plane and wait

> for a vblank */

> -	intel_wait_for_vblank(dev, crtc->pipe);

> +	/*

> +	 * We can only enable IPS after we enable a plane and wait

> for a vblank

> +	 * This function is called from post_plane_update, which is

> run after

> +	 * a vblank wait.

> +	 */


There are actually other callers, such as intel_dp.c, but I suppose
they are correct since they can only be called when the planes are
already enabled, so:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>


>  

>  	assert_plane_enabled(dev_priv, crtc->plane);

>  	if (IS_BROADWELL(dev)) {

>