Re: [PATCH v1 1/4] video: simplefb-client: switch to dev_get_resource

[Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>]

On 7/30/25 11:11, Ahmad Fatoum wrote:
> Hi Ivaylo,
>
> thanks for your patch.
>
> On 7/29/25 22:36, Ivaylo Ivanov wrote:
>> Since the framebuffer memory resource resides in ram, it has already
>> been requested and mapped, so only get the resource to avoid
>> requesting a busy resource. This is also the approach for linux.
> even if it's regular RAM (as opposed to video memory) we aren't
> guaranteed, it's mapped suitably.

In my case at least, the mainline device tree covers all of dram -
(0x80000000 + n GB)

>
> I think a remap of the region to writecombine is in order here. It
> probably works right now, because we flush caches before shutting down
> the MMU, but if you were to run fbtest, I'd expect that you will see
> artifacts due to some pixels remaining in dirty cache line.

So you suggest remapping it, like ipufb for example?

    fbi->info.screen_base = dma_alloc_writecombine(DMA_DEVICE_BROKEN,
                           info->line_length * info->yres,
                           DMA_ADDRESS_BROKEN);
>
>> While at it, use IOMEM for mem->start and drop an unnecessary newline.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> Patch is correct, except for that aspect of the commit message:

I don't get it, do I have to change the commit message only, or
code logic too?

Thanks and best regards,
Ivaylo

>
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> Thanks,
> Ahmad
>
>> ---
>>  drivers/video/simplefb-client.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/simplefb-client.c b/drivers/video/simplefb-client.c
>> index dafec617..41ad8ffa 100644
>> --- a/drivers/video/simplefb-client.c
>> +++ b/drivers/video/simplefb-client.c
>> @@ -96,7 +96,7 @@ static int simplefb_probe(struct device *dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	mem = dev_request_mem_resource(dev, 0);
>> +	mem = dev_get_resource(dev, IORESOURCE_MEM, 0);
>>  	if (IS_ERR(mem)) {
>>  		dev_err(dev, "No memory resource\n");
>>  		return PTR_ERR(mem);
>> @@ -116,10 +116,9 @@ static int simplefb_probe(struct device *dev)
>>  	info->blue = params.format->blue;
>>  	info->transp = params.format->transp;
>>  
>> -	info->screen_base = (void *)mem->start;
>> +	info->screen_base = IOMEM(mem->start);
>>  	info->screen_size = resource_size(mem);
>>  
>> -
>>  	info->fbops = &simplefb_ops;
>>  
>>  	info->dev.parent = dev;