On Mon, 11 Nov 2019, Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> wrote:
> We need to configure TE interrupt in two places.
> Port interrupt and DSI interrupt mask registers.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 58 +++++++++++++++++++++++++++++++--
> 1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index dae00f7dd7df..f27afde409bf 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -41,6 +41,7 @@
> #include "display/intel_hotplug.h"
> #include "display/intel_lpe_audio.h"
> #include "display/intel_psr.h"
> +#include "display/intel_dsi.h"
>
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_irq.h"
> @@ -2571,12 +2572,45 @@ int ilk_enable_vblank(struct drm_crtc *crtc)
> return 0;
> }
>
> +static void gen11_dsi_configure_te(struct drm_i915_private *dev_priv,
> + struct drm_display_mode *mode, bool enable)
> +{
> + enum port port;
> + u32 tmp;
> +
> + if (mode->private_flags & I915_MODE_FLAG_DSI_USE_TE1)
> + port = PORT_B;
> + else
> + port = PORT_A;
> +
> + tmp = I915_READ(DSI_INTR_MASK_REG(port));
> + if (enable)
> + tmp &= ~DSI_TE_EVENT;
> + else
> + tmp |= DSI_TE_EVENT;
> +
> + I915_WRITE(DSI_INTR_MASK_REG(port), tmp);
> +}
> +
> int bdw_enable_vblank(struct drm_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - enum pipe pipe = to_intel_crtc(crtc)->pipe;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum pipe pipe = intel_crtc->pipe;
> + struct drm_vblank_crtc *vblank;
> + struct drm_display_mode *mode;
> unsigned long irqflags;
>
> + vblank = &crtc->dev->vblank[drm_crtc_index(crtc)];
> + mode = &vblank->hwmode;
> +
> + if ((INTEL_GEN(dev_priv) >= 11) &&
> + (mode->private_flags &
> + (I915_MODE_FLAG_DSI_USE_TE1 | I915_MODE_FLAG_DSI_USE_TE0))) {
> + gen11_dsi_configure_te(dev_priv, mode, true);
> + return 0;
> + }
> +
I'd hide this more inside gen11_dsi_configure_te():
if (gen11_dsi_configure_te(crtc))
return 0;
and make that function early return false if neither TE flag is set. It
also doesn't have to check for gen, because you can trust those flags
are only set where it matters.
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -2642,9 +2676,22 @@ void ilk_disable_vblank(struct drm_crtc *crtc)
> void bdw_disable_vblank(struct drm_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - enum pipe pipe = to_intel_crtc(crtc)->pipe;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum pipe pipe = intel_crtc->pipe;
> + struct drm_vblank_crtc *vblank;
> + struct drm_display_mode *mode;
> unsigned long irqflags;
>
> + vblank = &crtc->dev->vblank[drm_crtc_index(crtc)];
> + mode = &vblank->hwmode;
> +
> + if ((INTEL_GEN(dev_priv) >= 11) &&
> + (mode->private_flags &
> + (I915_MODE_FLAG_DSI_USE_TE1 | I915_MODE_FLAG_DSI_USE_TE0))) {
> + gen11_dsi_configure_te(dev_priv, mode, false);
> + return;
> + }
> +
Ditto, keep this function clean.
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3350,6 +3397,13 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
> }
>
> + if (INTEL_GEN(dev_priv) >= 11) {
> + enum port port;
> +
> + if (intel_bios_is_dsi_present(dev_priv, &port))
> + de_port_masked |= DSI0_TE | DSI1_TE;
> + }
> +
Not really happy about this one, but perhaps acceptable for now.
BR,
Jani.
> for_each_pipe(dev_priv, pipe) {
> dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
