diff mbox series

[3/3] drm/i915: Factor out a function disabling fused-off display

Message ID 20230206163512.2841513-3-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: Move display power initialization during driver probing later | expand

Commit Message

Imre Deak Feb. 6, 2023, 4:35 p.m. UTC 
Factor out a function used both on older and new platforms to disable
the display functionality if the display is fused-off.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++-----------
 1 file changed, 18 insertions(+), 16 deletions(-)

Comments

iczero Feb. 7, 2023, 2:45 a.m. UTC | #1 
On 2/6/23 11:35, Imre Deak wrote:
> Factor out a function used both on older and new platforms to disable
> the display functionality if the display is fused-off.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++-----------
>   1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 044ac552c9280..9d6d1fad9f1d9 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -369,6 +369,21 @@ void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
>   	intel_device_info_subplatform_init(i915);
>   }
>   
> +static void disable_fused_off_display(struct drm_i915_private *i915)
> +{
> +	struct intel_runtime_info *runtime = RUNTIME_INFO(i915);
> +
> +	drm_info(&i915->drm, "Display fused off, disabling\n");
> +
> +	runtime->pipe_mask = 0;
> +	runtime->cpu_transcoder_mask = 0;
> +	runtime->fbc_mask = 0;
> +	runtime->has_hdcp = 0;
> +	runtime->fbc_mask = 0;
> +	runtime->has_dmc = 0;
> +	runtime->has_dsc = 0;
> +}
> +
>   /**
>    * intel_device_info_runtime_init - initialize runtime info
>    * @dev_priv: the i915 device
> @@ -454,11 +469,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>   		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
>   		    (HAS_PCH_CPT(dev_priv) &&
>   		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
> -			drm_info(&dev_priv->drm,
> -				 "Display fused off, disabling\n");
> -			runtime->pipe_mask = 0;
> -			runtime->cpu_transcoder_mask = 0;
> -			runtime->fbc_mask = 0;
> +			disable_fused_off_display(dev_priv);
>   		} else if (fuse_strap & IVB_PIPE_C_DISABLE) {
>   			drm_info(&dev_priv->drm, "PipeC fused off\n");
>   			runtime->pipe_mask &= ~BIT(PIPE_C);
> @@ -502,17 +513,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>   	}
>   
>   	if ((IS_DGFX(dev_priv) || DISPLAY_VER(dev_priv) >= 14) &&
> -	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT)) {
> -		drm_info(&dev_priv->drm, "Display fused off, disabling\n");
> -
> -		runtime->pipe_mask = 0;
> -		runtime->cpu_transcoder_mask = 0;
> -		runtime->fbc_mask = 0;
> -		runtime->has_hdcp = 0;
> -		runtime->fbc_mask = 0;
> -		runtime->has_dmc = 0;
> -		runtime->has_dsc = 0;
> -	}
> +	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT))
> +		disable_fused_off_display(dev_priv);
>   
>   	if (GRAPHICS_VER(dev_priv) == 6 && i915_vtd_active(dev_priv)) {
>   		drm_info(&dev_priv->drm,
>
Tested on Intel Arc A370M. Thank you for the fix.

Tested-by: iczero <iczero@hellomouse.net>
Jani Nikula Feb. 7, 2023, 10:21 a.m. UTC | #2 
On Mon, 06 Feb 2023, Imre Deak <imre.deak@intel.com> wrote:
> Factor out a function used both on older and new platforms to disable
> the display functionality if the display is fused-off.

The single point of truth for disabling display is the if
(!HAS_DISPLAY()) path near the end of intel_device_info_runtime_init().

I think it's fine to abstract it if you want, but it should *only* be
called from that *one* place in one path.

So that's a no for this one.

BR,
Jani.

>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++-----------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 044ac552c9280..9d6d1fad9f1d9 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -369,6 +369,21 @@ void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
>  	intel_device_info_subplatform_init(i915);
>  }
>  
> +static void disable_fused_off_display(struct drm_i915_private *i915)
> +{
> +	struct intel_runtime_info *runtime = RUNTIME_INFO(i915);
> +
> +	drm_info(&i915->drm, "Display fused off, disabling\n");
> +
> +	runtime->pipe_mask = 0;
> +	runtime->cpu_transcoder_mask = 0;
> +	runtime->fbc_mask = 0;
> +	runtime->has_hdcp = 0;
> +	runtime->fbc_mask = 0;
> +	runtime->has_dmc = 0;
> +	runtime->has_dsc = 0;
> +}
> +
>  /**
>   * intel_device_info_runtime_init - initialize runtime info
>   * @dev_priv: the i915 device
> @@ -454,11 +469,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
>  		    (HAS_PCH_CPT(dev_priv) &&
>  		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
> -			drm_info(&dev_priv->drm,
> -				 "Display fused off, disabling\n");
> -			runtime->pipe_mask = 0;
> -			runtime->cpu_transcoder_mask = 0;
> -			runtime->fbc_mask = 0;
> +			disable_fused_off_display(dev_priv);
>  		} else if (fuse_strap & IVB_PIPE_C_DISABLE) {
>  			drm_info(&dev_priv->drm, "PipeC fused off\n");
>  			runtime->pipe_mask &= ~BIT(PIPE_C);
> @@ -502,17 +513,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  	}
>  
>  	if ((IS_DGFX(dev_priv) || DISPLAY_VER(dev_priv) >= 14) &&
> -	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT)) {
> -		drm_info(&dev_priv->drm, "Display fused off, disabling\n");
> -
> -		runtime->pipe_mask = 0;
> -		runtime->cpu_transcoder_mask = 0;
> -		runtime->fbc_mask = 0;
> -		runtime->has_hdcp = 0;
> -		runtime->fbc_mask = 0;
> -		runtime->has_dmc = 0;
> -		runtime->has_dsc = 0;
> -	}
> +	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT))
> +		disable_fused_off_display(dev_priv);
>  
>  	if (GRAPHICS_VER(dev_priv) == 6 && i915_vtd_active(dev_priv)) {
>  		drm_info(&dev_priv->drm,
Imre Deak Feb. 7, 2023, 11:46 a.m. UTC | #3 
On Tue, Feb 07, 2023 at 12:21:34PM +0200, Jani Nikula wrote:
> On Mon, 06 Feb 2023, Imre Deak <imre.deak@intel.com> wrote:
> > Factor out a function used both on older and new platforms to disable
> > the display functionality if the display is fused-off.
> 
> The single point of truth for disabling display is the if
> (!HAS_DISPLAY()) path near the end of intel_device_info_runtime_init().
> 
> I think it's fine to abstract it if you want, but it should *only* be
> called from that *one* place in one path.

I think for consistency GEN 7,8 should also just clear
runtime->pipe_mask depending on the later block to clear the other
fields.

> So that's a no for this one.

Ok, I can drop this patch.

> BR,
> Jani.
> 
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++-----------
> >  1 file changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 044ac552c9280..9d6d1fad9f1d9 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -369,6 +369,21 @@ void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
> >  	intel_device_info_subplatform_init(i915);
> >  }
> >  
> > +static void disable_fused_off_display(struct drm_i915_private *i915)
> > +{
> > +	struct intel_runtime_info *runtime = RUNTIME_INFO(i915);
> > +
> > +	drm_info(&i915->drm, "Display fused off, disabling\n");
> > +
> > +	runtime->pipe_mask = 0;
> > +	runtime->cpu_transcoder_mask = 0;
> > +	runtime->fbc_mask = 0;
> > +	runtime->has_hdcp = 0;
> > +	runtime->fbc_mask = 0;
> > +	runtime->has_dmc = 0;
> > +	runtime->has_dsc = 0;
> > +}
> > +
> >  /**
> >   * intel_device_info_runtime_init - initialize runtime info
> >   * @dev_priv: the i915 device
> > @@ -454,11 +469,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
> >  		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
> >  		    (HAS_PCH_CPT(dev_priv) &&
> >  		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
> > -			drm_info(&dev_priv->drm,
> > -				 "Display fused off, disabling\n");
> > -			runtime->pipe_mask = 0;
> > -			runtime->cpu_transcoder_mask = 0;
> > -			runtime->fbc_mask = 0;
> > +			disable_fused_off_display(dev_priv);
> >  		} else if (fuse_strap & IVB_PIPE_C_DISABLE) {
> >  			drm_info(&dev_priv->drm, "PipeC fused off\n");
> >  			runtime->pipe_mask &= ~BIT(PIPE_C);
> > @@ -502,17 +513,8 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
> >  	}
> >  
> >  	if ((IS_DGFX(dev_priv) || DISPLAY_VER(dev_priv) >= 14) &&
> > -	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT)) {
> > -		drm_info(&dev_priv->drm, "Display fused off, disabling\n");
> > -
> > -		runtime->pipe_mask = 0;
> > -		runtime->cpu_transcoder_mask = 0;
> > -		runtime->fbc_mask = 0;
> > -		runtime->has_hdcp = 0;
> > -		runtime->fbc_mask = 0;
> > -		runtime->has_dmc = 0;
> > -		runtime->has_dsc = 0;
> > -	}
> > +	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT))
> > +		disable_fused_off_display(dev_priv);
> >  
> >  	if (GRAPHICS_VER(dev_priv) == 6 && i915_vtd_active(dev_priv)) {
> >  		drm_info(&dev_priv->drm,
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 044ac552c9280..9d6d1fad9f1d9 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -369,6 +369,21 @@  void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
 	intel_device_info_subplatform_init(i915);
 }
 
+static void disable_fused_off_display(struct drm_i915_private *i915)
+{
+	struct intel_runtime_info *runtime = RUNTIME_INFO(i915);
+
+	drm_info(&i915->drm, "Display fused off, disabling\n");
+
+	runtime->pipe_mask = 0;
+	runtime->cpu_transcoder_mask = 0;
+	runtime->fbc_mask = 0;
+	runtime->has_hdcp = 0;
+	runtime->fbc_mask = 0;
+	runtime->has_dmc = 0;
+	runtime->has_dsc = 0;
+}
+
 /**
  * intel_device_info_runtime_init - initialize runtime info
  * @dev_priv: the i915 device
@@ -454,11 +469,7 @@  void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
 		    (HAS_PCH_CPT(dev_priv) &&
 		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
-			drm_info(&dev_priv->drm,
-				 "Display fused off, disabling\n");
-			runtime->pipe_mask = 0;
-			runtime->cpu_transcoder_mask = 0;
-			runtime->fbc_mask = 0;
+			disable_fused_off_display(dev_priv);
 		} else if (fuse_strap & IVB_PIPE_C_DISABLE) {
 			drm_info(&dev_priv->drm, "PipeC fused off\n");
 			runtime->pipe_mask &= ~BIT(PIPE_C);
@@ -502,17 +513,8 @@  void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 	}
 
 	if ((IS_DGFX(dev_priv) || DISPLAY_VER(dev_priv) >= 14) &&
-	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT)) {
-		drm_info(&dev_priv->drm, "Display fused off, disabling\n");
-
-		runtime->pipe_mask = 0;
-		runtime->cpu_transcoder_mask = 0;
-		runtime->fbc_mask = 0;
-		runtime->has_hdcp = 0;
-		runtime->fbc_mask = 0;
-		runtime->has_dmc = 0;
-		runtime->has_dsc = 0;
-	}
+	    !(intel_de_read(dev_priv, GU_CNTL_PROTECTED) & DEPRESENT))
+		disable_fused_off_display(dev_priv);
 
 	if (GRAPHICS_VER(dev_priv) == 6 && i915_vtd_active(dev_priv)) {
 		drm_info(&dev_priv->drm,