Re: [PATCH v2 21/43] drm/fbdev-dma: Implement damage handling and deferred I/O

[Javier Martinez Canillas <javierm@redhat.com>]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Hi
>
> Am 16.04.24 um 14:18 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>

[...]

>>> +static int drm_fbdev_dma_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
>>> +{
>>> +	struct drm_fb_helper *fb_helper = info->par;
>>> +	struct drm_framebuffer *fb = fb_helper->fb;
>>> +	struct drm_gem_dma_object *dma = drm_fb_dma_get_gem_obj(fb, 0);
>>> +
>>> +	if (!dma->map_noncoherent)
>>> +		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
>> I noticed that some drivers do:
>>
>>                   vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>>
>> I see that vm_get_page_prot() is a per-architecture function, but I don't
>> know about the implications of getting the pgprot_t from the vma->vm_flags
>> set or just using the current vma->vm_page_prot value...
>
> That's in interesting observation. The code in the patch adds a WC flag 
> to the existing vm_page_prot. The code in your example first creates a 
> new vm_page_prot from the vm_flags field. Fbdev drivers generally use 
> the former approach. So where does the original vm_page_prot value come 
> from? (I think that's also the question behind your comment.)
>

Yes, also if the vm_flags were set (and where) for this VMA.

> I've looked through the kernel's mmap code from the syscall [1] to the 
> place where it invokes the mmap callback. [2] Shortly before doing so, 
> mmap_region() set's vm_page_prot from vm_flags like in your example. [3] 
> I would assume there's no reason for drivers to call vm_get_page_prot() 
> by themselves. DRM drivers specially seem to have the habit of doing so.
>

Got it, makes sense. Thanks for taking a look.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat