mbox series

[v3,0/2] vfio: add vgpu edid support

Message ID 20180921083013.15028-1-kraxel@redhat.com (mailing list archive)
Headers show
Series vfio: add vgpu edid support | expand

Message

Gerd Hoffmann Sept. 21, 2018, 8:30 a.m. UTC
Update vfio api, adapt mbochs sample driver.

v3: refine vfio region layout, improve documentation.
v2: try vfio region approach.

Gerd Hoffmann (2):
  vfio: add edid api for display (vgpu) devices.
  vfio: add edid support to mbochs sample driver

 include/uapi/linux/vfio.h  |  50 +++++++++++++++++
 samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 167 insertions(+), 19 deletions(-)

Comments

Alex Williamson Sept. 25, 2018, 6:23 p.m. UTC | #1
On Fri, 21 Sep 2018 10:30:11 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Update vfio api, adapt mbochs sample driver.
> 
> v3: refine vfio region layout, improve documentation.
> v2: try vfio region approach.
> 
> Gerd Hoffmann (2):
>   vfio: add edid api for display (vgpu) devices.
>   vfio: add edid support to mbochs sample driver
> 
>  include/uapi/linux/vfio.h  |  50 +++++++++++++++++
>  samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 167 insertions(+), 19 deletions(-)
> 

This looks good to me, but I'm not clear if the discussion about pixel
clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
within this interface?  Any comments from NVIDIA?  Thanks,

Alex
Kirti Wankhede Sept. 27, 2018, 7:48 p.m. UTC | #2
On 9/25/2018 11:53 PM, Alex Williamson wrote:
> On Fri, 21 Sep 2018 10:30:11 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>> Update vfio api, adapt mbochs sample driver.
>>
>> v3: refine vfio region layout, improve documentation.
>> v2: try vfio region approach.
>>
>> Gerd Hoffmann (2):
>>   vfio: add edid api for display (vgpu) devices.
>>   vfio: add edid support to mbochs sample driver
>>
>>  include/uapi/linux/vfio.h  |  50 +++++++++++++++++
>>  samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 167 insertions(+), 19 deletions(-)
>>
> 
> This looks good to me, but I'm not clear if the discussion about pixel
> clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
> within this interface?  Any comments from NVIDIA?  Thanks,
> 

As of now, default edid with basic modes is provided with NVIDIA driver,
so don't have such requirement.
As I understand, if VFIO_REGION_SUBTYPE_GFX_EDID region is not provided
by vendor driver, console vnc should work as it is now.

Thanks,
Kirti
Gerd Hoffmann Sept. 28, 2018, 5:47 a.m. UTC | #3
Hi,

> > This looks good to me, but I'm not clear if the discussion about pixel
> > clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
> > within this interface?  Any comments from NVIDIA?  Thanks,
> 
> As of now, default edid with basic modes is provided with NVIDIA driver,
> so don't have such requirement.

The advantage would be that the default mode can be changed, so the host
can give a hint to the guest which of the video modes should be used.

The qemu test branch[1] adds xres and yres properties as showcase, i.e.
you can use -device vfio-pci,xres=1280,yres=900,...

> As I understand, if VFIO_REGION_SUBTYPE_GFX_EDID region is not provided
> by vendor driver, console vnc should work as it is now.

Yes, the region is optional.

cheers,
  Gerd

[1] https://git.kraxel.org/cgit/qemu/log/?h=sirius/edid-vfio
Kirti Wankhede Sept. 28, 2018, 8:43 a.m. UTC | #4
On 9/28/2018 11:17 AM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> This looks good to me, but I'm not clear if the discussion about pixel
>>> clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
>>> within this interface?  Any comments from NVIDIA?  Thanks,
>>
>> As of now, default edid with basic modes is provided with NVIDIA driver,
>> so don't have such requirement.
> 
> The advantage would be that the default mode can be changed, so the host
> can give a hint to the guest which of the video modes should be used.
> 
> The qemu test branch[1] adds xres and yres properties as showcase, i.e.
> you can use -device vfio-pci,xres=1280,yres=900,...
> 
>> As I understand, if VFIO_REGION_SUBTYPE_GFX_EDID region is not provided
>> by vendor driver, console vnc should work as it is now.
> 
> Yes, the region is optional.
> 

Ok.
Looks good to me. Just add comment as I mentioned in other mail.

Thanks,
Kirti

> cheers,
>   Gerd
> 
> [1] https://git.kraxel.org/cgit/qemu/log/?h=sirius/edid-vfio
>
Zhenyu Wang Oct. 10, 2018, 9:05 a.m. UTC | #5
On 2018.09.25 12:23:56 -0600, Alex Williamson wrote:
> On Fri, 21 Sep 2018 10:30:11 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > Update vfio api, adapt mbochs sample driver.
> > 
> > v3: refine vfio region layout, improve documentation.
> > v2: try vfio region approach.
> > 
> > Gerd Hoffmann (2):
> >   vfio: add edid api for display (vgpu) devices.
> >   vfio: add edid support to mbochs sample driver
> > 
> >  include/uapi/linux/vfio.h  |  50 +++++++++++++++++
> >  samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 167 insertions(+), 19 deletions(-)
> > 
> 
> This looks good to me, but I'm not clear if the discussion about pixel
> clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
> within this interface?  Any comments from NVIDIA?  Thanks,
> 

For current vfio gfx, I think this is fine. For really limited HW
that might have clock gap, we could skip edid region support on it.

I'm not sure if we'd like to extend it for multiple display support?
As it seems edid blob definition is self-contained, maybe we'll only
have some region index reference added?

thanks
Gerd Hoffmann Oct. 10, 2018, 10:41 a.m. UTC | #6
On Wed, Oct 10, 2018 at 05:05:26PM +0800, Zhenyu Wang wrote:
> On 2018.09.25 12:23:56 -0600, Alex Williamson wrote:
> > On Fri, 21 Sep 2018 10:30:11 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> > 
> > > Update vfio api, adapt mbochs sample driver.
> > > 
> > > v3: refine vfio region layout, improve documentation.
> > > v2: try vfio region approach.
> > > 
> > > Gerd Hoffmann (2):
> > >   vfio: add edid api for display (vgpu) devices.
> > >   vfio: add edid support to mbochs sample driver
> > > 
> > >  include/uapi/linux/vfio.h  |  50 +++++++++++++++++
> > >  samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
> > >  2 files changed, 167 insertions(+), 19 deletions(-)
> > > 
> > 
> > This looks good to me, but I'm not clear if the discussion about pixel
> > clocks fully ran its course.  Zhenyu, are you satisfied GVT-g can work
> > within this interface?  Any comments from NVIDIA?  Thanks,
> > 
> 
> For current vfio gfx, I think this is fine. For really limited HW
> that might have clock gap, we could skip edid region support on it.
> 
> I'm not sure if we'd like to extend it for multiple display support?
> As it seems edid blob definition is self-contained, maybe we'll only
> have some region index reference added?

We do not have multiple display support in the vfio display api anyway.

cheers,
  Gerd
Alex Williamson Oct. 11, 2018, 7:01 p.m. UTC | #7
On Fri, 21 Sep 2018 10:30:11 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Update vfio api, adapt mbochs sample driver.
> 
> v3: refine vfio region layout, improve documentation.
> v2: try vfio region approach.
> 
> Gerd Hoffmann (2):
>   vfio: add edid api for display (vgpu) devices.
>   vfio: add edid support to mbochs sample driver
> 
>  include/uapi/linux/vfio.h  |  50 +++++++++++++++++
>  samples/vfio-mdev/mbochs.c | 136 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 167 insertions(+), 19 deletions(-)
> 

I've pushed this to my next branch for v4.20.  Thanks,

Alex