Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function

From: Javier Martinez Canillas <javierm@redhat.com>
To: "Michal Suchánek" <msuchanek@suse.de>
Cc: linux-fbdev@vger.kernel.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	airlied@linux.ie, mpe@ellerman.id.au, deller@gmx.de,
	mark.cave-ayland@ilande.co.uk, linuxppc-dev@lists.ozlabs.org,
	dri-devel@lists.freedesktop.org, paulus@samba.org,
	maxime@cerno.tech, geert@linux-m68k.org, sam@ravnborg.org
Subject: Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function
Date: Tue, 26 Jul 2022 21:22:05 +0200	[thread overview]
Message-ID: <90aef621-b686-12dd-de55-9a680f5783d7@redhat.com> (raw)
In-Reply-To: <20220726144024.GP17705@kitsune.suse.cz>

Hello Michal,

On 7/26/22 16:40, Michal Suchánek wrote:
> Hello,
> 
> On Tue, Jul 26, 2022 at 03:38:37PM +0200, Javier Martinez Canillas wrote:
>> On 7/20/22 16:27, Thomas Zimmermann wrote:
>>> Add a per-model device-function structure in preparation of adding
>>> color-management support. Detection of the individual models has been
>>> taken from fbdev's offb.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>>
>> [...]
>>
>>> +static bool is_avivo(__be32 vendor, __be32 device)
>>> +{
>>> +	/* This will match most R5xx */
>>> +	return (vendor == 0x1002) &&
>>> +	       ((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
>>> +}
>>
>> Maybe add some constant macros to not have these magic numbers ?
> 
> This is based on the existing fbdev implementation's magic numbers:
> 
> drivers/video/fbdev/offb.c:                 ((*did >= 0x7100 && *did < 0x7800) ||
>

Ah, I see. Then we might have to go with the magic numbers...

> Of course, it would be great if somebody knowledgeable could clarify
> those.
>

Indeed.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat