| Message ID | 20230312092244.451465-9-ray.huang@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add VirtIO GPU and Passthrough GPU support on Xen | expand |
On 3/12/23 12:22, Huang Rui wrote: > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > Request Venus when initializing VirGL. > > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> > --- > hw/display/virtio-gpu-virgl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > index fe03dc916f..f5ce206b93 100644 > --- a/hw/display/virtio-gpu-virgl.c > +++ b/hw/display/virtio-gpu-virgl.c > @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) > { > int ret; > > +#ifdef VIRGL_RENDERER_VENUS > + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); > +#else > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > +#endif Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be set. Please test the patches with the latest virglrenderer and etc. The #ifdef also doesn't allow adding new flags, it should look like: #ifdef VIRGL_RENDERER_VENUS flags |= VIRGL_RENDERER_RENDER_SERVER; #endif ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
On 3/12/23 20:51, Dmitry Osipenko wrote: > On 3/12/23 12:22, Huang Rui wrote: >> From: Antonio Caggiano <antonio.caggiano@collabora.com> >> >> Request Venus when initializing VirGL. >> >> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> >> --- >> hw/display/virtio-gpu-virgl.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c >> index fe03dc916f..f5ce206b93 100644 >> --- a/hw/display/virtio-gpu-virgl.c >> +++ b/hw/display/virtio-gpu-virgl.c >> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) >> { >> int ret; >> >> +#ifdef VIRGL_RENDERER_VENUS >> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); >> +#else >> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); >> +#endif > > Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be > set. Please test the patches with the latest virglrenderer and etc. > > The #ifdef also doesn't allow adding new flags, it should look like: > > #ifdef VIRGL_RENDERER_VENUS > flags |= VIRGL_RENDERER_RENDER_SERVER; > #endif > > ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); > BTW, Alex reviewed the Venus v3 patches a month ago [1] and the review comments need to be addressed. AFAICS, you're actually using the very old Venus patches here that stopped working about a year ago, so again you're using a very outdated virglrenderer version. Please take it all into account if you'll beat me to posting the next version of Venus patches ;) [1] https://lore.kernel.org/qemu-devel/20220926142422.22325-1-antonio.caggiano@collabora.com/
On Mon, Mar 13, 2023 at 01:51:03AM +0800, Dmitry Osipenko wrote: > On 3/12/23 12:22, Huang Rui wrote: > > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > > > Request Venus when initializing VirGL. > > > > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> > > --- > > hw/display/virtio-gpu-virgl.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > > index fe03dc916f..f5ce206b93 100644 > > --- a/hw/display/virtio-gpu-virgl.c > > +++ b/hw/display/virtio-gpu-virgl.c > > @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) > > { > > int ret; > > > > +#ifdef VIRGL_RENDERER_VENUS > > + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); > > +#else > > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > > +#endif > > Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be > set. Please test the patches with the latest virglrenderer and etc. > > The #ifdef also doesn't allow adding new flags, it should look like: > > #ifdef VIRGL_RENDERER_VENUS > flags |= VIRGL_RENDERER_RENDER_SERVER; > #endif > > ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); In fact, we have rebased to the latest virglrenderer: We check both VIRGL_RENDERER_RENDER_SERVER or VIRGL_RENDERER_VENUS in virglrenderer, alternative of them works. https://gitlab.freedesktop.org/rui/virglrenderer/-/commit/c1322a8a84379b1ef7939f56c6761b0114716f45 Thanks, Ray
On Mon, Mar 13, 2023 at 10:22:24AM +0800, Dmitry Osipenko wrote: > On 3/12/23 20:51, Dmitry Osipenko wrote: > > On 3/12/23 12:22, Huang Rui wrote: > >> From: Antonio Caggiano <antonio.caggiano@collabora.com> > >> > >> Request Venus when initializing VirGL. > >> > >> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> > >> --- > >> hw/display/virtio-gpu-virgl.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > >> index fe03dc916f..f5ce206b93 100644 > >> --- a/hw/display/virtio-gpu-virgl.c > >> +++ b/hw/display/virtio-gpu-virgl.c > >> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) > >> { > >> int ret; > >> > >> +#ifdef VIRGL_RENDERER_VENUS > >> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); > >> +#else > >> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > >> +#endif > > > > Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be > > set. Please test the patches with the latest virglrenderer and etc. > > > > The #ifdef also doesn't allow adding new flags, it should look like: > > > > #ifdef VIRGL_RENDERER_VENUS > > flags |= VIRGL_RENDERER_RENDER_SERVER; > > #endif > > > > ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); > > > > BTW, Alex reviewed the Venus v3 patches a month ago [1] and the review > comments need to be addressed. AFAICS, you're actually using the very > old Venus patches here that stopped working about a year ago, so again > you're using a very outdated virglrenderer version. > > Please take it all into account if you'll beat me to posting the next > version of Venus patches ;) > > [1] > https://lore.kernel.org/qemu-devel/20220926142422.22325-1-antonio.caggiano@collabora.com/ > Thanks Dmitry point it out, I will use the latest v3 patches, and try to address the comments in next version. :-) Thanks, Ray
On 3/13/23 18:55, Huang Rui wrote: > On Mon, Mar 13, 2023 at 01:51:03AM +0800, Dmitry Osipenko wrote: >> On 3/12/23 12:22, Huang Rui wrote: >>> From: Antonio Caggiano <antonio.caggiano@collabora.com> >>> >>> Request Venus when initializing VirGL. >>> >>> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> >>> --- >>> hw/display/virtio-gpu-virgl.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c >>> index fe03dc916f..f5ce206b93 100644 >>> --- a/hw/display/virtio-gpu-virgl.c >>> +++ b/hw/display/virtio-gpu-virgl.c >>> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) >>> { >>> int ret; >>> >>> +#ifdef VIRGL_RENDERER_VENUS >>> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); >>> +#else >>> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); >>> +#endif >> >> Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be >> set. Please test the patches with the latest virglrenderer and etc. >> >> The #ifdef also doesn't allow adding new flags, it should look like: >> >> #ifdef VIRGL_RENDERER_VENUS >> flags |= VIRGL_RENDERER_RENDER_SERVER; >> #endif >> >> ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); > > In fact, we have rebased to the latest virglrenderer: > > We check both VIRGL_RENDERER_RENDER_SERVER or VIRGL_RENDERER_VENUS in > virglrenderer, alternative of them works. > > https://gitlab.freedesktop.org/rui/virglrenderer/-/commit/c1322a8a84379b1ef7939f56c6761b0114716f45 All the extra changes you made to virglrenderer that Qemu depends on need to go upstream. Please open all the relevant merge requests. Thanks!
On Thu, Mar 16, 2023 at 07:14:47AM +0800, Dmitry Osipenko wrote: > On 3/13/23 18:55, Huang Rui wrote: > > On Mon, Mar 13, 2023 at 01:51:03AM +0800, Dmitry Osipenko wrote: > >> On 3/12/23 12:22, Huang Rui wrote: > >>> From: Antonio Caggiano <antonio.caggiano@collabora.com> > >>> > >>> Request Venus when initializing VirGL. > >>> > >>> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> > >>> --- > >>> hw/display/virtio-gpu-virgl.c | 4 ++++ > >>> 1 file changed, 4 insertions(+) > >>> > >>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > >>> index fe03dc916f..f5ce206b93 100644 > >>> --- a/hw/display/virtio-gpu-virgl.c > >>> +++ b/hw/display/virtio-gpu-virgl.c > >>> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) > >>> { > >>> int ret; > >>> > >>> +#ifdef VIRGL_RENDERER_VENUS > >>> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); > >>> +#else > >>> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > >>> +#endif > >> > >> Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be > >> set. Please test the patches with the latest virglrenderer and etc. > >> > >> The #ifdef also doesn't allow adding new flags, it should look like: > >> > >> #ifdef VIRGL_RENDERER_VENUS > >> flags |= VIRGL_RENDERER_RENDER_SERVER; > >> #endif > >> > >> ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); > > > > In fact, we have rebased to the latest virglrenderer: > > > > We check both VIRGL_RENDERER_RENDER_SERVER or VIRGL_RENDERER_VENUS in > > virglrenderer, alternative of them works. > > > > https://gitlab.freedesktop.org/rui/virglrenderer/-/commit/c1322a8a84379b1ef7939f56c6761b0114716f45 > > All the extra changes you made to virglrenderer that Qemu depends on > need to go upstream. Please open all the relevant merge requests. Thanks! > Dmitry, sorry to late response, I have created relevant merge requests below: Virglrenderer: https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1068 Mesa: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22108 I'd appreciate any comments. :-) Thanks, Ray
On 3/24/23 16:22, Huang Rui wrote: > On Thu, Mar 16, 2023 at 07:14:47AM +0800, Dmitry Osipenko wrote: >> On 3/13/23 18:55, Huang Rui wrote: >>> On Mon, Mar 13, 2023 at 01:51:03AM +0800, Dmitry Osipenko wrote: >>>> On 3/12/23 12:22, Huang Rui wrote: >>>>> From: Antonio Caggiano <antonio.caggiano@collabora.com> >>>>> >>>>> Request Venus when initializing VirGL. >>>>> >>>>> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> >>>>> --- >>>>> hw/display/virtio-gpu-virgl.c | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c >>>>> index fe03dc916f..f5ce206b93 100644 >>>>> --- a/hw/display/virtio-gpu-virgl.c >>>>> +++ b/hw/display/virtio-gpu-virgl.c >>>>> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) >>>>> { >>>>> int ret; >>>>> >>>>> +#ifdef VIRGL_RENDERER_VENUS >>>>> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); >>>>> +#else >>>>> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); >>>>> +#endif >>>> >>>> Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be >>>> set. Please test the patches with the latest virglrenderer and etc. >>>> >>>> The #ifdef also doesn't allow adding new flags, it should look like: >>>> >>>> #ifdef VIRGL_RENDERER_VENUS >>>> flags |= VIRGL_RENDERER_RENDER_SERVER; >>>> #endif >>>> >>>> ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); >>> >>> In fact, we have rebased to the latest virglrenderer: >>> >>> We check both VIRGL_RENDERER_RENDER_SERVER or VIRGL_RENDERER_VENUS in >>> virglrenderer, alternative of them works. >>> >>> https://gitlab.freedesktop.org/rui/virglrenderer/-/commit/c1322a8a84379b1ef7939f56c6761b0114716f45 >> >> All the extra changes you made to virglrenderer that Qemu depends on >> need to go upstream. Please open all the relevant merge requests. Thanks! >> > > Dmitry, sorry to late response, I have created relevant merge requests > below: > > Virglrenderer: > https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1068 > > Mesa: > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22108 > > I'd appreciate any comments. :-) Thanks, Ray. I'll try to get to the patches soon.
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index fe03dc916f..f5ce206b93 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) { int ret; +#ifdef VIRGL_RENDERER_VENUS + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs); +#else ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); +#endif if (ret != 0) { error_report("virgl could not be initialized: %d", ret); return ret;