Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't request GMBUS to generate irqs when called while irqs are off

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Fri, Oct 29, 2021 at 10:18:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We will need to do some i2c poking from the encoder->shutdown() hook.
> Currently that gets called after irqs have been turned off. We still
> poll the gmbus status bits even if the interrupt never arrives so
> things will work just fine. But seems like asking gmbus to generate
> interrupts we will never see is a bit pointless, so don't.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index ceb1bf8a8c3c..3b8b84177085 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -334,6 +334,15 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>  	algo->data = bus;
>  }
>  
> +static bool has_gmbus_irq(struct drm_i915_private *i915)
> +{
> +	/*
> +	 * encoder->shutdown() may want to use GMBUS
> +	 * after irqs have already been disabled.
> +	 */
> +	return HAS_GMBUS_IRQ(i915) && intel_irqs_enabled(i915);
> +}
> +
>  static int gmbus_wait(struct drm_i915_private *dev_priv, u32 status, u32 irq_en)
>  {
>  	DEFINE_WAIT(wait);
> @@ -344,7 +353,7 @@ static int gmbus_wait(struct drm_i915_private *dev_priv, u32 status, u32 irq_en)
>  	 * we also need to check for NAKs besides the hw ready/idle signal, we
>  	 * need to wake up periodically and check that ourselves.
>  	 */
> -	if (!HAS_GMBUS_IRQ(dev_priv))
> +	if (!has_gmbus_irq(dev_priv))
>  		irq_en = 0;
>  
>  	add_wait_queue(&dev_priv->gmbus_wait_queue, &wait);
> @@ -375,7 +384,7 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv)
>  
>  	/* Important: The hw handles only the first bit, so set only one! */
>  	irq_enable = 0;
> -	if (HAS_GMBUS_IRQ(dev_priv))
> +	if (has_gmbus_irq(dev_priv))
>  		irq_enable = GMBUS_IDLE_EN;
>  
>  	add_wait_queue(&dev_priv->gmbus_wait_queue, &wait);
> -- 
> 2.32.0
>