On Wed, Dec 14, 2022 at 4:54 PM Robin Murphy <robin.murphy@xxxxxxx> wrote:
>
> On 2022-12-12 02:08, Luben Tuikov wrote:
> > Fix screen corruption on older 32-bit systems using AGP chips.
> >
> > On older systems with little memory, for instance 1.5 GiB, using an AGP chip,
> > the device's DMA mask is 0xFFFFFFFF, but the memory mask is 0x7FFFFFF, and
> > subsequently dma_addressing_limited() returns 0xFFFFFFFF < 0x7FFFFFFF,
> > false. As such the result of this static inline isn't suitable for the last
> > argument to ttm_device_init()--it simply needs to now whether to use GFP_DMA32
> > when allocating DMA buffers.
>
> This sounds wrong to me. If the issues happen on systems without PAE it
> clearly can't have anything to with the actual DMA address size. Not to
> mention that AFAICS 32-bit x86 doesn't even have ZONE_DMA32, so
> GFP_DMA32 would be functionally meaningless anyway. Although the
> reported symptoms initially sounded like they could be caused by DMA
> going to the wrong place, that is also equally consistent with a loss of
> cache coherency.
>
> My (limited) understanding of AGP is that the GART can effectively alias
> memory to a second physical address, so I could well believe that
> something somewhere in the driver stack needs to perform some cache
> maintenance to avoid coherency issues, and that in these particular
> setups whatever that is might be assuming the memory is direct-mapped
> and thus going wrong for highmem pages.
>
> So as I said before, I really think this is not about using GFP_DMA32 at
> all, but about *not* using GFP_HIGHUSER.
One of the wonderful features of AGP is that it has to be used with
uncached memory. The aperture basically just provides a remapping of
physical pages into a linear aperture that you point the GPU at. TTM
has to jump through quite a few hoops to get uncached memory in the
first place, so it's likely that that somehow isn't compatible with
HIGHMEM. Can you get uncached HIGHMEM?
Alex
>
> Thanks,
> Robin.
>
> > Partially reverts commit 33b3ad3788aba846fc8b9a065fe2685a0b64f713.
> >
> > v2: Amend the commit description.
> >
> > Cc: Mikhail Krylov <sqarert@xxxxxxxxx>
> > Cc: Alex Deucher <Alexander.Deucher@xxxxxxx>
> > Cc: Robin Murphy <robin.murphy@xxxxxxx>
> > Cc: Direct Rendering Infrastructure - Development <dri-devel@xxxxxxxxxxxxxxxxxxxxx>
> > Cc: AMD Graphics <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
> > Fixes: 33b3ad3788aba8 ("drm/radeon: handle PCIe root ports with addressing limitations")
> > Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>
> > ---
> > drivers/gpu/drm/radeon/radeon.h | 1 +
> > drivers/gpu/drm/radeon/radeon_device.c | 2 +-
> > drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> > 3 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> > index 37dec92339b16a..4fe38fd9be3267 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -2426,6 +2426,7 @@ struct radeon_device {
> > struct radeon_wb wb;
> > struct radeon_dummy_page dummy_page;
> > bool shutdown;
> > + bool need_dma32;
> > bool need_swiotlb;
> > bool accel_working;
> > bool fastfb_working; /* IGP feature*/
> > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> > index 6344454a772172..3643a3cfe061bd 100644
> > --- a/drivers/gpu/drm/radeon/radeon_device.c
> > +++ b/drivers/gpu/drm/radeon/radeon_device.c
> > @@ -1370,7 +1370,7 @@ int radeon_device_init(struct radeon_device *rdev,
> > if (rdev->family == CHIP_CEDAR)
> > dma_bits = 32;
> > #endif
> > -
> > + rdev->need_dma32 = dma_bits == 32;
> > r = dma_set_mask_and_coherent(&rdev->pdev->dev, DMA_BIT_MASK(dma_bits));
> > if (r) {
> > pr_warn("radeon: No suitable DMA available\n");
> > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> > index bdb4c0e0736ba2..3debaeb720d173 100644
> > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > @@ -696,7 +696,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
> > rdev->ddev->anon_inode->i_mapping,
> > rdev->ddev->vma_offset_manager,
> > rdev->need_swiotlb,
> > - dma_addressing_limited(&rdev->pdev->dev));
> > + rdev->need_dma32);
> > if (r) {
> > DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
> > return r;
> >
> > base-commit: 20e03e7f6e8efd42168db6d3fe044b804e0ede8f
