[PATCH weston v4 4/4] simple-dmabuf-drm: support etnaviv drm as well

[Pekka Paalanen]

On Mon, 19 Mar 2018 15:24:12 -0500
Derek Foreman <derekf at osg.samsung.com> wrote:

> On 2018-03-19 11:45 AM, Guido Günther wrote:
> > Signed-off-by: Guido Günther <agx at sigxcpu.org>
> > ---
> >   Makefile.am                 |  1 +
> >   clients/simple-dmabuf-drm.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
> >   configure.ac                |  5 ++-
> >   3 files changed, 82 insertions(+), 1 deletion(-)

> > +static int
> > +etna_map_bo(struct buffer *buf)
> > +{
> > +	buf->mmap = etna_bo_map(buf->etna_bo);
> > +
> > +	if (buf->mmap != NULL)
> > +		return 1;
> > +
> > +	return 0;
> > +}
> > +
> > +static void
> > +etna_unmap_bo(struct buffer *buf)
> > +{
> > +	if (munmap(buf->mmap, buf->stride * buf->height) < 0)
> > +		fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));  
> 
> Pekka had said something about cache flushing here in a previous 
> comment, I think.  I think this munmap() is enough, but if it isn't I'd 
> like to know what's required too.

Hi,

well, I was kind of hand-waving there. Surely the unmap function needs
to do *something*, otherwise it's at least a leak, leaves a driver
unmap/flush path untested, and what the flushing comment referred to
specifically, you often need to tell the driver/hardware somehow after
CPU access is over, because cache coherency is usually all lies.

I have no knowledge to review what a proper unmap is for this driver,
so if you say this code gets called and it seems to work ok, that's
enough for me. As long as it's doing something.

If something like etna_bo_unmap() does not exist, while etna_bo_map()
exists, you might want to look into why that is. Such asymmetry is
often an indicator of something missing.

> 
> > +	buf->mmap = NULL;
> > +}

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180320/ee03c788/attachment.sig>