| Message ID | 20220426164108.1051295-5-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mgag200: Improve damage handling | expand |
Hi Am 26.04.22 um 18:41 schrieb Jocelyn Falempe: > This function writes some pattern to video memory, to check for conflict. > In case of conflicts, it returns a random memory capacity (offset of the > conflict). > Using devm_arch_io_reserve_memtype_wc() should garantee that no other > driver is using this memory region. > In case of real memory conflicts, as it is video memory, the user will > notice it easily. So there is no need for this function. I don't think we can remove this function. It doesn't test for concurrent access, but for non-existing video memory. Apparently, some systems have larger PCI apertures than actual video memory. Best regards Thomas > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/mgag200/mgag200_mm.c | 50 ++++------------------------ > 1 file changed, 7 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c > index fa996d46feed..68299b560a98 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mm.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c > @@ -32,48 +32,6 @@ > > #include "mgag200_drv.h" > > -static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, > - size_t size) > -{ > - int offset; > - int orig; > - int test1, test2; > - int orig1, orig2; > - size_t vram_size; > - > - /* Probe */ > - orig = ioread16(mem); > - iowrite16(0, mem); > - > - vram_size = size; > - > - if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) > - vram_size = vram_size - 0x400000; > - > - for (offset = 0x100000; offset < vram_size; offset += 0x4000) { > - orig1 = ioread8(mem + offset); > - orig2 = ioread8(mem + offset + 0x100); > - > - iowrite16(0xaa55, mem + offset); > - iowrite16(0xaa55, mem + offset + 0x100); > - > - test1 = ioread16(mem + offset); > - test2 = ioread16(mem); > - > - iowrite16(orig1, mem + offset); > - iowrite16(orig2, mem + offset + 0x100); > - > - if (test1 != 0xaa55) > - break; > - > - if (test2) > - break; > - } > - > - iowrite16(orig, mem); > - > - return offset - 65536; > -} > > int mgag200_mm_init(struct mga_device *mdev) > { > @@ -81,6 +39,7 @@ int mgag200_mm_init(struct mga_device *mdev) > struct pci_dev *pdev = to_pci_dev(dev->dev); > u8 misc; > resource_size_t start, len; > + size_t vram_size; > > WREG_ECRT(0x04, 0x00); > > @@ -106,7 +65,12 @@ int mgag200_mm_init(struct mga_device *mdev) > if (!mdev->vram) > return -ENOMEM; > > - mdev->mc.vram_size = mgag200_probe_vram(mdev, mdev->vram, len); > + vram_size = len; > + /* G200_EW3 has only 12MB of memory */ > + if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) > + vram_size -= 0x400000; > + > + mdev->mc.vram_size = vram_size; > mdev->mc.vram_base = start; > mdev->mc.vram_window = len; >
On 04/05/2022 12:16, Thomas Zimmermann wrote: > Hi > > Am 26.04.22 um 18:41 schrieb Jocelyn Falempe: >> This function writes some pattern to video memory, to check for conflict. >> In case of conflicts, it returns a random memory capacity (offset of the >> conflict). >> Using devm_arch_io_reserve_memtype_wc() should garantee that no other >> driver is using this memory region. >> In case of real memory conflicts, as it is video memory, the user will >> notice it easily. So there is no need for this function. > > I don't think we can remove this function. It doesn't test for > concurrent access, but for non-existing video memory. Apparently, some > systems have larger PCI apertures than actual video memory. Ok, I tough writing to non-existing memory would raise an exception/Bus error. If it's silently ignored by hardware, this is probably the only way to make sure the memory is there. let's drop this patch in v2 ;) > > Best regards > Thomas > >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/mgag200/mgag200_mm.c | 50 ++++------------------------ >> 1 file changed, 7 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c >> b/drivers/gpu/drm/mgag200/mgag200_mm.c >> index fa996d46feed..68299b560a98 100644 >> --- a/drivers/gpu/drm/mgag200/mgag200_mm.c >> +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c >> @@ -32,48 +32,6 @@ >> #include "mgag200_drv.h" >> -static size_t mgag200_probe_vram(struct mga_device *mdev, void >> __iomem *mem, >> - size_t size) >> -{ >> - int offset; >> - int orig; >> - int test1, test2; >> - int orig1, orig2; >> - size_t vram_size; >> - >> - /* Probe */ >> - orig = ioread16(mem); >> - iowrite16(0, mem); >> - >> - vram_size = size; >> - >> - if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) >> - vram_size = vram_size - 0x400000; >> - >> - for (offset = 0x100000; offset < vram_size; offset += 0x4000) { >> - orig1 = ioread8(mem + offset); >> - orig2 = ioread8(mem + offset + 0x100); >> - >> - iowrite16(0xaa55, mem + offset); >> - iowrite16(0xaa55, mem + offset + 0x100); >> - >> - test1 = ioread16(mem + offset); >> - test2 = ioread16(mem); >> - >> - iowrite16(orig1, mem + offset); >> - iowrite16(orig2, mem + offset + 0x100); >> - >> - if (test1 != 0xaa55) >> - break; >> - >> - if (test2) >> - break; >> - } >> - >> - iowrite16(orig, mem); >> - >> - return offset - 65536; >> -} >> int mgag200_mm_init(struct mga_device *mdev) >> { >> @@ -81,6 +39,7 @@ int mgag200_mm_init(struct mga_device *mdev) >> struct pci_dev *pdev = to_pci_dev(dev->dev); >> u8 misc; >> resource_size_t start, len; >> + size_t vram_size; >> WREG_ECRT(0x04, 0x00); >> @@ -106,7 +65,12 @@ int mgag200_mm_init(struct mga_device *mdev) >> if (!mdev->vram) >> return -ENOMEM; >> - mdev->mc.vram_size = mgag200_probe_vram(mdev, mdev->vram, len); >> + vram_size = len; >> + /* G200_EW3 has only 12MB of memory */ >> + if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) >> + vram_size -= 0x400000; >> + >> + mdev->mc.vram_size = vram_size; >> mdev->mc.vram_base = start; >> mdev->mc.vram_window = len; >
diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index fa996d46feed..68299b560a98 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -32,48 +32,6 @@ #include "mgag200_drv.h" -static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem, - size_t size) -{ - int offset; - int orig; - int test1, test2; - int orig1, orig2; - size_t vram_size; - - /* Probe */ - orig = ioread16(mem); - iowrite16(0, mem); - - vram_size = size; - - if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) - vram_size = vram_size - 0x400000; - - for (offset = 0x100000; offset < vram_size; offset += 0x4000) { - orig1 = ioread8(mem + offset); - orig2 = ioread8(mem + offset + 0x100); - - iowrite16(0xaa55, mem + offset); - iowrite16(0xaa55, mem + offset + 0x100); - - test1 = ioread16(mem + offset); - test2 = ioread16(mem); - - iowrite16(orig1, mem + offset); - iowrite16(orig2, mem + offset + 0x100); - - if (test1 != 0xaa55) - break; - - if (test2) - break; - } - - iowrite16(orig, mem); - - return offset - 65536; -} int mgag200_mm_init(struct mga_device *mdev) { @@ -81,6 +39,7 @@ int mgag200_mm_init(struct mga_device *mdev) struct pci_dev *pdev = to_pci_dev(dev->dev); u8 misc; resource_size_t start, len; + size_t vram_size; WREG_ECRT(0x04, 0x00); @@ -106,7 +65,12 @@ int mgag200_mm_init(struct mga_device *mdev) if (!mdev->vram) return -ENOMEM; - mdev->mc.vram_size = mgag200_probe_vram(mdev, mdev->vram, len); + vram_size = len; + /* G200_EW3 has only 12MB of memory */ + if ((mdev->type == G200_EW3) && (vram_size >= 0x1000000)) + vram_size -= 0x400000; + + mdev->mc.vram_size = vram_size; mdev->mc.vram_base = start; mdev->mc.vram_window = len;
This function writes some pattern to video memory, to check for conflict. In case of conflicts, it returns a random memory capacity (offset of the conflict). Using devm_arch_io_reserve_memtype_wc() should garantee that no other driver is using this memory region. In case of real memory conflicts, as it is video memory, the user will notice it easily. So there is no need for this function. Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/mgag200/mgag200_mm.c | 50 ++++------------------------ 1 file changed, 7 insertions(+), 43 deletions(-)