Re: [PATCH v3 1/3] drm: add SimpleDRM driver

From: Noralf TrÃnnes
Date: Wed Aug 17 2016 - 06:19:13 EST



Den 17.08.2016 11:30, skrev Daniel Vetter:
On Tue, Aug 16, 2016 at 9:38 PM, Noralf TrÃnnes <noralf@xxxxxxxxxxx> wrote:
That's still a lot for what amounts to reimplementing mmap on shmem, but
badly. What I mean with redirecting is pointing the entire ->mmap
operation to the mmap implementation for the underlying mmap. Roughly:

/* normal gem mmap checks first */

/* redirect to shmem mmap */
vma->vm_file = obj->filp;
vma->vm_pgoff = 0;

return obj->filp->f_op->mmap(obj->filp, vma);

Much less code ;-)

obj->filp is NULL in my case.

And looking at the docs, that's expected since I have driver specific
backing?

/**
* @filp:
*
* SHMEM file node used as backing storage for swappable buffer objects.
* GEM also supports driver private objects with driver-specific backing
* storage (contiguous CMA memory, special reserved blocks). In this
* case @filp is NULL.
*/
Hm, I totally misread the driver code. I assumed that we'd just
allocate normal shmem gem objects, and then copy them on-demand onto
the frontbuffer (in the dirty or plane update callbacks). Essentially
treat the firmware fb area as a manual upload display, except that we
don't use i2c or spi to do the upload, but normal mmio writes. I think
that would greatly simplify the driver, and more important: It would
also work like any other kms driver. Currently sdrm is violiting the
spec a bit by aliasing all dumb buffers to the same underlying backing
storage, and that's a bit evil.

The other bit I noticed (and why I was confused): The prime import
code reinvents a lot of wheels, and it digs into the backing storage
directly. Instead it should just call dma_buf_vmap/dma_buf_vunmap and
let the exporter figure out how it works.

Is there a driver I can look at that does it the way we want?
I'm afraid that this gem framework is still over my head.
Copying code or patterns is easy, starting from scratch demands understanding :-)


Noralf.