Re: [PATCH 3/5] drm/simpledrm: Request memory region in driver

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 24.01.22 um 15:23 schrieb Jocelyn Falempe:
> On 24/01/2022 13:36, Thomas Zimmermann wrote:
>> Requesting the framebuffer memory in simpledrm marks the memory
>> range as busy. This used to be done by the firmware sysfb code,
>> but the driver is the correct place.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/tiny/simpledrm.c | 20 +++++++++++++++-----
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c 
>> b/drivers/gpu/drm/tiny/simpledrm.c
>> index 04146da2d1d8..f72b71511a65 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -526,21 +526,31 @@ static int simpledrm_device_init_mm(struct 
>> simpledrm_device *sdev)
>>   {
>>       struct drm_device *dev = &sdev->dev;
>>       struct platform_device *pdev = sdev->pdev;
>> -    struct resource *mem;
>> +    struct resource *res, *mem;
>>       void __iomem *screen_base;
>>       int ret;
>> -    mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    if (!mem)
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    if (!res)
>>           return -EINVAL;
>> -    ret = devm_aperture_acquire_from_firmware(dev, mem->start, 
>> resource_size(mem));
>> +    ret = devm_aperture_acquire_from_firmware(dev, res->start, 
>> resource_size(res));
>>       if (ret) {
>>           drm_err(dev, "could not acquire memory range %pr: error %d\n",
>> -            mem, ret);
>> +            res, ret);
>>           return ret;
>>       }
>> +    mem = devm_request_mem_region(&pdev->dev, res->start, 
>> resource_size(res),
>> +                      sdev->dev.driver->name);
>> +    if (!mem) {
>> +        /*
>> +         * We cannot make this fatal. Sometimes this comes from magic
>> +         * spaces our resource handlers simply don't know about
>> +         */
>> +        drm_warn(dev, "could not acquire memory region %pr\n", res);
>> +    }
>> +
>>       screen_base = devm_ioremap_wc(&pdev->dev, mem->start,
>>                         resource_size(mem));
> 
> if mem is NULL, accessing mem->start will segfault after the warning.
> I think you renamed "mem" to "res" so probably it should be renamed here 
> too ?

Thanks for reviewing. Will be fixed in the next version. That code used 
to fail and i changed it to a warning after sync'ing with the simplefb 
driver. :/

Best regards
Thomas

> 
>>       if (!screen_base)
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]