Re: [PATCH 3/5] drm/i915: check for more ASLC interrupts

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 12 Sep 2013, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Sometimes I see the "non asle set request??" message on my Haswell
> machine, so I decided to get the spec and see if some bits are missing
> from the mask. We do have some bits missing from the mask, so this
> patch adds them. But I still see the "non asle set request??" message
> on my machine :(
>
> Also use the proper ASLC name to indicate the registers we're talking
> about.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index c4fb2ae..a01e0f8 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -117,12 +117,17 @@ struct opregion_asle {
>  #define ASLE_ARDY_READY		(1 << 0)
>  #define ASLE_ARDY_NOT_READY	(0 << 0)
>  
> -/* ASLE irq request bits */
> -#define ASLE_SET_ALS_ILLUM     (1 << 0)
> -#define ASLE_SET_BACKLIGHT     (1 << 1)
> -#define ASLE_SET_PFIT          (1 << 2)
> -#define ASLE_SET_PWM_FREQ      (1 << 3)
> -#define ASLE_REQ_MSK           0xf
> +/* ASLE Interrupt Command (ASLC) bits */
> +#define ASLC_SET_ALS_ILLUM		(1 << 0)
> +#define ASLC_SET_BACKLIGHT		(1 << 1)
> +#define ASLC_SET_PFIT			(1 << 2)
> +#define ASLC_SET_PWM_FREQ		(1 << 3)
> +#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
> +#define ASLC_BUTTON_ARRAY		(1 << 5)
> +#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
> +#define ASLC_DOCKING_INDICATOR		(1 << 7)
> +#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
> +#define ASLC_REQ_MSK			0x1ff

While at it, please also s/ASLE/ASLC/ on the response bits, and add the
correspoinding response defines for the added bits above.

>  /* response bits of ASLE irq request */
>  #define ASLE_ALS_ILLUM_FAILED	(1<<10)
> @@ -421,25 +426,31 @@ void intel_opregion_asle_intr(struct drm_device *dev)
>  	if (!asle)
>  		return;
>  
> -	asle_req = ioread32(&asle->aslc) & ASLE_REQ_MSK;
> +	asle_req = ioread32(&asle->aslc) & ASLC_REQ_MSK;
>  
>  	if (!asle_req) {
>  		DRM_DEBUG_DRIVER("non asle set request??\n");

Maybe we should log the original ioread32(&asle->aslc) in this case.

>  		return;
>  	}
>  
> -	if (asle_req & ASLE_SET_ALS_ILLUM)
> +	if (asle_req & ASLC_SET_ALS_ILLUM)
>  		asle_stat |= asle_set_als_illum(dev, ioread32(&asle->alsi));
>  
> -	if (asle_req & ASLE_SET_BACKLIGHT)
> +	if (asle_req & ASLC_SET_BACKLIGHT)
>  		asle_stat |= asle_set_backlight(dev, ioread32(&asle->bclp));
>  
> -	if (asle_req & ASLE_SET_PFIT)
> +	if (asle_req & ASLC_SET_PFIT)
>  		asle_stat |= asle_set_pfit(dev, ioread32(&asle->pfit));
>  
> -	if (asle_req & ASLE_SET_PWM_FREQ)
> +	if (asle_req & ASLC_SET_PWM_FREQ)
>  		asle_stat |= asle_set_pwm_freq(dev, ioread32(&asle->pfmb));
>  
> +	if (asle_req & (ASLC_SUPPORTED_ROTATION_ANGLES | ASLC_BUTTON_ARRAY |
> +			ASLC_CONVERTIBLE_INDICATOR | ASLC_DOCKING_INDICATOR |
> +			ASLC_ISCT_STATE_CHANGE)) {
> +		DRM_DEBUG_DRIVER("ASLC interrupt not supported\n");

Should we report failure for these, similar to what everything other
than the backlight request do?

BR,
Jani.

> +	}
> +
>  	iowrite32(asle_stat, &asle->aslc);
>  }
>  
> -- 
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center