Re: [PATCH 4/4] drm/i915: stop conflating HAS_DISPLAY() and disabled display

["Souza, Jose" <jose.souza@intel.com>]

On Mon, 2019-09-02 at 21:08 +0300, Jani Nikula wrote:
> Stop setting ->pipe_mask to zero when display is disabled, allowing
> us
> to have different code paths for not actually having display
> hardware,
> and having display hardware disabled. This lets us develop those two
> avenues independently.
> 
> There are no functional changes for when there is no display.
> However,
> all uses of for_each_pipe() and for_each_pipe_masked() will start
> running for the disabled display case. Put one of the more
> significant
> ones behind checks for INTEL_DISPLAY_ENABLED(), otherwise the cases
> should not be hit with disabled display, or they seem benign. Fingers
> crossed.
> 
> All in all, this might not be the ideal solution. In fact we may have
> had something along the lines of this in the past, but we ended up
> conflating the two cases. Possibly even by recommendation by yours
> truly; I did not dare dig up that part of the history. But the
> perfect
> is the enemy of the good, this is a straightforward change, and lets
> us
> get actual work done in both fronts without interfering with each
> other.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++-----
>  drivers/gpu/drm/i915/intel_device_info.c     |  8 ++------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index c3bb18afe6d7..15f00dee6368 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16217,11 +16217,13 @@ int intel_modeset_init(struct drm_device
> *dev)
>  		      INTEL_NUM_PIPES(dev_priv),
>  		      INTEL_NUM_PIPES(dev_priv) > 1 ? "s" : "");
>  
> -	for_each_pipe(dev_priv, pipe) {
> -		ret = intel_crtc_init(dev_priv, pipe);
> -		if (ret) {
> -			drm_mode_config_cleanup(dev);
> -			return ret;
> +	if (HAS_DISPLAY(dev_priv) && INTEL_DISPLAY_ENABLED(dev_priv)) {
> +		for_each_pipe(dev_priv, pipe) {
> +			ret = intel_crtc_init(dev_priv, pipe);
> +			if (ret) {
> +				drm_mode_config_cleanup(dev);
> +				return ret;
> +			}

The CI warning are coming from this change, there is more thing to do
before not initialize the CRTCS.

>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index 50b05a5de53b..5d2da7e68a60 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -894,12 +894,8 @@ void intel_device_info_runtime_init(struct
> drm_i915_private *dev_priv)
>  			runtime->num_sprites[pipe] = 1;
>  	}
>  
> -	if (i915_modparams.disable_display) {
> -		DRM_INFO("Display disabled (module parameter)\n");
> -		info->pipe_mask = 0;
> -	} else if (HAS_DISPLAY(dev_priv) &&
> -		   (IS_GEN_RANGE(dev_priv, 7, 8)) &&
> -		   HAS_PCH_SPLIT(dev_priv)) {
> +	if (HAS_DISPLAY(dev_priv) && IS_GEN_RANGE(dev_priv, 7, 8) &&
> +	    HAS_PCH_SPLIT(dev_priv)) {

This block looks right.

>  		u32 fuse_strap = I915_READ(FUSE_STRAP);
>  		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx