Re: [PATCH 02/10] drm/i915: extract ilk_display_irq_handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/7/19 Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>:
> Paulo Zanoni <przanoni@xxxxxxxxx> writes:
>
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> It's the code that deals with de_iir.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Minor observation on the whole irq stuff: ilk, ironlake and ivb,
> ivybridge are both used and I couldn't figure out the pattern.

You mean the function names, right? This is something that confuses me
too, all our code is inconsistent and I'd like to know what's the
preferred way. We seem to use both ways: vlv_find_best_dpll vs
valleyview_crtc_enable, ironlake_crtc_mode_set vs ilk_update_plane,
haswell_modeset_global_resources vs ivb_modeset_global_resources. I
tend to prefer the shorter versions because I believe these acronyms
should be obvious (or easy to discover) for people reading our code,
but having some guidance on which one to use on each case would be
good. Opinions?

>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>

Thanks for all the reviews!

>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 104 +++++++++++++++++++++-------------------
>>  1 file changed, 56 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index f397f9a..39160a2 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1202,6 +1202,60 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>>               cpt_serr_int_handler(dev);
>>  }
>>
>> +static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
>> +{
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +     if (de_iir & DE_AUX_CHANNEL_A)
>> +             dp_aux_irq_handler(dev);
>> +
>> +     if (de_iir & DE_GSE)
>> +             intel_opregion_asle_intr(dev);
>> +
>> +     if (de_iir & DE_PIPEA_VBLANK)
>> +             drm_handle_vblank(dev, 0);
>> +
>> +     if (de_iir & DE_PIPEB_VBLANK)
>> +             drm_handle_vblank(dev, 1);
>> +
>> +     if (de_iir & DE_POISON)
>> +             DRM_ERROR("Poison interrupt\n");
>> +
>> +     if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
>> +             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
>> +                     DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
>> +
>> +     if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
>> +             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
>> +                     DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
>> +
>> +     if (de_iir & DE_PLANEA_FLIP_DONE) {
>> +             intel_prepare_page_flip(dev, 0);
>> +             intel_finish_page_flip_plane(dev, 0);
>> +     }
>> +
>> +     if (de_iir & DE_PLANEB_FLIP_DONE) {
>> +             intel_prepare_page_flip(dev, 1);
>> +             intel_finish_page_flip_plane(dev, 1);
>> +     }
>> +
>> +     /* check event from PCH */
>> +     if (de_iir & DE_PCH_EVENT) {
>> +             u32 pch_iir = I915_READ(SDEIIR);
>> +
>> +             if (HAS_PCH_CPT(dev))
>> +                     cpt_irq_handler(dev, pch_iir);
>> +             else
>> +                     ibx_irq_handler(dev, pch_iir);
>> +
>> +             /* should clear PCH hotplug event before clear CPU irq */
>> +             I915_WRITE(SDEIIR, pch_iir);
>> +     }
>> +
>> +     if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
>> +             ironlake_rps_change_irq_handler(dev);
>> +}
>> +
>>  static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
>>  {
>>       struct drm_device *dev = (struct drm_device *) arg;
>> @@ -1360,54 +1414,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
>>       else
>>               snb_gt_irq_handler(dev, dev_priv, gt_iir);
>>
>> -     if (de_iir & DE_AUX_CHANNEL_A)
>> -             dp_aux_irq_handler(dev);
>> -
>> -     if (de_iir & DE_GSE)
>> -             intel_opregion_asle_intr(dev);
>> -
>> -     if (de_iir & DE_PIPEA_VBLANK)
>> -             drm_handle_vblank(dev, 0);
>> -
>> -     if (de_iir & DE_PIPEB_VBLANK)
>> -             drm_handle_vblank(dev, 1);
>> -
>> -     if (de_iir & DE_POISON)
>> -             DRM_ERROR("Poison interrupt\n");
>> -
>> -     if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
>> -             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
>> -                     DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
>> -
>> -     if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
>> -             if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
>> -                     DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
>> -
>> -     if (de_iir & DE_PLANEA_FLIP_DONE) {
>> -             intel_prepare_page_flip(dev, 0);
>> -             intel_finish_page_flip_plane(dev, 0);
>> -     }
>> -
>> -     if (de_iir & DE_PLANEB_FLIP_DONE) {
>> -             intel_prepare_page_flip(dev, 1);
>> -             intel_finish_page_flip_plane(dev, 1);
>> -     }
>> -
>> -     /* check event from PCH */
>> -     if (de_iir & DE_PCH_EVENT) {
>> -             u32 pch_iir = I915_READ(SDEIIR);
>> -
>> -             if (HAS_PCH_CPT(dev))
>> -                     cpt_irq_handler(dev, pch_iir);
>> -             else
>> -                     ibx_irq_handler(dev, pch_iir);
>> -
>> -             /* should clear PCH hotplug event before clear CPU irq */
>> -             I915_WRITE(SDEIIR, pch_iir);
>> -     }
>> -
>> -     if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
>> -             ironlake_rps_change_irq_handler(dev);
>> +     if (de_iir)
>> +             ilk_display_irq_handler(dev, de_iir);
>>
>>       if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
>>               gen6_rps_irq_handler(dev_priv, pm_iir);
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux