Re: [PATCH 25/42] drm/i915: Remove use of crtc->config from intel_psr.c

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

Op 12-05-15 om 11:20 schreef Daniel Vetter:
> On Mon, May 11, 2015 at 04:25:01PM +0200, Maarten Lankhorst wrote:
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 25 ++++++++++++++-----------
>>  1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 5ee0fa57ed19..868817402c11 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -73,14 +73,15 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>>  }
>>  
>>  static void intel_psr_write_vsc(struct intel_dp *intel_dp,
>> -				    struct edp_vsc_psr *vsc_psr)
>> +				struct edp_vsc_psr *vsc_psr)
>>  {
>>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>>  	struct drm_device *dev = dig_port->base.base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
>> -	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(crtc->config->cpu_transcoder);
>> -	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(crtc->config->cpu_transcoder);
>> +	struct intel_crtc_state *pipe_config =
>> +		to_intel_crtc_state(dig_port->base.base.crtc->state);
>> +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder);
>> +	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(pipe_config->cpu_transcoder);
>>  	uint32_t *data = (uint32_t *) vsc_psr;
>>  	unsigned int i;
>>  
>> @@ -282,13 +283,13 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>>  				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
>>  }
>>  
>> -static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>> +static bool intel_psr_match_conditions(struct intel_dp *intel_dp,
>> +				       struct intel_crtc_state *pipe_config)
> I spotted this pattern in a few other places as well already, where you
> add a local variable to avoid the dereference dance again. But this is
> called from a pre_enable hook, i.e. we can just directly access
> crtc->state to get at the right pipe config. If you instead pass it as a
> parameter I have to hunt around to make sure it's the right one.
>
> Imo passing pipe_config should only be done if the code can be called in
> the compute_config/check phase or in the disable phase. That then gives
> reviewers a nice heads-up about the potential trickiness.
>
Ok.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx