Re: [PATCH 4/4] mgag200: remove mgag200_probe_vram()

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

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@xxxxxxxxxx>
> ---
>   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;
>   

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature