| Message ID | 1365895584-20999-3-git-send-email-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Sonntag, den 14.04.2013, 01:26 +0200 schrieb Rafa? Mi?ecki: > We need interrupts on format change for R6xx only, where hardware seems > to be somehow bugged and requires setting audio info manually. How should this be tested? > Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> > --- > drivers/gpu/drm/radeon/evergreen.c | 127 +----------------------------------- > 1 file changed, 1 insertion(+), 126 deletions(-) Thanks, Paul
2013/4/14 Paul Menzel <paulepanter@users.sourceforge.net>: > Am Sonntag, den 14.04.2013, 01:26 +0200 schrieb Rafa? Mi?ecki: >> We need interrupts on format change for R6xx only, where hardware seems >> to be somehow bugged and requires setting audio info manually. > > How should this be tested? Just play some audio using different settings (like LPCM, AC3, DTS, various rates). On R6xx we had to adjust some registers on every format change. It's not needed on Evergreen, GPU setup itself automatically.
On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > We need interrupts on format change for R6xx only, where hardware seems > to be somehow bugged and requires setting audio info manually. Can you confirm that this is actually needed on older chips? AFAIK, it shouldn't be required for any chips. It's mainly for debugging. Alex > > Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> > --- > drivers/gpu/drm/radeon/evergreen.c | 127 +----------------------------------- > 1 file changed, 1 insertion(+), 126 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c > index 305a657..34d4347 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -2702,7 +2702,6 @@ int evergreen_irq_set(struct radeon_device *rdev) > 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; > > if (!rdev->irq.installed) { > @@ -2724,13 +2723,6 @@ int evergreen_irq_set(struct radeon_device *rdev) > hpd5 = RREG32(DC_HPD5_INT_CONTROL) & ~DC_HPDx_INT_EN; > hpd6 = RREG32(DC_HPD6_INT_CONTROL) & ~DC_HPDx_INT_EN; > > - afmt1 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - afmt2 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - afmt3 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - afmt4 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - afmt5 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - afmt6 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; > - > dma_cntl = RREG32(DMA_CNTL) & ~TRAP_ENABLE; > > if (rdev->family >= CHIP_CAYMAN) { > @@ -2822,30 +2814,6 @@ int evergreen_irq_set(struct radeon_device *rdev) > DRM_DEBUG("evergreen_irq_set: hpd 6\n"); > hpd6 |= DC_HPDx_INT_EN; > } > - if (rdev->irq.afmt[0]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 0\n"); > - afmt1 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > - if (rdev->irq.afmt[1]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 1\n"); > - afmt2 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > - if (rdev->irq.afmt[2]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 2\n"); > - afmt3 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > - if (rdev->irq.afmt[3]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 3\n"); > - afmt4 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > - if (rdev->irq.afmt[4]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 4\n"); > - afmt5 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > - if (rdev->irq.afmt[5]) { > - DRM_DEBUG("evergreen_irq_set: hdmi 5\n"); > - afmt6 |= AFMT_AZ_FORMAT_WTRIG_MASK; > - } > > if (rdev->family >= CHIP_CAYMAN) { > cayman_cp_int_cntl_setup(rdev, 0, cp_int_cntl); > @@ -2890,13 +2858,6 @@ int evergreen_irq_set(struct radeon_device *rdev) > WREG32(DC_HPD5_INT_CONTROL, hpd5); > WREG32(DC_HPD6_INT_CONTROL, hpd6); > > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, afmt1); > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, afmt2); > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, afmt3); > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, afmt4); > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, afmt5); > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, afmt6); > - > return 0; > } > > @@ -2921,13 +2882,6 @@ static void evergreen_irq_ack(struct radeon_device *rdev) > rdev->irq.stat_regs.evergreen.d6grph_int = RREG32(GRPH_INT_STATUS + EVERGREEN_CRTC5_REGISTER_OFFSET); > } > > - rdev->irq.stat_regs.evergreen.afmt_status1 = RREG32(AFMT_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET); > - rdev->irq.stat_regs.evergreen.afmt_status2 = RREG32(AFMT_STATUS + EVERGREEN_CRTC1_REGISTER_OFFSET); > - rdev->irq.stat_regs.evergreen.afmt_status3 = RREG32(AFMT_STATUS + EVERGREEN_CRTC2_REGISTER_OFFSET); > - rdev->irq.stat_regs.evergreen.afmt_status4 = RREG32(AFMT_STATUS + EVERGREEN_CRTC3_REGISTER_OFFSET); > - rdev->irq.stat_regs.evergreen.afmt_status5 = RREG32(AFMT_STATUS + EVERGREEN_CRTC4_REGISTER_OFFSET); > - rdev->irq.stat_regs.evergreen.afmt_status6 = RREG32(AFMT_STATUS + EVERGREEN_CRTC5_REGISTER_OFFSET); > - > if (rdev->irq.stat_regs.evergreen.d1grph_int & GRPH_PFLIP_INT_OCCURRED) > WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET, GRPH_PFLIP_INT_CLEAR); > if (rdev->irq.stat_regs.evergreen.d2grph_int & GRPH_PFLIP_INT_OCCURRED) > @@ -3001,36 +2955,6 @@ static void evergreen_irq_ack(struct radeon_device *rdev) > tmp |= DC_HPDx_INT_ACK; > WREG32(DC_HPD6_INT_CONTROL, tmp); > } > - if (rdev->irq.stat_regs.evergreen.afmt_status1 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, tmp); > - } > - if (rdev->irq.stat_regs.evergreen.afmt_status2 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, tmp); > - } > - if (rdev->irq.stat_regs.evergreen.afmt_status3 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, tmp); > - } > - if (rdev->irq.stat_regs.evergreen.afmt_status4 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, tmp); > - } > - if (rdev->irq.stat_regs.evergreen.afmt_status5 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, tmp); > - } > - if (rdev->irq.stat_regs.evergreen.afmt_status6 & AFMT_AZ_FORMAT_WTRIG) { > - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET); > - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; > - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, tmp); > - } > } > > static void evergreen_irq_disable(struct radeon_device *rdev) > @@ -3079,7 +3003,6 @@ int evergreen_irq_process(struct radeon_device *rdev) > u32 src_id, src_data; > u32 ring_index; > bool queue_hotplug = false; > - bool queue_hdmi = false; > > if (!rdev->ih.enabled || rdev->shutdown) > return IRQ_NONE; > @@ -3313,53 +3236,7 @@ restart_ih: > } > break; > case 44: /* hdmi */ > - switch (src_data) { > - case 0: > - if (rdev->irq.stat_regs.evergreen.afmt_status1 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status1 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI0\n"); > - } > - break; > - case 1: > - if (rdev->irq.stat_regs.evergreen.afmt_status2 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status2 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI1\n"); > - } > - break; > - case 2: > - if (rdev->irq.stat_regs.evergreen.afmt_status3 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status3 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI2\n"); > - } > - break; > - case 3: > - if (rdev->irq.stat_regs.evergreen.afmt_status4 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status4 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI3\n"); > - } > - break; > - case 4: > - if (rdev->irq.stat_regs.evergreen.afmt_status5 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status5 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI4\n"); > - } > - break; > - case 5: > - if (rdev->irq.stat_regs.evergreen.afmt_status6 & AFMT_AZ_FORMAT_WTRIG) { > - rdev->irq.stat_regs.evergreen.afmt_status6 &= ~AFMT_AZ_FORMAT_WTRIG; > - queue_hdmi = true; > - DRM_DEBUG("IH: HDMI5\n"); > - } > - break; > - default: > - DRM_ERROR("Unhandled interrupt: %d %d\n", src_id, src_data); > - break; > - } > + DRM_ERROR("Unhandled HDMI interrupt: %d %d\n", src_id, src_data); > break; > case 146: > case 147: > @@ -3418,8 +3295,6 @@ restart_ih: > } > if (queue_hotplug) > schedule_work(&rdev->hotplug_work); > - if (queue_hdmi) > - schedule_work(&rdev->audio_work); > rdev->ih.rptr = rptr; > WREG32(IH_RB_RPTR, rdev->ih.rptr); > atomic_set(&rdev->ih.lock, 0); > -- > 1.7.10.4 >
2013/4/14 Alex Deucher <alexdeucher@gmail.com>: > On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> We need interrupts on format change for R6xx only, where hardware seems >> to be somehow bugged and requires setting audio info manually. > > Can you confirm that this is actually needed on older chips? AFAIK, > it shouldn't be required for any chips. It's mainly for debugging. I can't really right now :( My notebook with RV620 died (hard disk ended it's life and power cable got broken). I hope to resurrect him in about a week. If that isn't needed on R6xx, I'm not sure why we implemented it in first place at all. Christian? Do you have idea why this was required? I remember than in first place we were using timer, then we switched to the interrupts. But why we needed it at all?
On Sun, Apr 14, 2013 at 11:55 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>> We need interrupts on format change for R6xx only, where hardware seems >>> to be somehow bugged and requires setting audio info manually. >> >> Can you confirm that this is actually needed on older chips? AFAIK, >> it shouldn't be required for any chips. It's mainly for debugging. > > I can't really right now :( My notebook with RV620 died (hard disk > ended it's life and power cable got broken). I hope to resurrect him > in about a week. > > If that isn't needed on R6xx, I'm not sure why we implemented it in > first place at all. Christian? Do you have idea why this was required? > I remember than in first place we were using timer, then we switched > to the interrupts. But why we needed it at all? I suspect it was just assumed to be necessary due to the original RE. Alex > > -- > Rafa?
2013/4/14 Alex Deucher <alexdeucher@gmail.com>: > On Sun, Apr 14, 2013 at 11:55 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>> We need interrupts on format change for R6xx only, where hardware seems >>>> to be somehow bugged and requires setting audio info manually. >>> >>> Can you confirm that this is actually needed on older chips? AFAIK, >>> it shouldn't be required for any chips. It's mainly for debugging. >> >> I can't really right now :( My notebook with RV620 died (hard disk >> ended it's life and power cable got broken). I hope to resurrect him >> in about a week. >> >> If that isn't needed on R6xx, I'm not sure why we implemented it in >> first place at all. Christian? Do you have idea why this was required? >> I remember than in first place we were using timer, then we switched >> to the interrupts. But why we needed it at all? > > I suspect it was just assumed to be necessary due to the original RE. I'm OK with removing that from R6xx too, if it's not needed. I just want to check that first, to don't break audio accidentally. In case of Evergreen I was able to test it, so I dares to submit this patch ;)
Am 14.04.2013 20:02, schrieb Rafa? Mi?ecki: > 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >> On Sun, Apr 14, 2013 at 11:55 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>>> We need interrupts on format change for R6xx only, where hardware seems >>>>> to be somehow bugged and requires setting audio info manually. >>>> Can you confirm that this is actually needed on older chips? AFAIK, >>>> it shouldn't be required for any chips. It's mainly for debugging. >>> I can't really right now :( My notebook with RV620 died (hard disk >>> ended it's life and power cable got broken). I hope to resurrect him >>> in about a week. >>> >>> If that isn't needed on R6xx, I'm not sure why we implemented it in >>> first place at all. Christian? Do you have idea why this was required? >>> I remember than in first place we were using timer, then we switched >>> to the interrupts. But why we needed it at all? >> I suspect it was just assumed to be necessary due to the original RE. > I'm OK with removing that from R6xx too, if it's not needed. I just > want to check that first, to don't break audio accidentally. In case > of Evergreen I was able to test it, so I dares to submit this patch ;) Well, originally I was just imitating fglrx behavior with this, but since I now have access to the AMD documentation I can't find a reason why fglrx was actually doing it like this. In theory format changes should work on their own, but it is still possible they did this because of some bug or something like this. I can't really test it anymore either, so no idea if it is really required or not. Christian.
On Mon, Apr 15, 2013 at 4:08 AM, Christian König <deathsimple@vodafone.de> wrote: > Am 14.04.2013 20:02, schrieb Rafa? Mi?ecki: > >> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>> >>> On Sun, Apr 14, 2013 at 11:55 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>> >>>> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>>>> >>>>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> >>>>> wrote: >>>>>> >>>>>> We need interrupts on format change for R6xx only, where hardware >>>>>> seems >>>>>> to be somehow bugged and requires setting audio info manually. >>>>> >>>>> Can you confirm that this is actually needed on older chips? AFAIK, >>>>> it shouldn't be required for any chips. It's mainly for debugging. >>>> >>>> I can't really right now :( My notebook with RV620 died (hard disk >>>> ended it's life and power cable got broken). I hope to resurrect him >>>> in about a week. >>>> >>>> If that isn't needed on R6xx, I'm not sure why we implemented it in >>>> first place at all. Christian? Do you have idea why this was required? >>>> I remember than in first place we were using timer, then we switched >>>> to the interrupts. But why we needed it at all? >>> >>> I suspect it was just assumed to be necessary due to the original RE. >> >> I'm OK with removing that from R6xx too, if it's not needed. I just >> want to check that first, to don't break audio accidentally. In case >> of Evergreen I was able to test it, so I dares to submit this patch ;) > > > Well, originally I was just imitating fglrx behavior with this, but since I > now have access to the AMD documentation I can't find a reason why fglrx was > actually doing it like this. In theory format changes should work on their > own, but it is still possible they did this because of some bug or something > like this. > > I can't really test it anymore either, so no idea if it is really required > or not. For both evergreen and older asics, maybe rather than removing the code, we could just disable the interrupt src. I.e., remove the call to radeon_irq_kms_enable_afmt(). That way we can always re-enable it if we need it for testing or debugging. Alex
2013/4/15 Alex Deucher <alexdeucher@gmail.com>: > On Mon, Apr 15, 2013 at 4:08 AM, Christian König > <deathsimple@vodafone.de> wrote: >> Am 14.04.2013 20:02, schrieb Rafa? Mi?ecki: >> >>> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>>> >>>> On Sun, Apr 14, 2013 at 11:55 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>>> >>>>> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>>>>> >>>>>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> >>>>>> wrote: >>>>>>> >>>>>>> We need interrupts on format change for R6xx only, where hardware >>>>>>> seems >>>>>>> to be somehow bugged and requires setting audio info manually. >>>>>> >>>>>> Can you confirm that this is actually needed on older chips? AFAIK, >>>>>> it shouldn't be required for any chips. It's mainly for debugging. >>>>> >>>>> I can't really right now :( My notebook with RV620 died (hard disk >>>>> ended it's life and power cable got broken). I hope to resurrect him >>>>> in about a week. >>>>> >>>>> If that isn't needed on R6xx, I'm not sure why we implemented it in >>>>> first place at all. Christian? Do you have idea why this was required? >>>>> I remember than in first place we were using timer, then we switched >>>>> to the interrupts. But why we needed it at all? >>>> >>>> I suspect it was just assumed to be necessary due to the original RE. >>> >>> I'm OK with removing that from R6xx too, if it's not needed. I just >>> want to check that first, to don't break audio accidentally. In case >>> of Evergreen I was able to test it, so I dares to submit this patch ;) >> >> >> Well, originally I was just imitating fglrx behavior with this, but since I >> now have access to the AMD documentation I can't find a reason why fglrx was >> actually doing it like this. In theory format changes should work on their >> own, but it is still possible they did this because of some bug or something >> like this. >> >> I can't really test it anymore either, so no idea if it is really required >> or not. > > For both evergreen and older asics, maybe rather than removing the > code, we could just disable the interrupt src. I.e., remove the call > to radeon_irq_kms_enable_afmt(). That way we can always re-enable it > if we need it for testing or debugging. Even removing the code still keeps it in git history :) Just give me some more time, so I can resurrect my old RV620 and make some tests.
2013/4/14 Alex Deucher <alexdeucher@gmail.com>: > On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> We need interrupts on format change for R6xx only, where hardware seems >> to be somehow bugged and requires setting audio info manually. > > Can you confirm that this is actually needed on older chips? AFAIK, > it shouldn't be required for any chips. It's mainly for debugging. I've finally managed to install, run and debug fglrx on RV620. That was really painful and ignoring Xorg 1.13 release in fglrx legacy didn't help :| Please take a look at attached log from fglrx. It seems that fglrx on every audio format change does: 1) Read audio status (R600_AUDIO_STATUS_BITS, R600_AUDIO_RATE_BPS_CHANNEL) 2) Writes sth to HDMI0_STATUS and HDMI0_INFOFRAME_CONTROL0 3) Updates audio info frame 4) Recalculates checksum of audio info frame 5) Triggers update with HDMI0_AUDIO_INFO_UPDATE bit That are some extra reads that look like polling, probably for some frame sync. In the attached logs you can't see any real changes in audio info frame, because I wasn't able to play more that 2 channels (because of lacking support for multichannel audio in alsa). However I start thinking we may need audio interrupts for all ASICs. And removing them wasn't a good idea at al. After all, how we could handle audio infoframe update on audio format change? The thing we may try dropping is code updating HDMI0_60958_0 and HDMI0_60958_1 registers. I don't see fglrx writing to that registers on audio format change, so maybe hardware does handle that automatically? That needs verifying with fglrx and radeon.
2013/4/21 Rafa? Mi?ecki <zajec5@gmail.com>: > 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>> We need interrupts on format change for R6xx only, where hardware seems >>> to be somehow bugged and requires setting audio info manually. >> >> Can you confirm that this is actually needed on older chips? AFAIK, >> it shouldn't be required for any chips. It's mainly for debugging. > > I've finally managed to install, run and debug fglrx on RV620. That > was really painful and ignoring Xorg 1.13 release in fglrx legacy > didn't help :| > > Please take a look at attached log from fglrx. It seems that fglrx on > every audio format change does: > 1) Read audio status (R600_AUDIO_STATUS_BITS, R600_AUDIO_RATE_BPS_CHANNEL) > 2) Writes sth to HDMI0_STATUS and HDMI0_INFOFRAME_CONTROL0 > 3) Updates audio info frame > 4) Recalculates checksum of audio info frame > 5) Triggers update with HDMI0_AUDIO_INFO_UPDATE bit Btw. could we get a meaning of bit 0x100 in HDMI0_STATUS?
On Sun, Apr 21, 2013 at 3:15 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > 2013/4/21 Rafa? Mi?ecki <zajec5@gmail.com>: >> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>> We need interrupts on format change for R6xx only, where hardware seems >>>> to be somehow bugged and requires setting audio info manually. >>> >>> Can you confirm that this is actually needed on older chips? AFAIK, >>> it shouldn't be required for any chips. It's mainly for debugging. >> >> I've finally managed to install, run and debug fglrx on RV620. That >> was really painful and ignoring Xorg 1.13 release in fglrx legacy >> didn't help :| >> >> Please take a look at attached log from fglrx. It seems that fglrx on >> every audio format change does: >> 1) Read audio status (R600_AUDIO_STATUS_BITS, R600_AUDIO_RATE_BPS_CHANNEL) >> 2) Writes sth to HDMI0_STATUS and HDMI0_INFOFRAME_CONTROL0 >> 3) Updates audio info frame >> 4) Recalculates checksum of audio info frame >> 5) Triggers update with HDMI0_AUDIO_INFO_UPDATE bit > > Btw. could we get a meaning of bit 0x100 in HDMI0_STATUS? There is no bit 8 defined in that register. Also FWIW, HDMI0_STATUS is a read only register. Alex > > -- > Rafa?
2013/4/21 Alex Deucher <alexdeucher@gmail.com>: > On Sun, Apr 21, 2013 at 3:15 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >> 2013/4/21 Rafa? Mi?ecki <zajec5@gmail.com>: >>> 2013/4/14 Alex Deucher <alexdeucher@gmail.com>: >>>> On Sat, Apr 13, 2013 at 7:26 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote: >>>>> We need interrupts on format change for R6xx only, where hardware seems >>>>> to be somehow bugged and requires setting audio info manually. >>>> >>>> Can you confirm that this is actually needed on older chips? AFAIK, >>>> it shouldn't be required for any chips. It's mainly for debugging. >>> >>> I've finally managed to install, run and debug fglrx on RV620. That >>> was really painful and ignoring Xorg 1.13 release in fglrx legacy >>> didn't help :| >>> >>> Please take a look at attached log from fglrx. It seems that fglrx on >>> every audio format change does: >>> 1) Read audio status (R600_AUDIO_STATUS_BITS, R600_AUDIO_RATE_BPS_CHANNEL) >>> 2) Writes sth to HDMI0_STATUS and HDMI0_INFOFRAME_CONTROL0 >>> 3) Updates audio info frame >>> 4) Recalculates checksum of audio info frame >>> 5) Triggers update with HDMI0_AUDIO_INFO_UPDATE bit >> >> Btw. could we get a meaning of bit 0x100 in HDMI0_STATUS? > > There is no bit 8 defined in that register. Also FWIW, HDMI0_STATUS > is a read only register. Ahh, sorry. 0x7400 is HDMI0_CONTROL not HDMI0_STATUS! My silly mistake. It makes sense of course: # define HDMI0_ERROR_ACK (1 << 8) Still I can't see how we could drop interrupts and keep updating audio infoframe.
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..34d4347 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2702,7 +2702,6 @@ int evergreen_irq_set(struct radeon_device *rdev) 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; if (!rdev->irq.installed) { @@ -2724,13 +2723,6 @@ int evergreen_irq_set(struct radeon_device *rdev) hpd5 = RREG32(DC_HPD5_INT_CONTROL) & ~DC_HPDx_INT_EN; hpd6 = RREG32(DC_HPD6_INT_CONTROL) & ~DC_HPDx_INT_EN; - afmt1 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - afmt2 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - afmt3 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - afmt4 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - afmt5 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - afmt6 = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET) & ~AFMT_AZ_FORMAT_WTRIG_MASK; - dma_cntl = RREG32(DMA_CNTL) & ~TRAP_ENABLE; if (rdev->family >= CHIP_CAYMAN) { @@ -2822,30 +2814,6 @@ int evergreen_irq_set(struct radeon_device *rdev) DRM_DEBUG("evergreen_irq_set: hpd 6\n"); hpd6 |= DC_HPDx_INT_EN; } - if (rdev->irq.afmt[0]) { - DRM_DEBUG("evergreen_irq_set: hdmi 0\n"); - afmt1 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } - if (rdev->irq.afmt[1]) { - DRM_DEBUG("evergreen_irq_set: hdmi 1\n"); - afmt2 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } - if (rdev->irq.afmt[2]) { - DRM_DEBUG("evergreen_irq_set: hdmi 2\n"); - afmt3 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } - if (rdev->irq.afmt[3]) { - DRM_DEBUG("evergreen_irq_set: hdmi 3\n"); - afmt4 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } - if (rdev->irq.afmt[4]) { - DRM_DEBUG("evergreen_irq_set: hdmi 4\n"); - afmt5 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } - if (rdev->irq.afmt[5]) { - DRM_DEBUG("evergreen_irq_set: hdmi 5\n"); - afmt6 |= AFMT_AZ_FORMAT_WTRIG_MASK; - } if (rdev->family >= CHIP_CAYMAN) { cayman_cp_int_cntl_setup(rdev, 0, cp_int_cntl); @@ -2890,13 +2858,6 @@ int evergreen_irq_set(struct radeon_device *rdev) WREG32(DC_HPD5_INT_CONTROL, hpd5); WREG32(DC_HPD6_INT_CONTROL, hpd6); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, afmt1); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, afmt2); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, afmt3); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, afmt4); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, afmt5); - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, afmt6); - return 0; } @@ -2921,13 +2882,6 @@ static void evergreen_irq_ack(struct radeon_device *rdev) rdev->irq.stat_regs.evergreen.d6grph_int = RREG32(GRPH_INT_STATUS + EVERGREEN_CRTC5_REGISTER_OFFSET); } - rdev->irq.stat_regs.evergreen.afmt_status1 = RREG32(AFMT_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET); - rdev->irq.stat_regs.evergreen.afmt_status2 = RREG32(AFMT_STATUS + EVERGREEN_CRTC1_REGISTER_OFFSET); - rdev->irq.stat_regs.evergreen.afmt_status3 = RREG32(AFMT_STATUS + EVERGREEN_CRTC2_REGISTER_OFFSET); - rdev->irq.stat_regs.evergreen.afmt_status4 = RREG32(AFMT_STATUS + EVERGREEN_CRTC3_REGISTER_OFFSET); - rdev->irq.stat_regs.evergreen.afmt_status5 = RREG32(AFMT_STATUS + EVERGREEN_CRTC4_REGISTER_OFFSET); - rdev->irq.stat_regs.evergreen.afmt_status6 = RREG32(AFMT_STATUS + EVERGREEN_CRTC5_REGISTER_OFFSET); - if (rdev->irq.stat_regs.evergreen.d1grph_int & GRPH_PFLIP_INT_OCCURRED) WREG32(GRPH_INT_STATUS + EVERGREEN_CRTC0_REGISTER_OFFSET, GRPH_PFLIP_INT_CLEAR); if (rdev->irq.stat_regs.evergreen.d2grph_int & GRPH_PFLIP_INT_OCCURRED) @@ -3001,36 +2955,6 @@ static void evergreen_irq_ack(struct radeon_device *rdev) tmp |= DC_HPDx_INT_ACK; WREG32(DC_HPD6_INT_CONTROL, tmp); } - if (rdev->irq.stat_regs.evergreen.afmt_status1 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, tmp); - } - if (rdev->irq.stat_regs.evergreen.afmt_status2 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, tmp); - } - if (rdev->irq.stat_regs.evergreen.afmt_status3 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, tmp); - } - if (rdev->irq.stat_regs.evergreen.afmt_status4 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, tmp); - } - if (rdev->irq.stat_regs.evergreen.afmt_status5 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, tmp); - } - if (rdev->irq.stat_regs.evergreen.afmt_status6 & AFMT_AZ_FORMAT_WTRIG) { - tmp = RREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET); - tmp |= AFMT_AZ_FORMAT_WTRIG_ACK; - WREG32(AFMT_AUDIO_PACKET_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, tmp); - } } static void evergreen_irq_disable(struct radeon_device *rdev) @@ -3079,7 +3003,6 @@ int evergreen_irq_process(struct radeon_device *rdev) u32 src_id, src_data; u32 ring_index; bool queue_hotplug = false; - bool queue_hdmi = false; if (!rdev->ih.enabled || rdev->shutdown) return IRQ_NONE; @@ -3313,53 +3236,7 @@ restart_ih: } break; case 44: /* hdmi */ - switch (src_data) { - case 0: - if (rdev->irq.stat_regs.evergreen.afmt_status1 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status1 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI0\n"); - } - break; - case 1: - if (rdev->irq.stat_regs.evergreen.afmt_status2 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status2 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI1\n"); - } - break; - case 2: - if (rdev->irq.stat_regs.evergreen.afmt_status3 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status3 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI2\n"); - } - break; - case 3: - if (rdev->irq.stat_regs.evergreen.afmt_status4 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status4 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI3\n"); - } - break; - case 4: - if (rdev->irq.stat_regs.evergreen.afmt_status5 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status5 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI4\n"); - } - break; - case 5: - if (rdev->irq.stat_regs.evergreen.afmt_status6 & AFMT_AZ_FORMAT_WTRIG) { - rdev->irq.stat_regs.evergreen.afmt_status6 &= ~AFMT_AZ_FORMAT_WTRIG; - queue_hdmi = true; - DRM_DEBUG("IH: HDMI5\n"); - } - break; - default: - DRM_ERROR("Unhandled interrupt: %d %d\n", src_id, src_data); - break; - } + DRM_ERROR("Unhandled HDMI interrupt: %d %d\n", src_id, src_data); break; case 146: case 147: @@ -3418,8 +3295,6 @@ restart_ih: } if (queue_hotplug) schedule_work(&rdev->hotplug_work); - if (queue_hdmi) - schedule_work(&rdev->audio_work); rdev->ih.rptr = rptr; WREG32(IH_RB_RPTR, rdev->ih.rptr); atomic_set(&rdev->ih.lock, 0);
We need interrupts on format change for R6xx only, where hardware seems to be somehow bugged and requires setting audio info manually. Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com> --- drivers/gpu/drm/radeon/evergreen.c | 127 +----------------------------------- 1 file changed, 1 insertion(+), 126 deletions(-)