Re: [PATCH v2] xen/displif: Protocol version 2

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>]

Ping

On 7/1/20 10:19 AM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> 1. Add protocol version as an integer
>
> Version string, which is in fact an integer, is hard to handle in the
> code that supports different protocol versions. To simplify that
> also add the version as 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.
>
> 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.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>   xen/include/public/io/displif.h | 91 +++++++++++++++++++++++++++++++--
>   1 file changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
> index cc5de9cb1f35..0055895510f7 100644
> --- a/xen/include/public/io/displif.h
> +++ b/xen/include/public/io/displif.h
> @@ -38,7 +38,8 @@
>    *                           Protocol version
>    ******************************************************************************
>    */
> -#define XENDISPL_PROTOCOL_VERSION     "1"
> +#define XENDISPL_PROTOCOL_VERSION     "2"
> +#define XENDISPL_PROTOCOL_VERSION_INT  2
>   
>   /*
>    ******************************************************************************
> @@ -202,6 +203,9 @@
>    *      Width and height of the connector in pixels separated by
>    *      XENDISPL_RESOLUTION_SEPARATOR. This defines visible area of the
>    *      display.
> + *      If backend provides extended display identification data (EDID) with
> + *      XENDISPL_OP_GET_EDID request then EDID values must take precedence
> + *      over the resolutions defined here.
>    *
>    *------------------ Connector Request Transport Parameters -------------------
>    *
> @@ -349,6 +353,8 @@
>   #define XENDISPL_OP_FB_DETACH         0x13
>   #define XENDISPL_OP_SET_CONFIG        0x14
>   #define XENDISPL_OP_PG_FLIP           0x15
> +/* The below command is available in protocol version 2 and above. */
> +#define XENDISPL_OP_GET_EDID          0x16
>   
>   /*
>    ******************************************************************************
> @@ -377,6 +383,10 @@
>   #define XENDISPL_FIELD_BE_ALLOC       "be-alloc"
>   #define XENDISPL_FIELD_UNIQUE_ID      "unique-id"
>   
> +#define XENDISPL_EDID_BLOCK_SIZE      128
> +#define XENDISPL_EDID_BLOCK_COUNT     256
> +#define XENDISPL_EDID_MAX_SIZE        (XENDISPL_EDID_BLOCK_SIZE * XENDISPL_EDID_BLOCK_COUNT)
> +
>   /*
>    ******************************************************************************
>    *                          STATUS RETURN CODES
> @@ -451,7 +461,9 @@
>    * +----------------+----------------+----------------+----------------+
>    * |                           gref_directory                          | 40
>    * +----------------+----------------+----------------+----------------+
> - * |                             reserved                              | 44
> + * |                             data_ofs                              | 44
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 48
>    * +----------------+----------------+----------------+----------------+
>    * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>    * +----------------+----------------+----------------+----------------+
> @@ -494,6 +506,7 @@
>    *   buffer size (buffer_sz) exceeds what can be addressed by this single page,
>    *   then reference to the next page must be supplied (see gref_dir_next_page
>    *   below)
> + * data_ofs - uint32_t, offset of the data in the buffer, octets
>    */
>   
>   #define XENDISPL_DBUF_FLG_REQ_ALLOC       (1 << 0)
> @@ -506,6 +519,7 @@ struct xendispl_dbuf_create_req {
>       uint32_t buffer_sz;
>       uint32_t flags;
>       grant_ref_t gref_directory;
> +    uint32_t data_ofs;
>   };
>   
>   /*
> @@ -731,6 +745,44 @@ struct xendispl_page_flip_req {
>       uint64_t fb_cookie;
>   };
>   
> +/*
> + * Request EDID - request EDID describing current connector:
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |               id                | _OP_GET_EDID   |   reserved     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                             buffer_sz                             | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                          gref_directory                           | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 16
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 64
> + * +----------------+----------------+----------------+----------------+
> + *
> + * Notes:
> + *   - This command is not available in protocol version 1 and should be
> + *     ignored.
> + *   - This request is optional and if not supported then visible area
> + *     is defined by the relevant XenStore's "resolution" property.
> + *   - Shared buffer, allocated for EDID storage, must not be less then
> + *     XENDISPL_EDID_MAX_SIZE octets.
> + *
> + * buffer_sz - uint32_t, buffer size to be allocated, octets
> + * gref_directory - grant_ref_t, a reference to the first shared page
> + *   describing EDID buffer references. See XENDISPL_OP_DBUF_CREATE for
> + *   grant page directory structure (struct xendispl_page_directory).
> + *
> + * See response format for this request.
> + */
> +
> +struct xendispl_get_edid_req {
> +    uint32_t buffer_sz;
> +    grant_ref_t gref_directory;
> +};
> +
>   /*
>    *---------------------------------- Responses --------------------------------
>    *
> @@ -753,6 +805,35 @@ struct xendispl_page_flip_req {
>    * id - uint16_t, private guest value, echoed from request
>    * status - int32_t, response status, zero on success and -XEN_EXX on failure
>    *
> + *
> + * Get EDID response - response for XENDISPL_OP_GET_EDID:
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |               id                |    operation   |    reserved    | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                              status                               | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                             edid_sz                               | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 16
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 64
> + * +----------------+----------------+----------------+----------------+
> + *
> + * Notes:
> + *   - This response is not available in protocol version 1 and should be
> + *     ignored.
> + *
> + * edid_sz - uint32_t, size of the EDID, octets
> + */
> +
> +struct xendispl_get_edid_resp {
> +    uint32_t edid_sz;
> +};
> +
> +/*
>    *----------------------------------- Events ----------------------------------
>    *
>    * Events are sent via a shared page allocated by the front and propagated by
> @@ -804,6 +885,7 @@ struct xendispl_req {
>           struct xendispl_fb_detach_req fb_detach;
>           struct xendispl_set_config_req set_config;
>           struct xendispl_page_flip_req pg_flip;
> +        struct xendispl_get_edid_req get_edid;
>           uint8_t reserved[56];
>       } op;
>   };
> @@ -813,7 +895,10 @@ struct xendispl_resp {
>       uint8_t operation;
>       uint8_t reserved;
>       int32_t status;
> -    uint8_t reserved1[56];
> +    union {
> +        struct xendispl_get_edid_resp get_edid;
> +        uint8_t reserved1[56];
> +    } op;
>   };
>   
>   struct xendispl_evt {