| Message ID | 20181005125154.19824-1-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] virtio-gpu: add VIRTIO_GPU_F_EDID feature | expand |
> On 5 Oct 2018, at 14:51, Gerd Hoffmann <kraxel@redhat.com> wrote: > > The feature allows the guest request an EDID blob (describing monitor > capabilities) for a given scanout (aka virtual monitor connector). > > It brings a new command message, which has just a scanout field (beside > the standard virtio-gpu header) and a response message which carries the > EDID data. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > include/uapi/linux/virtio_gpu.h | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h > index f43c3c6171..1cef1ff339 100644 > --- a/include/uapi/linux/virtio_gpu.h > +++ b/include/uapi/linux/virtio_gpu.h > @@ -41,6 +41,7 @@ > #include <linux/types.h> > > #define VIRTIO_GPU_F_VIRGL 0 > +#define VIRTIO_GPU_F_EDID 1 > > enum virtio_gpu_ctrl_type { > VIRTIO_GPU_UNDEFINED = 0, > @@ -56,6 +57,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING, > VIRTIO_GPU_CMD_GET_CAPSET_INFO, > VIRTIO_GPU_CMD_GET_CAPSET, > + VIRTIO_GPU_CMD_GET_EDID, > > /* 3d commands */ > VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, > @@ -76,6 +78,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_RESP_OK_DISPLAY_INFO, > VIRTIO_GPU_RESP_OK_CAPSET_INFO, > VIRTIO_GPU_RESP_OK_CAPSET, > + VIRTIO_GPU_RESP_OK_EDID, > > /* error responses */ > VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, > @@ -291,6 +294,20 @@ struct virtio_gpu_resp_capset { > __u8 capset_data[]; > }; > > +/* VIRTIO_GPU_CMD_GET_EDID */ > +struct virtio_gpu_get_edid { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 scanout; > +}; > + > +/* VIRTIO_GPU_RESP_OK_EDID */ > +struct virtio_gpu_resp_edid { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 scanout; > + __le32 size; > + __u8 edid[1024]; Wouldn’t it be enough to stick to EDID 2.0 (256 bytes)? If not, maybe add comment to explain why you chose 1024. > +}; > + > #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) > > struct virtio_gpu_config { > -- > 2.9.3 >
On Fri, Oct 05, 2018 at 04:38:11PM +0200, Christophe de Dinechin wrote: > > > > On 5 Oct 2018, at 14:51, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > The feature allows the guest request an EDID blob (describing monitor > > capabilities) for a given scanout (aka virtual monitor connector). > > > > It brings a new command message, which has just a scanout field (beside > > the standard virtio-gpu header) and a response message which carries the > > EDID data. > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > include/uapi/linux/virtio_gpu.h | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h > > index f43c3c6171..1cef1ff339 100644 > > --- a/include/uapi/linux/virtio_gpu.h > > +++ b/include/uapi/linux/virtio_gpu.h > > @@ -41,6 +41,7 @@ > > #include <linux/types.h> > > > > #define VIRTIO_GPU_F_VIRGL 0 > > +#define VIRTIO_GPU_F_EDID 1 > > > > enum virtio_gpu_ctrl_type { > > VIRTIO_GPU_UNDEFINED = 0, > > @@ -56,6 +57,7 @@ enum virtio_gpu_ctrl_type { > > VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING, > > VIRTIO_GPU_CMD_GET_CAPSET_INFO, > > VIRTIO_GPU_CMD_GET_CAPSET, > > + VIRTIO_GPU_CMD_GET_EDID, > > > > /* 3d commands */ > > VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, > > @@ -76,6 +78,7 @@ enum virtio_gpu_ctrl_type { > > VIRTIO_GPU_RESP_OK_DISPLAY_INFO, > > VIRTIO_GPU_RESP_OK_CAPSET_INFO, > > VIRTIO_GPU_RESP_OK_CAPSET, > > + VIRTIO_GPU_RESP_OK_EDID, > > > > /* error responses */ > > VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, > > @@ -291,6 +294,20 @@ struct virtio_gpu_resp_capset { > > __u8 capset_data[]; > > }; > > > > +/* VIRTIO_GPU_CMD_GET_EDID */ > > +struct virtio_gpu_get_edid { > > + struct virtio_gpu_ctrl_hdr hdr; > > + __le32 scanout; > > +}; > > + > > +/* VIRTIO_GPU_RESP_OK_EDID */ > > +struct virtio_gpu_resp_edid { > > + struct virtio_gpu_ctrl_hdr hdr; > > + __le32 scanout; > > + __le32 size; > > + __u8 edid[1024]; > > Wouldn’t it be enough to stick to EDID 2.0 (256 bytes)? > > If not, maybe add comment to explain why you chose 1024. EDID in the wild can be up to 512 bytes. -Daniel > > > +}; > > + > > #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) > > > > struct virtio_gpu_config { > > -- > > 2.9.3 > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> On 5 Oct 2018, at 16:41, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, Oct 05, 2018 at 04:38:11PM +0200, Christophe de Dinechin wrote: >> >> >>> On 5 Oct 2018, at 14:51, Gerd Hoffmann <kraxel@redhat.com> wrote: >>> >>> The feature allows the guest request an EDID blob (describing monitor >>> capabilities) for a given scanout (aka virtual monitor connector). >>> >>> It brings a new command message, which has just a scanout field (beside >>> the standard virtio-gpu header) and a response message which carries the >>> EDID data. >>> >>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >>> --- >>> include/uapi/linux/virtio_gpu.h | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h >>> index f43c3c6171..1cef1ff339 100644 >>> --- a/include/uapi/linux/virtio_gpu.h >>> +++ b/include/uapi/linux/virtio_gpu.h >>> @@ -41,6 +41,7 @@ >>> #include <linux/types.h> >>> >>> #define VIRTIO_GPU_F_VIRGL 0 >>> +#define VIRTIO_GPU_F_EDID 1 >>> >>> enum virtio_gpu_ctrl_type { >>> VIRTIO_GPU_UNDEFINED = 0, >>> @@ -56,6 +57,7 @@ enum virtio_gpu_ctrl_type { >>> VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING, >>> VIRTIO_GPU_CMD_GET_CAPSET_INFO, >>> VIRTIO_GPU_CMD_GET_CAPSET, >>> + VIRTIO_GPU_CMD_GET_EDID, >>> >>> /* 3d commands */ >>> VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, >>> @@ -76,6 +78,7 @@ enum virtio_gpu_ctrl_type { >>> VIRTIO_GPU_RESP_OK_DISPLAY_INFO, >>> VIRTIO_GPU_RESP_OK_CAPSET_INFO, >>> VIRTIO_GPU_RESP_OK_CAPSET, >>> + VIRTIO_GPU_RESP_OK_EDID, >>> >>> /* error responses */ >>> VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, >>> @@ -291,6 +294,20 @@ struct virtio_gpu_resp_capset { >>> __u8 capset_data[]; >>> }; >>> >>> +/* VIRTIO_GPU_CMD_GET_EDID */ >>> +struct virtio_gpu_get_edid { >>> + struct virtio_gpu_ctrl_hdr hdr; >>> + __le32 scanout; >>> +}; >>> + >>> +/* VIRTIO_GPU_RESP_OK_EDID */ >>> +struct virtio_gpu_resp_edid { >>> + struct virtio_gpu_ctrl_hdr hdr; >>> + __le32 scanout; >>> + __le32 size; >>> + __u8 edid[1024]; >> >> Wouldn’t it be enough to stick to EDID 2.0 (256 bytes)? >> >> If not, maybe add comment to explain why you chose 1024. > > EDID in the wild can be up to 512 bytes. Does this return a physical EDID? I thought it would be made-up by virtio-gpu. Thanks, Christophe > >> >>> +}; >>> + >>> #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) >>> >>> struct virtio_gpu_config { >>> -- >>> 2.9.3 >>> >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Hi, > >>> +/* VIRTIO_GPU_RESP_OK_EDID */ > >>> +struct virtio_gpu_resp_edid { > >>> + struct virtio_gpu_ctrl_hdr hdr; > >>> + __le32 scanout; > >>> + __le32 size; > >>> + __u8 edid[1024]; > >> > >> Wouldn’t it be enough to stick to EDID 2.0 (256 bytes)? > >> > >> If not, maybe add comment to explain why you chose 1024. > > > > EDID in the wild can be up to 512 bytes. > > Does this return a physical EDID? I thought it would be made-up by virtio-gpu. Well, edid has extensions, so it can become pretty large in theory, and I've figured it would be a good idea to leave some room just in case. It should be a rather infrequent operation, so the unused buffer space should not hurt much. Yes, it will be a edid generated by qemu. The current generator code will use at most 256 bytes. cheers, Gerd
Reviewed-by: Dave Airlie <airlied@redhat.com> On Fri, 5 Oct 2018 at 22:52, Gerd Hoffmann <kraxel@redhat.com> wrote: > > The feature allows the guest request an EDID blob (describing monitor > capabilities) for a given scanout (aka virtual monitor connector). > > It brings a new command message, which has just a scanout field (beside > the standard virtio-gpu header) and a response message which carries the > EDID data. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > include/uapi/linux/virtio_gpu.h | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h > index f43c3c6171..1cef1ff339 100644 > --- a/include/uapi/linux/virtio_gpu.h > +++ b/include/uapi/linux/virtio_gpu.h > @@ -41,6 +41,7 @@ > #include <linux/types.h> > > #define VIRTIO_GPU_F_VIRGL 0 > +#define VIRTIO_GPU_F_EDID 1 > > enum virtio_gpu_ctrl_type { > VIRTIO_GPU_UNDEFINED = 0, > @@ -56,6 +57,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING, > VIRTIO_GPU_CMD_GET_CAPSET_INFO, > VIRTIO_GPU_CMD_GET_CAPSET, > + VIRTIO_GPU_CMD_GET_EDID, > > /* 3d commands */ > VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, > @@ -76,6 +78,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_RESP_OK_DISPLAY_INFO, > VIRTIO_GPU_RESP_OK_CAPSET_INFO, > VIRTIO_GPU_RESP_OK_CAPSET, > + VIRTIO_GPU_RESP_OK_EDID, > > /* error responses */ > VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, > @@ -291,6 +294,20 @@ struct virtio_gpu_resp_capset { > __u8 capset_data[]; > }; > > +/* VIRTIO_GPU_CMD_GET_EDID */ > +struct virtio_gpu_get_edid { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 scanout; > +}; > + > +/* VIRTIO_GPU_RESP_OK_EDID */ > +struct virtio_gpu_resp_edid { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 scanout; > + __le32 size; > + __u8 edid[1024]; > +}; > + > #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) > > struct virtio_gpu_config { > -- > 2.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h index f43c3c6171..1cef1ff339 100644 --- a/include/uapi/linux/virtio_gpu.h +++ b/include/uapi/linux/virtio_gpu.h @@ -41,6 +41,7 @@ #include <linux/types.h> #define VIRTIO_GPU_F_VIRGL 0 +#define VIRTIO_GPU_F_EDID 1 enum virtio_gpu_ctrl_type { VIRTIO_GPU_UNDEFINED = 0, @@ -56,6 +57,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING, VIRTIO_GPU_CMD_GET_CAPSET_INFO, VIRTIO_GPU_CMD_GET_CAPSET, + VIRTIO_GPU_CMD_GET_EDID, /* 3d commands */ VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, @@ -76,6 +78,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_RESP_OK_DISPLAY_INFO, VIRTIO_GPU_RESP_OK_CAPSET_INFO, VIRTIO_GPU_RESP_OK_CAPSET, + VIRTIO_GPU_RESP_OK_EDID, /* error responses */ VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, @@ -291,6 +294,20 @@ struct virtio_gpu_resp_capset { __u8 capset_data[]; }; +/* VIRTIO_GPU_CMD_GET_EDID */ +struct virtio_gpu_get_edid { + struct virtio_gpu_ctrl_hdr hdr; + __le32 scanout; +}; + +/* VIRTIO_GPU_RESP_OK_EDID */ +struct virtio_gpu_resp_edid { + struct virtio_gpu_ctrl_hdr hdr; + __le32 scanout; + __le32 size; + __u8 edid[1024]; +}; + #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0) struct virtio_gpu_config {
The feature allows the guest request an EDID blob (describing monitor capabilities) for a given scanout (aka virtual monitor connector). It brings a new command message, which has just a scanout field (beside the standard virtio-gpu header) and a response message which carries the EDID data. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/uapi/linux/virtio_gpu.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)