Den 21.08.2018 17.41, skrev Daniel Vetter:
On Tue, Aug 21, 2018 at 04:59:46PM +0200, Noralf Trønnes wrote:
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@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
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?
Yup, most certainly is. As you analyzed in your other reply, smem_start
shouldn't matter at all for fbdev mmap support.
And the only other place it's used is the ioctl.
I'm pretty sure John owes us one, because the Oops he's seeing is in the
mali kernel driver for the mali userspace blob :-)
Ah, so it makes sense after all.
The change John did:
fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
is the conversions legal on all platforms? It doesn't have to be a legal
value, but it can't be oops'ing or fail to build for some other user.
If it's ok we can just add that line and be done with it.
Noralf.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
