diff mbox series

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

Message ID 070054f632d4ce26740949919660ed61d195942e.1567446845.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: deconflate display disable from no display | expand

Commit Message

Jani Nikula Sept. 2, 2019, 6:08 p.m. UTC
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(-)

Comments

Souza, Jose Sept. 4, 2019, 11:57 p.m. UTC | #1
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);
>
diff mbox series

Patch

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;
+			}
 		}
 	}
 
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)) {
 		u32 fuse_strap = I915_READ(FUSE_STRAP);
 		u32 sfuse_strap = I915_READ(SFUSE_STRAP);