diff mbox series

[v9,1/7] hw/display/sm501: Add debug property to control pixman usage

Message ID 61768ffaefa71b65a657d1365823bd43c7ee9354.1678188711.git.balaton@eik.bme.hu
State New
Headers show
Series Pegasos2 fixes and audio output support | expand

Commit Message

BALATON Zoltan March 7, 2023, 11:42 a.m. UTC 
Add a property to allow disabling pixman and always use the fallbacks
for different operations which is useful for testing different drawing
methods or debugging pixman related issues.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
 hw/display/sm501.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Philippe Mathieu-Daudé March 7, 2023, 3:11 p.m. UTC | #1 
On 7/3/23 12:42, BALATON Zoltan wrote:
> Add a property to allow disabling pixman and always use the fallbacks
> for different operations which is useful for testing different drawing
> methods or debugging pixman related issues.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Tested-by: Rene Engel <ReneEngel80@emailn.de>
> ---
>   hw/display/sm501.c | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)


>   static Property sm501_sysbus_properties[] = {
>       DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
> +    DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman, 7),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -2122,6 +2126,7 @@ static void sm501_realize_pci(PCIDevice *dev, Error **errp)
>   
>   static Property sm501_pci_properties[] = {
>       DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size, 64 * MiB),
> +    DEFINE_PROP_UINT8("x-pixman", SM501PCIState, state.use_pixman, 7),

FYI, alternatively:

DEFINE_PROP_BIT("x-pixman-fill", SM501PCIState, state.use_pixman, 0, 1),
DEFINE_PROP_BIT("x-pixman-blit", SM501PCIState, state.use_pixman, 1, 1),
DEFINE_PROP_BIT("x-pixman-overlap-blit", SM501PCIState,
                 state.use_pixman, 2, 1),

>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -2162,11 +2167,18 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data)
>       dc->vmsd = &vmstate_sm501_pci;
>   }
>   
> +static void sm501_pci_init(Object *o)
> +{
> +    object_property_set_description(o, "x-pixman", "Use pixman for: "
> +                                    "1: fill, 2: blit, 4: overlap blit");
> +}
BALATON Zoltan March 7, 2023, 3:15 p.m. UTC | #2 
On Tue, 7 Mar 2023, Philippe Mathieu-Daudé wrote:
> On 7/3/23 12:42, BALATON Zoltan wrote:
>> Add a property to allow disabling pixman and always use the fallbacks
>> for different operations which is useful for testing different drawing
>> methods or debugging pixman related issues.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>> ---
>>   hw/display/sm501.c | 18 +++++++++++++++---
>>   1 file changed, 15 insertions(+), 3 deletions(-)
>
>
>>   static Property sm501_sysbus_properties[] = {
>>       DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
>> +    DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman, 7),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   @@ -2122,6 +2126,7 @@ static void sm501_realize_pci(PCIDevice *dev, Error 
>> **errp)
>>     static Property sm501_pci_properties[] = {
>>       DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size, 64 * MiB),
>> +    DEFINE_PROP_UINT8("x-pixman", SM501PCIState, state.use_pixman, 7),
>
> FYI, alternatively:
>
> DEFINE_PROP_BIT("x-pixman-fill", SM501PCIState, state.use_pixman, 0, 1),
> DEFINE_PROP_BIT("x-pixman-blit", SM501PCIState, state.use_pixman, 1, 1),
> DEFINE_PROP_BIT("x-pixman-overlap-blit", SM501PCIState,
>                state.use_pixman, 2, 1),

Hmm, interesting but this would result in harder to use longer command 
lines. In this case when for a debug property expected to be used by 
experienced users a single value is easier to use.

Regards,
BALATON Zoltan

>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   @@ -2162,11 +2167,18 @@ static void sm501_pci_class_init(ObjectClass 
>> *klass, void *data)
>>       dc->vmsd = &vmstate_sm501_pci;
>>   }
>>   +static void sm501_pci_init(Object *o)
>> +{
>> +    object_property_set_description(o, "x-pixman", "Use pixman for: "
>> +                                    "1: fill, 2: blit, 4: overlap blit");
>> +}
>
>
>
diff mbox series

Patch

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 17835159fc..dbabbc4339 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -465,6 +465,7 @@  typedef struct SM501State {
     uint32_t last_width;
     uint32_t last_height;
     bool do_full_update; /* perform a full update next time */
+    uint8_t use_pixman;
     I2CBus *i2c_bus;
 
     /* mmio registers */
@@ -827,7 +828,7 @@  static void sm501_2d_operation(SM501State *s)
                 de = db + (width + (height - 1) * dst_pitch) * bypp;
                 overlap = (db < se && sb < de);
             }
-            if (overlap) {
+            if (overlap && (s->use_pixman & BIT(2))) {
                 /* pixman can't do reverse blit: copy via temporary */
                 int tmp_stride = DIV_ROUND_UP(width * bypp, sizeof(uint32_t));
                 uint32_t *tmp = tmp_buf;
@@ -852,13 +853,15 @@  static void sm501_2d_operation(SM501State *s)
                 if (tmp != tmp_buf) {
                     g_free(tmp);
                 }
-            } else {
+            } else if (!overlap && (s->use_pixman & BIT(1))) {
                 fallback = !pixman_blt((uint32_t *)&s->local_mem[src_base],
                                        (uint32_t *)&s->local_mem[dst_base],
                                        src_pitch * bypp / sizeof(uint32_t),
                                        dst_pitch * bypp / sizeof(uint32_t),
                                        8 * bypp, 8 * bypp, src_x, src_y,
                                        dst_x, dst_y, width, height);
+            } else {
+                fallback = true;
             }
             if (fallback) {
                 uint8_t *sp = s->local_mem + src_base;
@@ -891,7 +894,7 @@  static void sm501_2d_operation(SM501State *s)
             color = cpu_to_le16(color);
         }
 
-        if ((width == 1 && height == 1) ||
+        if (!(s->use_pixman & BIT(0)) || (width == 1 && height == 1) ||
             !pixman_fill((uint32_t *)&s->local_mem[dst_base],
                          dst_pitch * bypp / sizeof(uint32_t), 8 * bypp,
                          dst_x, dst_y, width, height, color)) {
@@ -2035,6 +2038,7 @@  static void sm501_realize_sysbus(DeviceState *dev, Error **errp)
 
 static Property sm501_sysbus_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
+    DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman, 7),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2122,6 +2126,7 @@  static void sm501_realize_pci(PCIDevice *dev, Error **errp)
 
 static Property sm501_pci_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size, 64 * MiB),
+    DEFINE_PROP_UINT8("x-pixman", SM501PCIState, state.use_pixman, 7),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2162,11 +2167,18 @@  static void sm501_pci_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_sm501_pci;
 }
 
+static void sm501_pci_init(Object *o)
+{
+    object_property_set_description(o, "x-pixman", "Use pixman for: "
+                                    "1: fill, 2: blit, 4: overlap blit");
+}
+
 static const TypeInfo sm501_pci_info = {
     .name          = TYPE_PCI_SM501,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(SM501PCIState),
     .class_init    = sm501_pci_class_init,
+    .instance_init = sm501_pci_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },