From: "Jürgen Groß" <jgross@suse.com>
To: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>,
Oleksandr Andrushchenko <andr2000@gmail.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"ian.jackson@eu.citrix.com" <ian.jackson@eu.citrix.com>,
"wl@xen.org" <wl@xen.org>,
"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>
Subject: Re: [PATCH 1/2] xen/displif: Protocol version 2
Date: Tue, 30 Jun 2020 09:57:23 +0200 [thread overview]
Message-ID: <abe88a6c-fba7-a901-46e8-43761ddf8582@suse.com> (raw)
In-Reply-To: <24252f32-ef40-0daf-a585-36cb00f149d0@epam.com>
On 30.06.20 09:39, Oleksandr Andrushchenko wrote:
> 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@epam.com>
>>>>>>>
>>>>>>> 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@epam.com>
>>>>>>> ---
>>>>>>> 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
This is something to ask the tools maintainers IMO.
Juergen
next prev parent reply other threads:[~2020-06-30 7:57 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-20 9:04 [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
2020-05-20 9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
2020-06-29 7:02 ` Jürgen Groß
2020-06-30 6:13 ` Oleksandr Andrushchenko
2020-06-30 7:03 ` Jürgen Groß
2020-06-30 7:09 ` Oleksandr Andrushchenko
2020-06-30 7:30 ` Jürgen Groß
2020-06-30 7:39 ` Oleksandr Andrushchenko
2020-06-30 7:57 ` Jürgen Groß [this message]
2020-05-20 9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
2020-06-30 9:42 ` Oleksandr Andrushchenko
2020-07-31 10:53 ` Oleksandr Andrushchenko
2020-09-28 15:20 ` Ian Jackson
2020-10-01 6:35 ` Oleksandr Andrushchenko
2021-06-08 7:54 ` Oleksandr Andrushchenko
2020-06-01 16:01 ` [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abe88a6c-fba7-a901-46e8-43761ddf8582@suse.com \
--to=jgross@suse.com \
--cc=Oleksandr_Andrushchenko@epam.com \
--cc=andr2000@gmail.com \
--cc=ian.jackson@eu.citrix.com \
--cc=konrad.wilk@oracle.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).