[RFC PATCH 0/2] Try to smartly allocate memory on the two BARs

[RFC PATCH 0/2] Try to smartly allocate memory on the two BARs

[Frediano Ziglio]

Currently a single type of surface is allocated in a specific BAR.
This also changes from userspace driver to the kernel one.
This way it could happen that allocation are failing even if there are
plenty of space in the other BAR.
For instance this can happen trying to change resolution as the old and
the new virtual screen is supposed to be contained in a single BAR.
The change allows allocation to occur in the BAR not being the default
for a surface type.

The patches prove to be really stable. I tested setting quite small BARs
(one or the oher) or changing default allocation BAR and 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 +++++++----
 3 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] Change the way slot is detected

[Frediano Ziglio]

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..3a1b055 100644
--- a/qxl/qxl_cmd.c
+++ b/qxl/qxl_cmd.c
@@ -511,7 +511,7 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
 	cmd->u.surface_create.height = surf->surf.height;
 	cmd->u.surface_create.stride = surf->surf.stride;
 	if (new_mem) {
-		int slot_id = surf->type == QXL_GEM_DOMAIN_VRAM ? qdev->main_mem_slot : qdev->surfaces_mem_slot;
+		int slot_id = qxl_bo_get_slot_id(qdev, surf);
 		struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
 
 		/* TODO - need to hold one of the locks to read tbo.offset */
diff --git a/qxl/qxl_drv.h b/qxl/qxl_drv.h
index 01a8694..60f0062 100644
--- a/qxl/qxl_drv.h
+++ b/qxl/qxl_drv.h
@@ -365,11 +365,18 @@ qxl_fb_virtual_address(struct qxl_device *qdev, unsigned long physical)
 	return 0;
 }
 
+static inline int
+qxl_bo_get_slot_id(struct qxl_device *qdev, struct qxl_bo *bo)
+{
+	return ((bo->tbo.cur_placement & TTM_PL_MASK_MEM) == TTM_PL_FLAG_VRAM) ?
+		qdev->main_mem_slot : qdev->surfaces_mem_slot;
+}
+
 static inline uint64_t
 qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
 			unsigned long offset)
 {
-	int slot_id = bo->type == QXL_GEM_DOMAIN_VRAM ? qdev->main_mem_slot : qdev->surfaces_mem_slot;
+	int slot_id = qxl_bo_get_slot_id(qdev, bo);
 	struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
 
 	/* TODO - need to hold one of the locks to read tbo.offset */
-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] Allocate objects in both video rams

[Frediano Ziglio]

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_object.c b/qxl/qxl_object.c
index 5e1d789..220de9a 100644
--- a/qxl/qxl_object.c
+++ b/qxl/qxl_object.c
@@ -58,14 +58,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned)
 
 	qbo->placement.placement = qbo->placements;
 	qbo->placement.busy_placement = qbo->placements;
-	if (domain == QXL_GEM_DOMAIN_VRAM)
+	if (domain == QXL_GEM_DOMAIN_VRAM) {
 		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_VRAM | pflag;
-	if (domain == QXL_GEM_DOMAIN_SURFACE)
 		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_PRIV0 | pflag;
-	if (domain == QXL_GEM_DOMAIN_CPU)
+	} else if (domain == QXL_GEM_DOMAIN_SURFACE) {
+		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_PRIV0 | pflag;
+		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_VRAM | pflag;
+	} else if (domain == QXL_GEM_DOMAIN_CPU) {
 		qbo->placements[c++].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM | pflag;
-	if (!c)
+	} else {
 		qbo->placements[c++].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
+	}
 	qbo->placement.num_placement = c;
 	qbo->placement.num_busy_placement = c;
 	for (i = 0; i < c; ++i) {
-- 
2.4.3

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [PATCH 1/2] Change the way slot is detected

[Dave Airlie]

On 12 October 2015 at 23:02, Frediano Ziglio <fziglio@redhat.com> wrote:
> Instead of relaying on surface type use the actual placement.
> This allow to have different placement for a single type of
> surface.

These two look fine, just 2 things,

a) missing Signed-off-by
b) no prefix - please prefix qxl kernel patches with drm/qxl.

Dave.

> ---
>  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..3a1b055 100644
> --- a/qxl/qxl_cmd.c
> +++ b/qxl/qxl_cmd.c
> @@ -511,7 +511,7 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
>         cmd->u.surface_create.height = surf->surf.height;
>         cmd->u.surface_create.stride = surf->surf.stride;
>         if (new_mem) {
> -               int slot_id = surf->type == QXL_GEM_DOMAIN_VRAM ? qdev->main_mem_slot : qdev->surfaces_mem_slot;
> +               int slot_id = qxl_bo_get_slot_id(qdev, surf);
>                 struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
>
>                 /* TODO - need to hold one of the locks to read tbo.offset */
> diff --git a/qxl/qxl_drv.h b/qxl/qxl_drv.h
> index 01a8694..60f0062 100644
> --- a/qxl/qxl_drv.h
> +++ b/qxl/qxl_drv.h
> @@ -365,11 +365,18 @@ qxl_fb_virtual_address(struct qxl_device *qdev, unsigned long physical)
>         return 0;
>  }
>
> +static inline int
> +qxl_bo_get_slot_id(struct qxl_device *qdev, struct qxl_bo *bo)
> +{
> +       return ((bo->tbo.cur_placement & TTM_PL_MASK_MEM) == TTM_PL_FLAG_VRAM) ?
> +               qdev->main_mem_slot : qdev->surfaces_mem_slot;
> +}
> +
>  static inline uint64_t
>  qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
>                         unsigned long offset)
>  {
> -       int slot_id = bo->type == QXL_GEM_DOMAIN_VRAM ? qdev->main_mem_slot : qdev->surfaces_mem_slot;
> +       int slot_id = qxl_bo_get_slot_id(qdev, bo);
>         struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
>
>         /* TODO - need to hold one of the locks to read tbo.offset */
> --
> 2.4.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel