Re: [PATCH v2 03/27] drm/i915: clean up intel_sanitize_crtc, v2

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 10-06-15 om 03:58 schreef Matt Roper:
> On Thu, Jun 04, 2015 at 02:47:33PM +0200, Maarten Lankhorst wrote:
>> Instead of breaking all connections manually, kill them with
>> atomic modeset, if it happens. Now that the everything's atomic
>> some code becomes obsolete.
>>
>> Forcing a atomic modeset with null mode and all connectors gone
>> updates the state too, in a cleaner way.
>>
>> Because load detection is atomic too there can be no mismatch between
>> crtc->active and crtc->state->active
>>
>> Changes since v1:
>> - Reword commit message.
>> - Set null mode and break all connectors when force disabling crtc.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |   8 +--
>>  drivers/gpu/drm/i915/intel_display.c | 108 ++++++++++++++++-------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |   2 +-
>>  3 files changed, 53 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 51fe0fbcd00c..db23916c1875 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -3637,7 +3637,7 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
>>  		bool active = pipe_config->base.active;
>>  
>>  		if (active) {
>> -			intel_crtc_control(&crtc->base, false);
>> +			intel_crtc_control(&crtc->base, false, false);
>>  			pipe_config = to_intel_crtc_state(crtc->base.state);
>>  		}
>>  
>> @@ -3647,7 +3647,7 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
>>  					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
>>  
>>  		if (active)
>> -			intel_crtc_control(&crtc->base, true);
>> +			intel_crtc_control(&crtc->base, true, false);
>>  	}
>>  	drm_modeset_unlock_all(dev);
>>  }
>> @@ -3671,7 +3671,7 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
>>  		bool active = pipe_config->base.active;
>>  
>>  		if (active) {
>> -			intel_crtc_control(&crtc->base, false);
>> +			intel_crtc_control(&crtc->base, false, false);
>>  			pipe_config = to_intel_crtc_state(crtc->base.state);
>>  		}
>>  
>> @@ -3681,7 +3681,7 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
>>  					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
>>  
>>  		if (active)
>> -			intel_crtc_control(&crtc->base, true);
>> +			intel_crtc_control(&crtc->base, true, false);
>>  	}
>>  	drm_modeset_unlock_all(dev);
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 0f031658a297..1936a28a57c1 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6192,7 +6192,7 @@ free:
>>  }
>>  
>>  /* Master function to enable/disable CRTC and corresponding power wells */
>> -int intel_crtc_control(struct drm_crtc *crtc, bool enable)
>> +int intel_crtc_control(struct drm_crtc *crtc, bool active, bool force)
> I realize intel_display.c hasn't been fully kerneldoc-ized yet, but this
> function seems like an important one to have a description of what it's
> doing and what the parameters mean (the term 'force' is a bit
> vague/generic and it may not be immediately obvious to a reader that
> what you're doing here is going to wind up detaching the links.
Yeah no worries, this patch can be dropped. I think we should strive to perform a single modeset the hw readout state.

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