Thomas Zimmermann <tzimmermann@xxxxxxx> writes:
> Hi
>
> Am 16.04.24 um 14:18 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@xxxxxxx> 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
