Re: [PATCH 2/4] fbdev/efifb: Use screen_info pointer from device

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1631 bytes --]

Hi Javier

Am 01.12.23 um 09:54 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
>> Use the screen_info instance from the device instead of dereferencing
>> the global screen_info state. Decouples the driver from per-architecture
>> code. Duplicated the screen_info data, so that efifb can modify it at
>> will.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
> 
> [...]
> 
>> +	si = dev_get_platdata(&dev->dev);
>> +	if (!si)
> 
> I would add a comment that this platform data is set when the device is
> registered by sysfb.
> 
>> +		return -ENODEV;
>> +	si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
>> +	if (!si)
>> +		return -ENOMEM;
>> +
> 
> Why a copy? In any case maybe the global screen_info should be duplicated
> when is set as the device platform data in sysfb_init() ?

We get our own copy of the global screen_info as platform-device data. 
Efifb modifies some of the values in our copy in efifb_setup(). If 
probing afterwards fails, the kernel might try a different driver, which 
would then operate on the values modified by efifb. Hence, there's this 
internal copy. The situation with vesafb is similar.

Best regards
Thomas

> 
> I agree with the direction of the patch though, so whatever you decide:
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]