>
> Add a helper functions to check video modes. Also add a helper to check
> framebuffer buffer objects, using the former for consistency. That way
> we should not fail in qxl_primary_atomic_check() because video modes
> which are too big will not be added to the mode list in the first place.
>
>
> Move final cleanups to qxl_drm_release() callback.
Can you explain in the commit why this is better or preferable?
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
I suppose this is to replace the former manual cleanup calls,
which were moved to qxl_drm_release, I think this cou
>
> The ttm_mem_io_* functions are actually internal to TTM and shouldn't be
> used in a driver.
>
As far as I can see by your second patch QXL is just using exported
(that is not internal) functions.
Not that the idea of making them internal is bad but this comment is
a wrong statement.
> Inst
>
> Am 27.09.19 um 18:31 schrieb Frediano Ziglio:
> >> The ttm_mem_io_* functions are actually internal to TTM and shouldn't be
> >> used in a driver.
> >>
> > As far as I can see by your second patch QXL is just using exported
> > (that is not in
>
> On 05.09.19 15:34, Jaak Ristioja wrote:
> > On 05.09.19 10:14, Gerd Hoffmann wrote:
> >> On Tue, Aug 06, 2019 at 09:00:10PM +0300, Jaak Ristioja wrote:
> >>> Hello!
> >>>
> >>> I'm writing to report a crash in the QXL / DRM code in the Linux kernel.
> >>> I originally filed the issue on Launch
> Am 30.09.19 um 11:51 schrieb Frediano Ziglio:
> >> Am 27.09.19 um 18:31 schrieb Frediano Ziglio:
> >>>> The ttm_mem_io_* functions are actually internal to TTM and shouldn't be
> >>>> used in a driver.
> >>>>
> >>> As fa
>
> On Tue, 6 Aug 2019 21:00:10 +0300 From: Jaak Ristioja
> > Hello!
> >
> > I'm writing to report a crash in the QXL / DRM code in the Linux kernel.
> > I originally filed the issue on LaunchPad and more details can be found
> > there, although I doubt whether these details are useful.
> >
>
Skip this for secondary qxl cards which don't have vga mode in the
> first place (Frediano).
>
> Cc: Frediano Ziglio
> Signed-off-by: Gerd Hoffmann
Acked-by: Frediano Ziglio
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 20 +++-
> 1 file changed, 19 in
>
> qxl has two modes: "native" (used by the drm driver) and "vga" (vga
> compatibility mode, typically used for boot display and firmware
> framebuffers).
>
> Accessing any vga ioport will switch the qxl device into vga mode.
> The qxl driver never does that, but other drivers accessing vga port
>
> It's always returning 0, and it's always ignored.
Missing the Signed-off-by.
Acked-by: Frediano Ziglio
You should add the Acked-by message when posting new
versions of the patch series.
Frediano
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
> drivers/gpu/drm/q
>
> When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG
> interrupt,
> we currently always notify userspace that there was some hotplug event.
>
> However, gnome-shell/mutter is reacting to this event by attempting a
> resolution change, which it does by issueing drmModeRmFB, drmM
y_copy_rom_client_monitors_config(qdev)) {
> + int status;
> +
> + status = qxl_display_copy_rom_client_monitors_config(qdev);
> + while (status == MONITORS_CONFIG_BAD_CRC) {
> qxl_io_log(qdev, "failed crc check for client_monitors_config,"
>" retrying\n");
> + status = qxl_display_copy_rom_client_monitors_config(qdev);
> + }
> + if (status == MONITORS_CONFIG_UNCHANGED) {
> + qxl_io_log(qdev, "config unchanged\n");
> + DRM_DEBUG("ignoring unchanged client monitors config");
> + return;
> }
>
> drm_modeset_lock_all(dev);
Acked-by: Frediano Ziglio
Frediano
>
> They are not used outside of their respective source file
>
> Signed-off-by: Christophe Fergeau
Acked-by: Frediano Ziglio
> ---
> drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
> drivers/gpu/drm/qxl/qxl_drv.h | 3 -
>
> The message has to be terminated by a newline as it's not going to get
> added automatically.
>
> Signed-off-by: Christophe Fergeau
Acked-by: Frediano Ziglio
> ---
> drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> qxl_crtc_set_from_monitors_config() is defined in qxl_drv.h but never
> implemented.
>
> Signed-off-by: Christophe Fergeau
Acked-by: Frediano Ziglio
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/g
>
> The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
> the resolutions we are going to present to user-space are going to be
> rounded down to a multiple of 8. In the QXL arbitrary resolution case,
> this is not useful.
> This commit forces the actual width/height that was r
>
> qdev->gem.objects was initialized directly in qxl_device_init() rather
> than going through qxl_gem_init(), and qxl_gem_fini() was never called.
>
Considering "qxl_gem_fini() was never called" did we have a leak?
> Signed-off-by: Christophe Fergeau
> ---
> drivers/gpu/drm/qxl/qxl_kms.c |
sually I would write something like
for (;;) {
status = qxl_display_copy_rom_client_monitors_config(qdev);
if (status != MONITORS_CONFIG_BAD_CRC)
break;
qxl_io_log(qdev, "failed crc check for client_monitors_conf
CPU usage on Qemu side too.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_cmd.c | 12 +++-
qxl/qxl_drv.h | 2 +-
qxl/qxl_ioctl.c | 2 +-
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drivers/gpu/drm/qxl/qxl_cmd.c b/qxl/qxl_cmd.c
index 9782364..bd5404e
>
> Hey,
>
> On Tue, May 19, 2015 at 05:54:54AM -0400, Frediano Ziglio wrote:
> > This problem happens using KMS surfaces and QXL driver.
> > To easy reproduce use KDE Plasma (which use surfaces a lot) and assure
> > you are using KMS surfaces (QXL driver o
This set of patches mainly contains fix for some memory issues
using quite aggressively surfaces and other minor problems like
images going black after a while.
Frediano Ziglio (11):
Do not cause spice-server to clean our objects
Do not leak memory if qxl_release_list_add fails
Fix print
If objects are moved back from system memory to VRAM (and spice id
created again) memory is already initialized so we need to set flag
to not clear memory.
If you don't do it after a while using desktop many images turns to
black or transparents.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_
If the function fails reference counter to the object is not decremented
causing leaks.
This is hard to spot as it happens only on very low memory situations.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_ioctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/qxl
reloc_info[i] is not still initialized in the print statement.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qxl/qxl_ioctl.c b/qxl/qxl_ioctl.c
index e8b5207..230ab84 100644
--- a/qxl/qxl_ioctl.c
+++ b/qxl/qxl_ioctl.c
Is we are not able to get source bo object from handle we free
destination bo object and call cleanup code however destination
object was already inserted in reloc_info array (num_relocs was
already incremented) so on cleanup we free destination again.
Signed-off-by: Frediano Ziglio
---
qxl
Only EBUSY error was handled. This could cause code to believe
reserve was successful while it failed.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_cmd.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/qxl/qxl_cmd.c b/qxl/qxl_cmd.c
index 85ed5dc..b18f84c 100644
--- a
This function return handle to allocated release object which is an int.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_release.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qxl/qxl_release.c b/qxl/qxl_release.c
index d9b2568..6fd8e50 100644
--- a/qxl/qxl_release.c
+++ b/qxl
Free resources correctly if function fails
Signed-off-by: Frediano Ziglio
---
qxl/qxl_release.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/qxl/qxl_release.c b/qxl/qxl_release.c
index 6fd8e50..00604ed 100644
--- a/qxl/qxl_release.c
+++ b/qxl/qxl_release.c
Enable format string checks for qxl_io_log and remove resulting warnings
which could lead to memory errors on different platform or just printing
wrong information.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_cmd.c | 2 +-
qxl/qxl_display.c | 2 +-
qxl/qxl_drv.h | 2 +-
qxl/qxl_release.c
moving main counter (the one used by qxl_bo_(un)ref)
to GEM object which cleanup routine (qxl_gem_object_free) remove the idr pointer
(using qxl_surface_evict) when the counters are still valid.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_gem.c| 10 --
qxl/qxl_object.c | 11 ---
2
In qxlhw_handle_to_bo we incremented counters twice, one time for release object
and one for reloc_info.
In the main function however reloc_info references was drop much earlier than
release so keeping the pointer only on release is safe and make cleaning
process easier.
Signed-off-by: Frediano
assuming EINVAL.
Signed-off-by: Frediano Ziglio
---
qxl/qxl_ioctl.c | 33 ++---
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/qxl/qxl_ioctl.c b/qxl/qxl_ioctl.c
index bb326ff..37f1faf 100644
--- a/qxl/qxl_ioctl.c
+++ b/qxl/qxl_ioctl.c
@@ -107,9 +107,9
>
> On Wed, May 27, 2015 at 8:47 AM, Josh Boyer
> wrote:
> > On Wed, May 27, 2015 at 6:03 AM, Frediano Ziglio
> > wrote:
> >> This set of patches mainly contains fix for some memory issues
> >> using quite aggressively surfaces and other minor problems lik
>
> > I was using a different repository with only QXL driver. I tested and all
> > patches apply and compile perfectly even with Linus master branch.
>
> Lets only post patches people can apply, it makes it harder to figure
> out stuff. I'll take a look at the patches, but it would be good to
>
> On 27 May 2015 at 20:04, Frediano Ziglio wrote:
> > qxl_bo structure has two reference counters, one in the GEM object and
> > another in the TTM object. The GEM object keep a counter to the TTM object
> > so when GEM counter reached zero the TTM counter (us
>
> The size calculation can overflow. I don't know if this leads to
> memory corruption, but it causes a static checker warning.
>
> Signed-off-by: Dan Carpenter
> ---
> v2: I don't know think the size is capped anywhere. In my first version
> of this patch, I introduced a divide by zero bug.
Instead of using container_of directly use to_qxl_bo macro.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm
These two patches fixes two lock dependencies issues.
Frediano Ziglio (2):
drm/qxl: use to_qxl_bo macro
drm/qxl: avoid dependency lock
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 4 +---
drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++--
3 files changed, 4
Instead of using container_of directly use to_qxl_bo macro.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm
all them and the possible deadlock.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_release.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index b66ec33..4efa8e2 100644
--- a/drivers/gpu
These two patches fixes two lock dependencies issues.
Changes from v1: Sent a wrong patch.
Frediano Ziglio (2):
drm/qxl: avoid buffer reservation in qxl_crtc_page_flip
drm/qxl: avoid dependency lock
drivers/gpu/drm/qxl/qxl_display.c | 10 +-
drivers/gpu/drm/qxl/qxl_release.c | 4
problem.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_display.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index a8dbb3e..656752d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
all them and the possible deadlock.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_release.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index b66ec33..4efa8e2 100644
--- a/drivers/gpu
> qxl surfaces (used for framebuffers and gem objects) can live in both
> VRAM and PRIV ttm domains. Update placement setup to include both. Put
> PRIV first in the list so it is preferred, so VRAM will have more room
> for objects which must be allocated there.
>
> Signed-off-by: Gerd Hoffmann
>
> Just use qxl_num_crtc directly everywhere instead of using
> qdev->monitors_config->max_allowed. Drops pointless indirection
> and also is less confusing.
>
To me is MORE confusing, why comparing number of something with
another number? Previously code was comparing number of monitors
with
>
> On Thu, Dec 06, 2018 at 05:59:25AM -0500, Frediano Ziglio wrote:
> > >
> > > Just use qxl_num_crtc directly everywhere instead of using
> > > qdev->monitors_config->max_allowed. Drops pointless indirection
> > > and also is less confusi
>
> On Thu, Dec 06, 2018 at 07:53:10AM -0500, Frediano Ziglio wrote:
> > >
> > > On Thu, Dec 06, 2018 at 05:59:25AM -0500, Frediano Ziglio wrote:
> > > > >
> > > > > Just use qxl_num_crtc directly everywhere instead of using
> >
>
> On Thu, Dec 06, 2018 at 05:55:58AM -0500, Frediano Ziglio wrote:
> >
> > > qxl surfaces (used for framebuffers and gem objects) can live in both
> > > VRAM and PRIV ttm domains. Update placement setup to include both. Put
> > > PRIV first in the lis
Il giorno mer 15 nov 2023 alle ore 06:57 Sasha Levin
ha scritto:
>
> From: Zongmin Zhou
>
> [ Upstream commit 0e8b9f258baed25f1c5672613699247c76b007b5 ]
>
> The allocated memory for qdev->dumb_heads should be released
> in qxl_destroy_monitors_object before qxl suspend.
> otherwise,qxl_create_mon
>
> From: Dave Airlie
>
> QXL hw can't change primary surfaces easily, the server sends a msg
> and the client flickers a lo when it does. The old pre-atomic
typo: lo -> lot
> page flip code endeavoured to workaround this with a copy operation.
>
not clear. Do you mean a memcpy like operatio
>
> From: Dave Airlie
>
> QXL hw can't change primary surfaces easily, the server sends a msg
> and the client flickers a lo when it does. The old pre-atomic
> page flip code endeavoured to workaround this with a copy operation.
>
> This worked by another accident of how the qxl virtual gpu is
continued working.
Setting large resolution is working fine while without these patches fails
for not so big BAR sizes.
Frediano Ziglio (2):
Change the way slot is detected
Allocate objects in both video rams
qxl/qxl_cmd.c| 2 +-
qxl/qxl_drv.h| 9 -
qxl/qxl_object.c | 11
Instead of relaying on surface type use the actual placement.
This allow to have different placement for a single type of
surface.
---
qxl/qxl_cmd.c | 2 +-
qxl/qxl_drv.h | 9 -
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/qxl/qxl_cmd.c b/qxl/qxl_cmd.c
index fdc1833..3a1b
If memory is not enough in the default BAR for a type try other BAR
this allow better memory usage and avoid memory allocation failure
if a BAR is quite small and other is quite unused.
---
qxl/qxl_object.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/qxl/qxl_obj
Is we are not able to get source bo object from handle we free
destination bo object and call cleanup code however destination
object was already inserted in reloc_info array (num_relocs was
already incremented) so on cleanup we free destination again.
Signed-off-by: Frediano Ziglio
Reviewed-by
In qxlhw_handle_to_bo we incremented counters twice, one time for release object
and one for reloc_info.
In the main function however reloc_info references was drop much earlier than
release so keeping the pointer only on release is safe and make cleaning
process easier.
Signed-off-by: Frediano
If objects are moved back from system memory to VRAM (and spice id
created again) memory is already initialized so we need to set flag
to not clear memory.
If you don't do it after a while using desktop many images turns to
black or transparents.
Signed-off-by: Frediano Ziglio
Reviewed-by:
If the function fails reference counter to the object is not decremented
causing leaks.
This is hard to spot as it happens only on very low memory situations.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +++-
1 file changed, 3 insertions
Free resources correctly if function fails
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_release.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index
assuming EINVAL.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 33 ++---
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index bb326ff
errors" should just print garbage and debugging is disabled by default, not
necessary.
Frediano Ziglio (11):
drm/qxl: Do not cause spice-server to clean our objects
drm/qxl: Do not leak memory if qxl_release_list_add fails
drm/qxl: Fix print statement not using uninitialize
reloc_info[i] is not still initialized in the print statement.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index
Only EBUSY error was handled. This could cause code to believe
reserve was successful while it failed.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_cmd.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl
This function return handle to allocated release object which is an int.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl
Enable format string checks for qxl_io_log and remove resulting warnings
which could lead to memory errors on different platform or just printing
wrong information.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl
moving main counter (the one used by qxl_bo_(un)ref)
to GEM object which cleanup routine (qxl_gem_object_free) remove the idr pointer
(using qxl_surface_evict) when the counters are still valid.
Signed-off-by: Frediano Ziglio
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/qxl/qxl_gem.c| 10
continued working.
Setting large resolution is working fine while without these patches fails
for not so big BAR sizes.
Changes from previous version:
- remove RFC;
- added signed-off-by;
- added prefix in commit title;
- rebased.
Frediano Ziglio (2):
drm/qxl: change the way slot is detected
drm
Instead of relaying on surface type use the actual placement.
This allow to have different placement for a single type of
surface.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_drv.h | 9 -
2 files changed, 9 insertions(+), 2 deletions
If memory is not enough in the default BAR for a type try other BAR
this allow better memory usage and avoid memory allocation failure
if a BAR is quite small and other is quite unused.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_object.c | 11 +++
1 file changed, 7
then 4 bytes (which
I don't think is supported under Linux).
However I agree with the patch.
Acked!
Frediano Ziglio
> The size calculation can overflow.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c
> b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 552dc06..5da9a60 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
> @@ -401,6 +401,8 @@ static
>
> This size calculation can overflow on 32 bit systems leading to memory
> corruption.
>
> Reported-by: Ilja Van Sprundel
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c
> b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index bda5c5f..eda6f30 100644
> --- a/drivers/gpu/dr
72 matches
Mail list logo
