Hi
Am 05.06.20 um 16:39 schrieb Sam Ravnborg:
> Hi Thomas.
>
> Some parts I did not understand here.
>
> On Fri, Jun 05, 2020 at 03:57:55PM +0200, Thomas Zimmermann wrote:
>> The VRAM setup in mgag200_drv.c is part of memory management and
>> should be done in the same place. Merge the code into the memory
>> management's init function.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> ---
>> drivers/gpu/drm/mgag200/mgag200_main.c | 75 --------------------------
>> drivers/gpu/drm/mgag200/mgag200_mm.c | 52 ++++++++++++++++++
>> 2 files changed, 52 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
>> index 3298eff7bd1b4..e9ad783c2b44d 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
>> @@ -12,77 +12,6 @@
>>
>> #include "mgag200_drv.h"
>>
>> -static int mga_probe_vram(struct mga_device *mdev, void __iomem *mem)
>> -{
>> - int offset;
>> - int orig;
>> - int test1, test2;
>> - int orig1, orig2;
>> - unsigned int vram_size;
>> -
>> - /* Probe */
>> - orig = ioread16(mem);
>> - iowrite16(0, mem);
>> -
>> - vram_size = mdev->mc.vram_window;
>> -
>> - 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;
>> -}
>> -
>> -/* Map the framebuffer from the card and configure the core */
>> -static int mga_vram_init(struct mga_device *mdev)
>> -{
>> - struct drm_device *dev = mdev->dev;
>> - void __iomem *mem;
>> -
>> - /* BAR 0 is VRAM */
>> - mdev->mc.vram_base = pci_resource_start(dev->pdev, 0);
>> - mdev->mc.vram_window = pci_resource_len(dev->pdev, 0);
>> -
>> - if (!devm_request_mem_region(dev->dev, mdev->mc.vram_base,
>> - mdev->mc.vram_window, "mgadrmfb_vram")) {
>> - DRM_ERROR("can't reserve VRAM\n");
>> - return -ENXIO;
>> - }
>> -
>> - mem = pci_iomap(dev->pdev, 0, 0);
>> - if (!mem)
>> - return -ENOMEM;
>> -
>> - mdev->mc.vram_size = mga_probe_vram(mdev, mem);
>> -
>> - pci_iounmap(dev->pdev, mem);
> mem is the result of pci_iomap() - but I do not see any call
> to pci_iomap() in the converted code.
>
> mdev->vram is used as argument in new code where mem was used in the old
> code.
> mdev->vram comes from ioremap(start, len) - will it result in the same?
Yes. pci_iomap() maps the whole PCI BAR (i.e., 0 in this case) memory.
The driver code reads the bar's start and length, which also covers the
full PCI BAR range. So in the end the probe function runs on the same
range of VRAM.
Best regards
Thomas
>
> Sam
>
>
>> -
>> - return 0;
>> -}
>> -
>> int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>> {
>> struct mga_device *mdev;
>> @@ -121,10 +50,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>> mdev->unique_rev_id);
>> }
>>
>> - ret = mga_vram_init(mdev);
>> - if (ret)
>> - return ret;
>> -
>> ret = mgag200_mm_init(mdev);
>> if (ret)
>> goto err_mm;
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c
>> index 73e30901e0631..f56b0456994f4 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mm.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c
>> @@ -30,6 +30,49 @@
>>
>> #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)
>> {
>> struct drm_device *dev = mdev->dev;
>> @@ -40,6 +83,11 @@ int mgag200_mm_init(struct mga_device *mdev)
>> start = pci_resource_start(dev->pdev, 0);
>> len = pci_resource_len(dev->pdev, 0);
>>
>> + if (!devm_request_mem_region(dev->dev, start, len, "mgadrmfb_vram")) {
>> + drm_err(dev, "can't reserve VRAM\n");
>> + return -ENXIO;
>> + }
>> +
>> arch_io_reserve_memtype_wc(start, len);
>>
>> mdev->fb_mtrr = arch_phys_wc_add(start, len);
>> @@ -50,6 +98,10 @@ int mgag200_mm_init(struct mga_device *mdev)
>> goto err_arch_phys_wc_del;
>> }
>>
>> + mdev->mc.vram_size = mgag200_probe_vram(mdev, mdev->vram, len);
>> + mdev->mc.vram_base = start;
>> + mdev->mc.vram_window = len;
>> +
>> mdev->vram_fb_available = mdev->mc.vram_size;
>>
>> return 0;
>> --
>> 2.26.2
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
