Re: [PATCH 07/10] drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 11, 2018 at 12:04:54PM +0200, Maarten Lankhorst wrote:
> The CRTC is idle at this point, so we can dereference crtc->state safely.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 19d7714df021..cbe70bc4d02d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15934,6 +15934,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *crtc;
> +	struct intel_crtc_state *crtc_state;

Can be moved into narrower scope.

>  	struct intel_encoder *encoder;
>  	int i;
>  
> @@ -15952,7 +15953,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_crtc(&dev_priv->drm, crtc) {
>  		drm_crtc_vblank_reset(&crtc->base);
>  
> -		if (crtc->active)
> +		if (crtc->base.state->active)
>  			drm_crtc_vblank_on(&crtc->base);
>  	}
>  
> @@ -15961,9 +15962,10 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_encoder(dev, encoder)
>  		intel_sanitize_encoder(encoder);
>  
> -	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +	for_each_intel_crtc(dev, crtc) {

I'd keep the dev_priv->drm, because we should just change the function
to take the dev_priv directly.

Apart from those
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		crtc_state = to_intel_crtc_state(crtc->base.state);
>  		intel_sanitize_crtc(crtc, ctx);
> -		intel_dump_pipe_config(crtc, crtc->config,
> +		intel_dump_pipe_config(crtc, crtc_state,
>  				       "[setup_hw_state]");
>  	}
>  
> @@ -15997,7 +15999,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_crtc(dev, crtc) {
>  		u64 put_domains;
>  
> -		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc->config);
> +		crtc_state = to_intel_crtc_state(crtc->base.state);
> +		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc_state);
>  		if (WARN_ON(put_domains))
>  			modeset_put_power_domains(dev_priv, put_domains);
>  	}
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx