[PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
Noralf Trønnes
noralf at tronnes.org
Tue Aug 21 14:59:46 UTC 2018
Den 21.08.2018 10.44, skrev Daniel Vetter:
> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf at tronnes.org> wrote:
>>> This switches the CMA helper drivers that use its fbdev emulation over
>>> to the generic fbdev emulation. It's the first phase of using generic
>>> fbdev. A later phase will use DRM client callbacks for the
>>> lastclose/hotplug/remove callbacks.
>>>
>>> There are currently 2 fbdev init/fini functions:
>>> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
>>> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>>>
>>> This is because the work on generic fbdev came up during a fbdev
>>> refactoring and thus wasn't completed. No point in completing that
>>> refactoring when drivers will soon move to drm_fb_helper_generic_probe().
>>>
>>> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>>>
>>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
>>> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> ---
>>> drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
>>> include/drm/drm_fb_cma_helper.h | 3 -
>>> 2 files changed, 42 insertions(+), 321 deletions(-)
>> ...
>>> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
>>> - struct drm_gem_cma_object *cma_obj)
>>> -{
>>> - struct fb_deferred_io *fbdefio;
>>> - struct fb_ops *fbops;
>>> -
>>> - /*
>>> - * Per device structures are needed because:
>>> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
>>> - * fbdefio: individual delays
>>> - */
>>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>> - if (!fbdefio || !fbops) {
>>> - kfree(fbdefio);
>>> - kfree(fbops);
>>> - return -ENOMEM;
>>> - }
>>> -
>>> - /* can't be offset from vaddr since dirty() uses cma_obj */
>>> - fbi->screen_buffer = cma_obj->vaddr;
>>> - /* fb_deferred_io_fault() needs a physical address */
>>> - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>> -
>>> - *fbops = *fbi->fbops;
>>> - fbi->fbops = fbops;
>>> -
>>> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
>>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>> - fbi->fbdefio = fbdefio;
>>> - fb_deferred_io_init(fbi);
>>> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>>> -{
>>> - if (!fbi->fbdefio)
>>> - return;
>>> -
>>> - fb_deferred_io_cleanup(fbi);
>>> - kfree(fbi->fbdefio);
>>> - kfree(fbi->fbops);
>>> -}
>>> -
>>> -static int
>>> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>> - struct drm_fb_helper_surface_size *sizes)
>>> -{
>>> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
>>> - struct drm_device *dev = helper->dev;
>>> - struct drm_gem_cma_object *obj;
>>> - struct drm_framebuffer *fb;
>>> - unsigned int bytes_per_pixel;
>>> - unsigned long offset;
>>> - struct fb_info *fbi;
>>> - size_t size;
>>> - int ret;
>>> -
>>> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
>>> - sizes->surface_width, sizes->surface_height,
>>> - sizes->surface_bpp);
>>> -
>>> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>> - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
>>> - obj = drm_gem_cma_create(dev, size);
>>> - if (IS_ERR(obj))
>>> - return -ENOMEM;
>>> -
>>> - fbi = drm_fb_helper_alloc_fbi(helper);
>>> - if (IS_ERR(fbi)) {
>>> - ret = PTR_ERR(fbi);
>>> - goto err_gem_free_object;
>>> - }
>>> -
>>> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
>>> - fbdev_cma->fb_funcs);
>>> - if (IS_ERR(fb)) {
>>> - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
>>> - ret = PTR_ERR(fb);
>>> - goto err_fb_info_destroy;
>>> - }
>>> -
>>> - helper->fb = fb;
>>> -
>>> - fbi->par = helper;
>>> - fbi->flags = FBINFO_FLAG_DEFAULT;
>>> - fbi->fbops = &drm_fbdev_cma_ops;
>>> -
>>> - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
>>> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
>>> -
>>> - offset = fbi->var.xoffset * bytes_per_pixel;
>>> - offset += fbi->var.yoffset * fb->pitches[0];
>>> -
>>> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
>>> - fbi->screen_base = obj->vaddr + offset;
>>> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
>> Hey Noralf, all,
>> I've been digging for a bit on the regression that this patch has
>> tripped on the HiKey board as reported here:
>> https://lkml.org/lkml/2018/8/16/81
>>
>> The first issue was that the kirin driver was setting
>> mode_config.max_width/height = 2048, which was causing errors as the
>> the requested resolution was 1920x2160 (due to surfaceflinger
>> requesting y*2 for page flipping). This was relatively simple enough
>> to figure out and fix, but bumping the values up on its own didn't
>> seem to resolve the issue entirely, as I started to see hard hangs on
>> bootup when userspace started using the emulated fbdev device.
>>
>> After confirming reverting the patch here worked around it, I started
>> digging through what might be different, and I think I've found it.
>> In the drm_fbdev_cma_create() function above, we set the
>> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
>> which now replaces that, we don't set smem_start value at all.
>>
>> Since we don't have a drm_gem_cma_object reference in
>> drm_fb_helper_generic_probe(), I instead added:
>> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>
>> And that got things working!
>>
>> So yea, I wanted to figure out if we are just missing the line above
>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>> setting the fix.smem_start in some other fashion?
> With the generic helpers we can't use the generic fb_mmap() implementation
> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
> you pls double-check that this is wired up correctly for kirin?
>
> At least I don't see any other place where we use smem_start in the fbdev
> code. It does get copied to userspace, but userspace should never use
> that.
I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
about the Mali blob using it and HiKey seems to have a Mali GPU:
00:17 <ssvb> BorgCuba:
the mali blob is supposed to open the framebuffer device,
get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
Could this be the problem here?
Noralf.
[1] https://irclog.whitequark.org/linux-rockchip/2015-12-22
More information about the dri-devel
mailing list