[PATCH 4/8] drm/i915: Add support to identify the SCDC Interrupt from IIR

[Shankar, Uma]

> -----Original Message-----
> From: Intel-gfx-trybot <intel-gfx-trybot-bounces at lists.freedesktop.org> On Behalf
> Of Ankit Nautiyal
> Sent: Friday, March 4, 2022 12:22 PM
> To: intel-gfx-trybot at lists.freedesktop.org
> Subject: [PATCH 4/8] drm/i915: Add support to identify the SCDC Interrupt from IIR
> 
> This patch identifies the SCDC Interrupt and schedules the bottomhalf by adding scdc
> work function in the system work queue.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 28 ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_hdmi.h |  1 +
>  drivers/gpu/drm/i915/i915_irq.c           | 32 +++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 1aa5bdc7b0dc..bbbc2b63aa62 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3213,3 +3213,31 @@ intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int
> slice_width, int num_slices,
> 
>  	return 0;
>  }
> +
> +enum scdc_pin intel_hdmi_get_scdc_pin(struct intel_encoder *encoder) {
> +	switch (encoder->hpd_pin) {
> +	case HPD_PORT_A:
> +		return SCDC_PORT_A;
> +	case HPD_PORT_B:
> +		return SCDC_PORT_B;
> +	case HPD_PORT_C:
> +		return SCDC_PORT_C;
> +	case HPD_PORT_TC1:
> +		return SCDC_PORT_TC1;
> +	case HPD_PORT_TC2:
> +		return SCDC_PORT_TC2;
> +	case HPD_PORT_TC3:
> +		return SCDC_PORT_TC3;
> +	case HPD_PORT_TC4:
> +		return SCDC_PORT_TC4;
> +	case HPD_PORT_TC5:
> +		return SCDC_PORT_TC5;
> +	case HPD_PORT_TC6:
> +		return SCDC_PORT_TC6;
> +	default:
> +		break;
> +	}
> +
> +	return SCDC_PORT_NONE;

Is this defined, I feel you have been using SCDC_NUM_PINS here in your github branch.
May be define Einval case there. You may have handled this already, just confirm and get
this updated in the series.

> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h
> b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index 93f65a917c36..49da11b30632 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -54,5 +54,6 @@ int intel_hdmi_dsc_get_num_slices(const struct
> intel_crtc_state *crtc_state,
>  				  int src_max_slices, int src_max_slice_width,
>  				  int hdmi_max_slices, int hdmi_throughput);  int
> intel_hdmi_dsc_get_slice_height(int vactive);
> +enum scdc_pin intel_hdmi_get_scdc_pin(struct intel_encoder *encoder);
> 
>  #endif /* __INTEL_HDMI_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index
> 4dc2542146cd..dad32edab874 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -40,6 +40,7 @@
>  #include "display/intel_display_types.h"
>  #include "display/intel_fifo_underrun.h"
>  #include "display/intel_hotplug.h"
> +#include "display/intel_hdmi.h"
>  #include "display/intel_lpe_audio.h"
>  #include "display/intel_psr.h"
> 
> @@ -1993,6 +1994,34 @@ static void cpt_irq_handler(struct drm_i915_private
> *dev_priv, u32 pch_iir)
>  		cpt_serr_int_handler(dev_priv);
>  }
> 
> +static void scdc_irq_handler(struct drm_i915_private *dev_priv, u32
> +pch_iir) {
> +	struct intel_encoder *encoder;
> +	enum scdc_pin pin;
> +	bool scdc_interrupt = false;
> +
> +	/*
> +	 * Identify the port on which scdc interrupt received
> +	 * #TODO Check if spin lock be used here before this loop to avoid
> +	 * race condition for event_bits?
> +	 */
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		if (encoder->type != INTEL_OUTPUT_HDMI)
> +			continue;

Leave a line gap

> +		pin = intel_hdmi_get_scdc_pin(encoder);
> +		if (pin == SCDC_PORT_NONE)
> +			continue;

Same here

> +		if (pch_iir & dev_priv->scdc_rr.scdc[pin] ||
> +		    pch_iir & dev_priv->scdc_rr.scdc[pin]) {

This got repeated, you missed TC here.

> +			dev_priv->scdc_rr.event_bits |= pin;
> +			scdc_interrupt = true;
> +			break;
> +		}
> +	}

Leave a line gap

> +	if (scdc_interrupt)
> +		queue_work(system_wq, &dev_priv->scdc_rr.scdc_rr_work); }
> +
>  static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)  {
>  	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_HOTPLUG_MASK_ICP; @@ -
> 2028,6 +2057,9 @@ static void icp_irq_handler(struct drm_i915_private *dev_priv,
> u32 pch_iir)
> 
>  	if (pch_iir & SDE_GMBUS_ICP)
>  		gmbus_irq_handler(dev_priv);
> +
> +	if (pch_iir & SDE_DDI_SCDC_MASK_ICP || pch_iir &
> SDE_TC_SCDC_MASK_ICP)
> +		scdc_irq_handler(dev_priv, pch_iir);
>  }
> 
>  static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> --
> 2.25.1