[PATCH v2 1/3] drm/radeon: Cleanup display interrupt handling for evergreen, si
The current code here is really, really bad. A huge amount of it looks to be copy pasted, it has some weird hatred of arrays and code sharing, switch cases everywhere for things that really don't need them, and it makes the file seem immensely more complex then it actually is. This is a pain for maintanence, and is vulnerable to more weird irq handling bugs. So, let's start cleaning this up a bit. Modify all of the IRQ handlers for evergreen/si so that they just use for loops. As well, we add a helper function radeon_irq_kms_set_irq_n_enabled(), whose purpose is just to update the state of registers that enable/disable interrupts while printing any changes to the set of enabled interrupts to the kernel log. Note in this commit, since vblank/vline irq acking is intertwined with page flip irq acking, we can't cut out all of the copy paste in evergreen/si_irq_ack() just yet. Changes since v1: - Preserve order we write back all registers Signed-off-by: Lyude--- drivers/gpu/drm/radeon/evergreen.c | 729 +++- drivers/gpu/drm/radeon/radeon.h | 13 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 35 ++ drivers/gpu/drm/radeon/si.c | 596 ++ 4 files changed, 314 insertions(+), 1059 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 0bf1035..44ac6d3 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -35,6 +35,10 @@ #include "evergreen_blit_shaders.h" #include "radeon_ucode.h" +#define DC_HPDx_CONTROL(x)(DC_HPD1_CONTROL + (x * 0xc)) +#define DC_HPDx_INT_CONTROL(x)(DC_HPD1_INT_CONTROL + (x * 0xc)) +#define DC_HPDx_INT_STATUS_REG(x) (DC_HPD1_INT_STATUS + (x * 0xc)) + /* * Indirect registers accessor */ @@ -1714,38 +1718,10 @@ void evergreen_pm_finish(struct radeon_device *rdev) */ bool evergreen_hpd_sense(struct radeon_device *rdev, enum radeon_hpd_id hpd) { - bool connected = false; - - switch (hpd) { - case RADEON_HPD_1: - if (RREG32(DC_HPD1_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_2: - if (RREG32(DC_HPD2_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_3: - if (RREG32(DC_HPD3_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_4: - if (RREG32(DC_HPD4_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_5: - if (RREG32(DC_HPD5_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_6: - if (RREG32(DC_HPD6_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - default: - break; - } + if (hpd == RADEON_HPD_NONE) + return false; - return connected; + return !!(RREG32(DC_HPDx_INT_STATUS_REG(hpd)) & DC_HPDx_SENSE); } /** @@ -1759,61 +1735,15 @@ bool evergreen_hpd_sense(struct radeon_device *rdev, enum radeon_hpd_id hpd) void evergreen_hpd_set_polarity(struct radeon_device *rdev, enum radeon_hpd_id hpd) { - u32 tmp; bool connected = evergreen_hpd_sense(rdev, hpd); - switch (hpd) { - case RADEON_HPD_1: - tmp = RREG32(DC_HPD1_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD1_INT_CONTROL, tmp); - break; - case RADEON_HPD_2: - tmp = RREG32(DC_HPD2_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD2_INT_CONTROL, tmp); - break; - case RADEON_HPD_3: - tmp = RREG32(DC_HPD3_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD3_INT_CONTROL, tmp); - break; - case RADEON_HPD_4: - tmp = RREG32(DC_HPD4_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD4_INT_CONTROL, tmp); - break; - case RADEON_HPD_5: - tmp = RREG32(DC_HPD5_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |=
[PATCH v2 1/3] drm/radeon: Cleanup display interrupt handling for evergreen, si
The current code here is really, really bad. A huge amount of it looks to be copy pasted, it has some weird hatred of arrays and code sharing, switch cases everywhere for things that really don't need them, and it makes the file seem immensely more complex then it actually is. This is a pain for maintanence, and is vulnerable to more weird irq handling bugs. So, let's start cleaning this up a bit. Modify all of the IRQ handlers for evergreen/si so that they just use for loops. As well, we add a helper function radeon_irq_kms_set_irq_n_enabled(), whose purpose is just to update the state of registers that enable/disable interrupts while printing any changes to the set of enabled interrupts to the kernel log. Note in this commit, since vblank/vline irq acking is intertwined with page flip irq acking, we can't cut out all of the copy paste in evergreen/si_irq_ack() just yet. Changes since v1: - Preserve order we write back all registers Signed-off-by: Lyude --- drivers/gpu/drm/radeon/evergreen.c | 729 +++- drivers/gpu/drm/radeon/radeon.h | 13 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 35 ++ drivers/gpu/drm/radeon/si.c | 596 ++ 4 files changed, 314 insertions(+), 1059 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 0bf1035..44ac6d3 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -35,6 +35,10 @@ #include "evergreen_blit_shaders.h" #include "radeon_ucode.h" +#define DC_HPDx_CONTROL(x)(DC_HPD1_CONTROL + (x * 0xc)) +#define DC_HPDx_INT_CONTROL(x)(DC_HPD1_INT_CONTROL + (x * 0xc)) +#define DC_HPDx_INT_STATUS_REG(x) (DC_HPD1_INT_STATUS + (x * 0xc)) + /* * Indirect registers accessor */ @@ -1714,38 +1718,10 @@ void evergreen_pm_finish(struct radeon_device *rdev) */ bool evergreen_hpd_sense(struct radeon_device *rdev, enum radeon_hpd_id hpd) { - bool connected = false; - - switch (hpd) { - case RADEON_HPD_1: - if (RREG32(DC_HPD1_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_2: - if (RREG32(DC_HPD2_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_3: - if (RREG32(DC_HPD3_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_4: - if (RREG32(DC_HPD4_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_5: - if (RREG32(DC_HPD5_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - case RADEON_HPD_6: - if (RREG32(DC_HPD6_INT_STATUS) & DC_HPDx_SENSE) - connected = true; - break; - default: - break; - } + if (hpd == RADEON_HPD_NONE) + return false; - return connected; + return !!(RREG32(DC_HPDx_INT_STATUS_REG(hpd)) & DC_HPDx_SENSE); } /** @@ -1759,61 +1735,15 @@ bool evergreen_hpd_sense(struct radeon_device *rdev, enum radeon_hpd_id hpd) void evergreen_hpd_set_polarity(struct radeon_device *rdev, enum radeon_hpd_id hpd) { - u32 tmp; bool connected = evergreen_hpd_sense(rdev, hpd); - switch (hpd) { - case RADEON_HPD_1: - tmp = RREG32(DC_HPD1_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD1_INT_CONTROL, tmp); - break; - case RADEON_HPD_2: - tmp = RREG32(DC_HPD2_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD2_INT_CONTROL, tmp); - break; - case RADEON_HPD_3: - tmp = RREG32(DC_HPD3_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD3_INT_CONTROL, tmp); - break; - case RADEON_HPD_4: - tmp = RREG32(DC_HPD4_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |= DC_HPDx_INT_POLARITY; - WREG32(DC_HPD4_INT_CONTROL, tmp); - break; - case RADEON_HPD_5: - tmp = RREG32(DC_HPD5_INT_CONTROL); - if (connected) - tmp &= ~DC_HPDx_INT_POLARITY; - else - tmp |=