ping
On 5/20/20 12:04 PM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com> > > Hello all, > > this series extends the existing displif protocol with new > request and adds additional parameter to the exiting one. > It also provides support for the new parameter in libgnttab > via ioctl. The relevant changes in the backend can be found at [1] > and the frontend at [2]. > > List of changes: > > 1. Change protocol version from string to integer > > Version string, which is in fact an integer, is hard to handle in the > code that supports different protocol versions. To simplify that > make the version an integer. > > 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE > > There are cases when display data buffer is created with non-zero > offset to the data start. Handle such cases and provide that offset > while creating a display buffer. Add the corresponding filed to the > protocol and handle it in libgnttab. > This change is required for bringing virtual display on iMX8 > platform which uses offset of 64 bytes for the buffers allocated > on GPU side and then imported into PV DRM frontend. Otherwise the > final picture looks shifted. > > 3. Add XENDISPL_OP_GET_EDID command > > Add an optional request for reading Extended Display Identification > Data (EDID) structure which allows better configuration of the > display connectors over the configuration set in XenStore. > With this change connectors may have multiple resolutions defined > with respect to detailed timing definitions and additional properties > normally provided by displays. > > If this request is not supported by the backend then visible area > is defined by the relevant XenStore's "resolution" property. > > If backend provides extended display identification data (EDID) with > XENDISPL_OP_GET_EDID request then EDID values must take precedence > over the resolutions defined in XenStore. > > 4. Bump protocol version to 2. > > Open questions and notes on the changes: > > 1. gnttab minor version change from 2 to 3 > As per my understanding it is required to bump the version when > I add the new functionality, but I would like to hear from the > maintainers on that. > > 2. gnttab and version 2 of the ioctls > Because we add an additional parameter (data offset) and the structures > used to pass ioctl arguments cannot be extended (there are no enough > reserved fields), so there are to ways to solve that: > - break the existing API and add data_ofs field into the existing > structures > - create a copy of the ioctl (v2) with the additional parameter. > > It seems to be easier to go the first way, but this breaks things, > so I decided to introduce v2 of the same ioctl which behaves gracefully > with respect to the existing users, but adds some amount of > copy-paste code (I was trying to minimize copy-paste so it is easier > to maintain, but the result looked ugly to me because of lots of > "if (protocol v1)" constructs. > > Please note that struct ioctl_gntdev_dmabuf_imp_to_refs, e.g. > version 1 of the ioctl, has uint32_t reserved field which can be > used for the data offset, but its counterpart > (ioctl_gntdev_dmabuf_exp_from_refs) > doesn't have any, so it seems not to be aligned to only introduce > version 2 of the ioctl for the later. > > The other question is if to keep that reserved field in > ioctl_gntdev_dmabuf_imp_to_refs_v2 structure or drop it. > > 3. displif protocol version string to int conversion > The existing protocol defines its version as a string "1" > which is ok, but makes it not so easy to be directly used by > C/C++ preprocessor which would like to see an integer for constructs > like "#if XENDISPL_PROTOCOL_VERSION > 2" > > At the same time this change may break the existing users of the protocol > which still expect version as a string. I tried something like > > #define STR_HELPER(x) #x > #define STR(x) STR_HELPER(x) > > #define XENDISPL_PROTOCOL_VERSION_INT 1 > #define XENDISPL_PROTOCOL_VERSION STR(XENDISPL_PROTOCOL_VERSION_INT) > > but not sure if this is the right and good way to solve the issue, > so in this series I have deliberately changed the protocol version to > int. > Other possible way could be that every user of the header has its local > copy (this is what we now use in the display backend). This way the > local copy can be changed in a way suitable for the concrete user. > This cannot be done (?) for the Linux Kernel though. > > Thank you, > Oleksandr > > [1] https://github.com/xen-troops/displ_be > [2] https://github.com/xen-troops/linux/pull/87 > > Oleksandr Andrushchenko (2): > xen/displif: Protocol version 2 > libgnttab: Add support for Linux dma-buf offset > > tools/include/xen-sys/Linux/gntdev.h | 53 +++++++++++++++++ > tools/libs/gnttab/Makefile | 2 +- > tools/libs/gnttab/freebsd.c | 15 +++++ > tools/libs/gnttab/gnttab_core.c | 17 ++++++ > tools/libs/gnttab/include/xengnttab.h | 13 ++++ > tools/libs/gnttab/libxengnttab.map | 6 ++ > tools/libs/gnttab/linux.c | 86 +++++++++++++++++++++++++++ > tools/libs/gnttab/minios.c | 15 +++++ > tools/libs/gnttab/private.h | 9 +++ > xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++- > 10 files changed, 295 insertions(+), 4 deletions(-) >
