[PATCH] drm/radeon: cleanup and fix crtc while programming mc

Jerome Glisse j.glisse at gmail.com
Thu Jul 26 19:04:05 PDT 2012


On Thu, Jul 26, 2012 at 8:35 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Thu, Jul 26, 2012 at 7:24 PM,  <j.glisse at gmail.com> wrote:
>> From: Jerome Glisse <jglisse at redhat.com>
>>
>> When we change start address of vram for the GPU memory controller
>> we need to make sure that nothing in the GPU still use the old vram
>> address. This patch cleanup and fix crtc address.
>>
>> However there is still someissue somewhere if we reenable the crtc
>> after updating the address at which they sancout. So to avoid any
>> issue disable crtc. Once we want to do flicker less transition
>> btw uefi and radeon we will probably want to revisit how we program
>> the GPU memory controller.
>>
>> This probably fix :
>> https://bugs.freedesktop.org/show_bug.cgi?id=52467
>> https://bugs.freedesktop.org/show_bug.cgi?id=42373
>>
>> Cc: <stable at vger.kernel.org>
>
> this should be stable at vger.kernel.org
>
>> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
>
> Also, I think we probably need to poll on bit 16
> (CRTC_CURRENT_MASTER_EN_STATE) of CRTC_CONTROL to make sure the crtc
> is actually disabled.  Something like the following in mc_stop():
>
> WREG32(CRTC_CONTROL, 0);
> while (RREG32(CRTC_CONTROL & CRTC_CURRENT_MASTER_EN_STATE))
>     udelay(1);
>
> Any chance you want to fix the rv515_mc_stop() and rv515_mc_resume()
> functions as well?
>
> Alex

I will respin with splitting out what fix and what just
improve/cleanup the code. I might change rv515 but i don't think it
will hit such issue, on rv515 crtc is enabled through VGA, here the
issue seems to only show up with uefi, i am trying to grab some non
uefi laptop with same gpu to check.

Cheers,
Jerome

>
>> ---
>>  drivers/gpu/drm/radeon/evergreen.c   |  178 ++++++++++++++++------------------
>>  drivers/gpu/drm/radeon/radeon_asic.h |   18 +++-
>>  2 files changed, 99 insertions(+), 97 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
>> index e585a3b..c6ede66 100644
>> --- a/drivers/gpu/drm/radeon/evergreen.c
>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>> @@ -1227,70 +1227,94 @@ void evergreen_agp_enable(struct radeon_device *rdev)
>>         WREG32(VM_CONTEXT1_CNTL, 0);
>>  }
>>
>> +static void evergreen_crtc_save(struct radeon_device *rdev, struct evergreen_mc_save *save, unsigned idx)
>> +{
>> +       save->crtc[idx].paddress = 0;
>> +       save->crtc[idx].saddress = 0;
>> +       save->crtc[idx].crtc_control = 0;
>> +
>> +       if (idx >= rdev->num_crtc) {
>> +               /* it seems accessing non existant crtc trigger high latency */
>> +               return;
>> +       }
>> +
>> +       save->crtc[idx].paddress = RREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + save->crtc[idx].offset);
>> +       save->crtc[idx].paddress |= ((uint64_t)RREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + save->crtc[idx].offset)) << 32ULL;
>> +       save->crtc[idx].saddress = RREG32(EVERGREEN_GRPH_SECONDARY_SURFACE_ADDRESS + save->crtc[idx].offset);
>> +       save->crtc[idx].saddress |= ((uint64_t)RREG32(EVERGREEN_GRPH_SECONDARY_SURFACE_ADDRESS_HIGH + save->crtc[idx].offset)) << 32ULL;
>> +       save->crtc[idx].crtc_control = RREG32(EVERGREEN_CRTC_CONTROL + save->crtc[idx].offset);
>> +       /* We need to disable all crtc as for some reason crtc scanout
>> +        * base address does not happen properly when changing the
>> +        * mc vram start address. Don't remove this or you will break
>> +        * things on uefi system.
>> +        */
>> +       save->crtc[idx].crtc_control = 0;
>> +       save->crtc[idx].vga_control = RREG32(save->crtc[idx].vga_control_offset);
>> +
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 1);
>> +       WREG32(EVERGREEN_CRTC_CONTROL + save->crtc[idx].offset, 0);
>> +       WREG32(save->crtc[idx].vga_control_offset, 0);
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 0);
>> +}
>> +
>>  void evergreen_mc_stop(struct radeon_device *rdev, struct evergreen_mc_save *save)
>>  {
>> -       save->vga_control[0] = RREG32(D1VGA_CONTROL);
>> -       save->vga_control[1] = RREG32(D2VGA_CONTROL);
>> +       save->crtc[0].offset = EVERGREEN_CRTC0_REGISTER_OFFSET;
>> +       save->crtc[1].offset = EVERGREEN_CRTC1_REGISTER_OFFSET;
>> +       save->crtc[2].offset = EVERGREEN_CRTC2_REGISTER_OFFSET;
>> +       save->crtc[3].offset = EVERGREEN_CRTC3_REGISTER_OFFSET;
>> +       save->crtc[4].offset = EVERGREEN_CRTC4_REGISTER_OFFSET;
>> +       save->crtc[5].offset = EVERGREEN_CRTC5_REGISTER_OFFSET;
>> +       save->crtc[0].vga_control_offset = D1VGA_CONTROL;
>> +       save->crtc[1].vga_control_offset = D2VGA_CONTROL;
>> +       save->crtc[2].vga_control_offset = EVERGREEN_D3VGA_CONTROL;
>> +       save->crtc[3].vga_control_offset = EVERGREEN_D4VGA_CONTROL;
>> +       save->crtc[4].vga_control_offset = EVERGREEN_D5VGA_CONTROL;
>> +       save->crtc[5].vga_control_offset = EVERGREEN_D6VGA_CONTROL;
>> +
>> +       save->fb_address = (uint64_t)(RREG32(MC_VM_FB_LOCATION) & 0xffff) << 24ULL;
>>         save->vga_render_control = RREG32(VGA_RENDER_CONTROL);
>>         save->vga_hdp_control = RREG32(VGA_HDP_CONTROL);
>> -       save->crtc_control[0] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET);
>> -       save->crtc_control[1] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET);
>> -       if (rdev->num_crtc >= 4) {
>> -               save->vga_control[2] = RREG32(EVERGREEN_D3VGA_CONTROL);
>> -               save->vga_control[3] = RREG32(EVERGREEN_D4VGA_CONTROL);
>> -               save->crtc_control[2] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET);
>> -               save->crtc_control[3] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               save->vga_control[4] = RREG32(EVERGREEN_D5VGA_CONTROL);
>> -               save->vga_control[5] = RREG32(EVERGREEN_D6VGA_CONTROL);
>> -               save->crtc_control[4] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET);
>> -               save->crtc_control[5] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET);
>> -       }
>>
>>         /* Stop all video */
>>         WREG32(VGA_RENDER_CONTROL, 0);
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 1);
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 1);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 1);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 1);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 1);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 1);
>> -       }
>> -       WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, 0);
>> -       WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, 0);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, 0);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
>> -       }
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 0);
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 0);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 0);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
>> +       evergreen_crtc_save(rdev, save, 0);
>> +       evergreen_crtc_save(rdev, save, 1);
>> +       evergreen_crtc_save(rdev, save, 2);
>> +       evergreen_crtc_save(rdev, save, 3);
>> +       evergreen_crtc_save(rdev, save, 4);
>> +       evergreen_crtc_save(rdev, save, 5);
>> +}
>> +
>> +static void evergreen_crtc_restore(struct radeon_device *rdev, struct evergreen_mc_save *save, unsigned idx)
>> +{
>> +       if (idx >= rdev->num_crtc) {
>> +               /* it seems accessing non existant crtc trigger high latency */
>> +               return;
>>         }
>>
>> -       WREG32(D1VGA_CONTROL, 0);
>> -       WREG32(D2VGA_CONTROL, 0);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_D3VGA_CONTROL, 0);
>> -               WREG32(EVERGREEN_D4VGA_CONTROL, 0);
>> +       /* update new primary and secondary address */
>> +       if (save->crtc[idx].paddress) {
>> +               save->crtc[idx].paddress -= save->fb_address;
>>         }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_D5VGA_CONTROL, 0);
>> -               WREG32(EVERGREEN_D6VGA_CONTROL, 0);
>> +       save->crtc[idx].paddress += rdev->mc.vram_start;
>> +       if (save->crtc[idx].saddress) {
>> +               save->crtc[idx].saddress -= save->fb_address;
>>         }
>> +       save->crtc[idx].saddress += rdev->mc.vram_start;
>> +
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 1);
>> +       WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + save->crtc[idx].offset, lower_32_bits(save->crtc[idx].paddress));
>> +       WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + save->crtc[idx].offset, upper_32_bits(save->crtc[idx].paddress) & 0xff);
>> +       WREG32(EVERGREEN_GRPH_SECONDARY_SURFACE_ADDRESS + save->crtc[idx].offset, lower_32_bits(save->crtc[idx].saddress));
>> +       WREG32(EVERGREEN_GRPH_SECONDARY_SURFACE_ADDRESS_HIGH + save->crtc[idx].offset, upper_32_bits(save->crtc[idx].saddress) & 0xff);
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 0);
>> +       WREG32(EVERGREEN_MASTER_UPDATE_MODE + save->crtc[idx].offset, 0);
>> +
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 1);
>> +       WREG32(save->crtc[idx].vga_control_offset, save->crtc[idx].vga_control);
>> +       WREG32(EVERGREEN_CRTC_CONTROL + save->crtc[idx].offset, save->crtc[idx].crtc_control | EVERGREEN_CRTC_DISP_READ_REQUEST_DISABLE);
>> +       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + save->crtc[idx].offset, 0);
>>  }
>>
>>  void evergreen_mc_resume(struct radeon_device *rdev, struct evergreen_mc_save *save)
>> @@ -1357,47 +1381,15 @@ void evergreen_mc_resume(struct radeon_device *rdev, struct evergreen_mc_save *s
>>         /* Unlock host access */
>>         WREG32(VGA_HDP_CONTROL, save->vga_hdp_control);
>>         mdelay(1);
>> +
>>         /* Restore video state */
>> -       WREG32(D1VGA_CONTROL, save->vga_control[0]);
>> -       WREG32(D2VGA_CONTROL, save->vga_control[1]);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_D3VGA_CONTROL, save->vga_control[2]);
>> -               WREG32(EVERGREEN_D4VGA_CONTROL, save->vga_control[3]);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_D5VGA_CONTROL, save->vga_control[4]);
>> -               WREG32(EVERGREEN_D6VGA_CONTROL, save->vga_control[5]);
>> -       }
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 1);
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 1);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 1);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 1);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 1);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 1);
>> -       }
>> -       WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, save->crtc_control[0]);
>> -       WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, save->crtc_control[1]);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, save->crtc_control[2]);
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, save->crtc_control[3]);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, save->crtc_control[4]);
>> -               WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, save->crtc_control[5]);
>> -       }
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 0);
>> -       WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 0);
>> -       if (rdev->num_crtc >= 4) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 0);
>> -       }
>> -       if (rdev->num_crtc >= 6) {
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
>> -               WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
>> -       }
>> +       evergreen_crtc_restore(rdev, save, 0);
>> +       evergreen_crtc_restore(rdev, save, 1);
>> +       evergreen_crtc_restore(rdev, save, 2);
>> +       evergreen_crtc_restore(rdev, save, 3);
>> +       evergreen_crtc_restore(rdev, save, 4);
>> +       evergreen_crtc_restore(rdev, save, 5);
>> +
>>         WREG32(VGA_RENDER_CONTROL, save->vga_render_control);
>>  }
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
>> index f4af243..d450218 100644
>> --- a/drivers/gpu/drm/radeon/radeon_asic.h
>> +++ b/drivers/gpu/drm/radeon/radeon_asic.h
>> @@ -388,12 +388,22 @@ void r700_cp_fini(struct radeon_device *rdev);
>>  /*
>>   * evergreen
>>   */
>> +struct evergreen_crtc_save {
>> +       uint64_t                        paddress;
>> +       uint64_t                        saddress;
>> +       uint32_t                        crtc_control;
>> +       uint32_t                        vga_control;
>> +       uint32_t                        offset;
>> +       uint32_t                        vga_control_offset;
>> +};
>> +
>>  struct evergreen_mc_save {
>> -       u32 vga_control[6];
>> -       u32 vga_render_control;
>> -       u32 vga_hdp_control;
>> -       u32 crtc_control[6];
>> +       struct evergreen_crtc_save      crtc[RADEON_MAX_CRTCS];
>> +       uint64_t                        fb_address;
>> +       uint32_t                        vga_render_control;
>> +       uint32_t                        vga_hdp_control;
>>  };
>> +
>>  void evergreen_pcie_gart_tlb_flush(struct radeon_device *rdev);
>>  int evergreen_init(struct radeon_device *rdev);
>>  void evergreen_fini(struct radeon_device *rdev);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list