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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 6/30/20 10:30 AM, Jürgen Groß wrote:
> On 30.06.20 09:09, Oleksandr Andrushchenko wrote:
>> On 6/30/20 10:03 AM, Jürgen Groß wrote:
>>> On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
>>>> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>>>>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> 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@xxxxxxxx>
>>>>>> ---
>>>>>>    xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>>>>>    1 file changed, 80 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/xen/include/public/io/displif.h 
>>>>>> b/xen/include/public/io/displif.h
>>>>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>>>>> --- a/xen/include/public/io/displif.h
>>>>>> +++ b/xen/include/public/io/displif.h
>>>>>> @@ -38,7 +38,7 @@
>>>>>>     *                           Protocol version
>>>>>> ******************************************************************************
>>>>>>     */
>>>>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>>>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>>>>
>>>>> This is not very nice regarding compatibility.
>>>>>
>>>>> Can't you add a new macro for the integer value?
>>>>
>>>> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
>>>>
>>>> so we do not break the existing users. Then if every user of the header has
>>>>
>>>> its local copy (this is what we now use in the display backend), then that
>>>> local copy can be changed in a way suitable for the concrete user, e.g. "2"
>>>>
>>>> redefined to 2. This cannot be done (?) for the Linux Kernel though.
>>>>
>>>> Or we can have
>>>>
>>>> #define XENDISPL_PROTOCOL_VERSION     "2"
>>>>
>>>> #define XENDISPL_PROTOCOL_VERSION_INT  2
>>>>
>>>> Jurgen, what's your preference here?
>>>
>>> I would prefer the latter, as it avoids the need to modify the header
>>> when copying it to a local project.
>>>
>> Ok, I'll have 2 definitions then
>>
>> Anything else (beside the comments on new functionality) I can add/change
>>
>> before sending v2 of the patch?
>
> I would have said so. :-)

Thank you,

the series also has a patch for libgnttab. Do you want me to send the protocol 
patch

separately or should we wait for Ian and Wei to review? These changes are 
related,

thus I sent then togheter

>
>
> Juergen

Thank you,

Oleksandr