Re: [PATCH] drm/radeon: use pflip irq on R600+

Christian König <deathsimple@xxxxxxxxxxx> · Fri, 02 May 2014 18:23:05 +0200

Am 02.05.2014 17:24, schrieb Alex Deucher:
On Mon, Apr 28, 2014 at 4:25 AM, Christian König
<deathsimple@xxxxxxxxxxx> wrote:
From: Christian König <christian.koenig@xxxxxxx>

Testing the update pending bit directly after issuing an
update is nonsense cause depending on the pixel clock the
CRTC needs a bit of time to execute the flip even when we
are in the VBLANK period.

This is just a non invasive patch to solve the problem at
hand, a more complete and cleaner solution should follow
in the next merge window.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76564

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
  drivers/gpu/drm/radeon/cik.c            | 76 +++++++++++++++++++++++++++++++++
  drivers/gpu/drm/radeon/cikd.h           |  9 ++++
  drivers/gpu/drm/radeon/evergreen.c      | 28 +++++++++---
  drivers/gpu/drm/radeon/r600.c           | 10 +++--
  drivers/gpu/drm/radeon/radeon.h         |  6 +++
  drivers/gpu/drm/radeon/radeon_display.c |  4 ++
  drivers/gpu/drm/radeon/si.c             | 28 +++++++++---
  7 files changed, 144 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 199eb19..ff1cad5 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -6693,6 +6693,19 @@ static void cik_disable_interrupt_state(struct radeon_device *rdev)
                 WREG32(LB_INTERRUPT_MASK + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
                 WREG32(LB_INTERRUPT_MASK + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
         }
+       /* pflip */
+       if (rdev->num_crtc >= 2) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, 0);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, 0);
+       }
+       if (rdev->num_crtc >= 4) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, 0);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, 0);
+       }
+       if (rdev->num_crtc >= 6) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
+       }

         /* dac hotplug */
         WREG32(DAC_AUTODETECT_INT_CONTROL, 0);
@@ -7049,6 +7062,25 @@ int cik_irq_set(struct radeon_device *rdev)
                 WREG32(LB_INTERRUPT_MASK + EVERGREEN_CRTC5_REGISTER_OFFSET, crtc6);
         }

+       if (rdev->num_crtc >= 2) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+       }
+       if (rdev->num_crtc >= 4) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+       }
+       if (rdev->num_crtc >= 6) {
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+       }
+

This will enable pflip interrupts all the time.  Shouldn't we tie this
the pflip interrupt enable flag? Otherwise, we may keep waking the CPU
even when we don't care about page flips, depending on when this
interrupt fires (every vupdate or only when there is a new base
address).

It only fires when there is a new base address, so I thought that it 
would be unnecessary to enable/disable it all the time which seems to 
take quite some time as well.

Christian.


Alex

         WREG32(DC_HPD1_INT_CONTROL, hpd1);
         WREG32(DC_HPD2_INT_CONTROL, hpd2);
         WREG32(DC_HPD3_INT_CONTROL, hpd3);
@@ -7085,6 +7117,29 @@ static inline void cik_irq_ack(struct radeon_device *rdev)
         rdev->irq.stat_regs.cik.disp_int_cont5 = RREG32(DISP_INTERRUPT_STATUS_CONTINUE5);
         rdev->irq.stat_regs.cik.disp_int_cont6 = RREG32(DISP_INTERRUPT_STATUS_CONTINUE6);

+       rdev->irq.stat_regs.cik.d1grph_int = RREG32(GRPH_INT_STATUS +
+               EVERGREEN_CRTC0_REGISTER_OFFSET);
+       rdev->irq.stat_regs.cik.d2grph_int = RREG32(GRPH_INT_STATUS +
+               EVERGREEN_CRTC1_REGISTER_OFFSET);
+       if (rdev->num_crtc >= 4) {
+               rdev->irq.stat_regs.cik.d3grph_int = RREG32(GRPH_INT_STATUS +
+                       EVERGREEN_CRTC2_REGISTER_OFFSET);
+               rdev->irq.stat_regs.cik.d4grph_int = RREG32(GRPH_INT_STATUS +
+                       EVERGREEN_CRTC3_REGISTER_OFFSET);
+       }
+       if (rdev->num_crtc >= 6) {
+               rdev->irq.stat_regs.cik.d5grph_int = RREG32(GRPH_INT_STATUS +
+                       EVERGREEN_CRTC4_REGISTER_OFFSET);
+               rdev->irq.stat_regs.cik.d6grph_int = RREG32(GRPH_INT_STATUS +
+                       EVERGREEN_CRTC5_REGISTER_OFFSET);
+       }
+
+       if (rdev->irq.stat_regs.cik.d1grph_int & GRPH_PFLIP_INT_OCCURRED)
+               WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_CLEAR);
+       if (rdev->irq.stat_regs.cik.d2grph_int & GRPH_PFLIP_INT_OCCURRED)
+               WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC1_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_CLEAR);
         if (rdev->irq.stat_regs.cik.disp_int & LB_D1_VBLANK_INTERRUPT)
                 WREG32(LB_VBLANK_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET, VBLANK_ACK);
         if (rdev->irq.stat_regs.cik.disp_int & LB_D1_VLINE_INTERRUPT)
@@ -7095,6 +7150,12 @@ static inline void cik_irq_ack(struct radeon_device *rdev)
                 WREG32(LB_VLINE_STATUS + EVERGREEN_CRTC1_REGISTER_OFFSET, VLINE_ACK);

         if (rdev->num_crtc >= 4) {
+               if (rdev->irq.stat_regs.cik.d3grph_int & GRPH_PFLIP_INT_OCCURRED)
+                       WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC2_REGISTER_OFFSET,
+                              GRPH_PFLIP_INT_CLEAR);
+               if (rdev->irq.stat_regs.cik.d4grph_int & GRPH_PFLIP_INT_OCCURRED)
+                       WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC3_REGISTER_OFFSET,
+                              GRPH_PFLIP_INT_CLEAR);
                 if (rdev->irq.stat_regs.cik.disp_int_cont2 & LB_D3_VBLANK_INTERRUPT)
                         WREG32(LB_VBLANK_STATUS + EVERGREEN_CRTC2_REGISTER_OFFSET, VBLANK_ACK);
                 if (rdev->irq.stat_regs.cik.disp_int_cont2 & LB_D3_VLINE_INTERRUPT)
@@ -7106,6 +7167,12 @@ static inline void cik_irq_ack(struct radeon_device *rdev)
         }

         if (rdev->num_crtc >= 6) {
+               if (rdev->irq.stat_regs.cik.d5grph_int & GRPH_PFLIP_INT_OCCURRED)
+                       WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC4_REGISTER_OFFSET,
+                              GRPH_PFLIP_INT_CLEAR);
+               if (rdev->irq.stat_regs.cik.d6grph_int & GRPH_PFLIP_INT_OCCURRED)
+                       WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC5_REGISTER_OFFSET,
+                              GRPH_PFLIP_INT_CLEAR);
                 if (rdev->irq.stat_regs.cik.disp_int_cont4 & LB_D5_VBLANK_INTERRUPT)
                         WREG32(LB_VBLANK_STATUS + EVERGREEN_CRTC4_REGISTER_OFFSET, VBLANK_ACK);
                 if (rdev->irq.stat_regs.cik.disp_int_cont4 & LB_D5_VLINE_INTERRUPT)
@@ -7457,6 +7524,15 @@ restart_ih:
                                 break;
                         }
                         break;
+               case 8: /* D1 page flip */
+               case 9: /* D2 page flip */
+               case 10: /* D3 page flip */
+               case 11: /* D4 page flip */
+               case 12: /* D5 page flip */
+               case 13: /* D6 page flip */
+                       DRM_DEBUG("IH: D%d flip\n", src_id - 7);
+                       radeon_crtc_handle_flip(rdev, src_id - 8);
+                       break;
                 case 42: /* HPD hotplug */
                         switch (src_data) {
                         case 0:
diff --git a/drivers/gpu/drm/radeon/cikd.h b/drivers/gpu/drm/radeon/cikd.h
index 2138732..dd79263 100644
--- a/drivers/gpu/drm/radeon/cikd.h
+++ b/drivers/gpu/drm/radeon/cikd.h
@@ -888,6 +888,15 @@
  #       define DC_HPD6_RX_INTERRUPT                     (1 << 18)
  #define DISP_INTERRUPT_STATUS_CONTINUE6                 0x6780

+/* 0x6858, 0x7458, 0x10058, 0x10c58, 0x11858, 0x12458 */
+#define GRPH_INT_STATUS                                 0x6858
+#       define GRPH_PFLIP_INT_OCCURRED                  (1 << 0)
+#       define GRPH_PFLIP_INT_CLEAR                     (1 << 8)
+/* 0x685c, 0x745c, 0x1005c, 0x10c5c, 0x1185c, 0x1245c */
+#define GRPH_INT_CONTROL                                0x685c
+#       define GRPH_PFLIP_INT_MASK                      (1 << 0)
+#       define GRPH_PFLIP_INT_TYPE                      (1 << 8)
+
  #define        DAC_AUTODETECT_INT_CONTROL                      0x67c8

  #define DC_HPD1_INT_STATUS                              0x601c
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index b406546..e4fed35 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -4371,7 +4371,6 @@ int evergreen_irq_set(struct radeon_device *rdev)
         u32 crtc1 = 0, crtc2 = 0, crtc3 = 0, crtc4 = 0, crtc5 = 0, crtc6 = 0;
         u32 hpd1, hpd2, hpd3, hpd4, hpd5, hpd6;
         u32 grbm_int_cntl = 0;
-       u32 grph1 = 0, grph2 = 0, grph3 = 0, grph4 = 0, grph5 = 0, grph6 = 0;
         u32 afmt1 = 0, afmt2 = 0, afmt3 = 0, afmt4 = 0, afmt5 = 0, afmt6 = 0;
         u32 dma_cntl, dma_cntl1 = 0;
         u32 thermal_int = 0;
@@ -4554,15 +4553,21 @@ int evergreen_irq_set(struct radeon_device *rdev)
                 WREG32(INT_MASK + EVERGREEN_CRTC5_REGISTER_OFFSET, crtc6);
         }

-       WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, grph1);
-       WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, grph2);
+       WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET,
+              GRPH_PFLIP_INT_MASK);
+       WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET,
+              GRPH_PFLIP_INT_MASK);
         if (rdev->num_crtc >= 4) {
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, grph3);
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, grph4);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
         }
         if (rdev->num_crtc >= 6) {
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, grph5);
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, grph6);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
         }

         WREG32(DC_HPD1_INT_CONTROL, hpd1);
@@ -4951,6 +4956,15 @@ restart_ih:
                                 break;
                         }
                         break;
+               case 8: /* D1 page flip */
+               case 9: /* D2 page flip */
+               case 10: /* D3 page flip */
+               case 11: /* D4 page flip */
+               case 12: /* D5 page flip */
+               case 13: /* D6 page flip */
+                       DRM_DEBUG("IH: D%d flip\n", src_id - 7);
+                       radeon_crtc_handle_flip(rdev, src_id - 8);
+                       break;
                 case 42: /* HPD hotplug */
                         switch (src_data) {
                         case 0:
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 6e887d0..c4ccd2a 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3505,7 +3505,6 @@ int r600_irq_set(struct radeon_device *rdev)
         u32 hpd1, hpd2, hpd3, hpd4 = 0, hpd5 = 0, hpd6 = 0;
         u32 grbm_int_cntl = 0;
         u32 hdmi0, hdmi1;
-       u32 d1grph = 0, d2grph = 0;
         u32 dma_cntl;
         u32 thermal_int = 0;

@@ -3614,8 +3613,8 @@ int r600_irq_set(struct radeon_device *rdev)
         WREG32(CP_INT_CNTL, cp_int_cntl);
         WREG32(DMA_CNTL, dma_cntl);
         WREG32(DxMODE_INT_MASK, mode_int);
-       WREG32(D1GRPH_INTERRUPT_CONTROL, d1grph);
-       WREG32(D2GRPH_INTERRUPT_CONTROL, d2grph);
+       WREG32(D1GRPH_INTERRUPT_CONTROL, DxGRPH_PFLIP_INT_MASK);
+       WREG32(D2GRPH_INTERRUPT_CONTROL, DxGRPH_PFLIP_INT_MASK);
         WREG32(GRBM_INT_CNTL, grbm_int_cntl);
         if (ASIC_IS_DCE3(rdev)) {
                 WREG32(DC_HPD1_INT_CONTROL, hpd1);
@@ -3918,6 +3917,11 @@ restart_ih:
                                 break;
                         }
                         break;
+               case 9: /* D1 pflip */
+               case 10: /* D2 pflip */
+                       DRM_DEBUG("IH: D%d flip\n", src_id - 8);
+                       radeon_crtc_handle_flip(rdev, src_id - 9);
+                       break;
                 case 19: /* HPD/DAC hotplug */
                         switch (src_data) {
                         case 0:
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b58e1af..6852861 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -730,6 +730,12 @@ struct cik_irq_stat_regs {
         u32 disp_int_cont4;
         u32 disp_int_cont5;
         u32 disp_int_cont6;
+       u32 d1grph_int;
+       u32 d2grph_int;
+       u32 d3grph_int;
+       u32 d4grph_int;
+       u32 d5grph_int;
+       u32 d6grph_int;
  };

  union radeon_irq_stat_regs {
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 8d99d5e..14bd701 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -284,6 +284,10 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
         u32 update_pending;
         int vpos, hpos;

+       /* can happen during initialization */
+       if (radeon_crtc == NULL)
+               return;
+
         spin_lock_irqsave(&rdev->ddev->event_lock, flags);
         work = radeon_crtc->unpin_work;
         if (work == NULL ||
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index ac708e0..4654130 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -5780,7 +5780,6 @@ int si_irq_set(struct radeon_device *rdev)
         u32 crtc1 = 0, crtc2 = 0, crtc3 = 0, crtc4 = 0, crtc5 = 0, crtc6 = 0;
         u32 hpd1 = 0, hpd2 = 0, hpd3 = 0, hpd4 = 0, hpd5 = 0, hpd6 = 0;
         u32 grbm_int_cntl = 0;
-       u32 grph1 = 0, grph2 = 0, grph3 = 0, grph4 = 0, grph5 = 0, grph6 = 0;
         u32 dma_cntl, dma_cntl1;
         u32 thermal_int = 0;

@@ -5919,16 +5918,22 @@ int si_irq_set(struct radeon_device *rdev)
         }

         if (rdev->num_crtc >= 2) {
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, grph1);
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, grph2);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
         }
         if (rdev->num_crtc >= 4) {
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, grph3);
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, grph4);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
         }
         if (rdev->num_crtc >= 6) {
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, grph5);
-               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, grph6);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
+               WREG32(GRPH_INT_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET,
+                      GRPH_PFLIP_INT_MASK);
         }

         if (!ASIC_IS_NODCE(rdev)) {
@@ -6292,6 +6297,15 @@ restart_ih:
                                 break;
                         }
                         break;
+               case 8: /* D1 page flip */
+               case 9: /* D2 page flip */
+               case 10: /* D3 page flip */
+               case 11: /* D4 page flip */
+               case 12: /* D5 page flip */
+               case 13: /* D6 page flip */
+                       DRM_DEBUG("IH: D%d flip\n", src_id - 7);
+                       radeon_crtc_handle_flip(rdev, src_id - 8);
+                       break;
                 case 42: /* HPD hotplug */
                         switch (src_data) {
                         case 0:
--
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel








