On Tue, 10 Oct 2023, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau <marcandre.lur...@redhat.com>
Drop the "x-pixman" property and use fallback path in such case.
Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
hw/display/sm501.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 0eecd00701..bc5faa19f0 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -438,6 +438,12 @@
#define SM501_HWC_WIDTH 64
#define SM501_HWC_HEIGHT 64
+#ifdef CONFIG_PIXMAN
+#define DEFAULT_X_PIXMAN 7
+#else
+#define DEFAULT_X_PIXMAN 0
+#endif
+
/* SM501 local memory size taken from "linux/drivers/mfd/sm501.c" */
static const uint32_t sm501_mem_local_size[] = {
[0] = 4 * MiB,
@@ -730,7 +736,6 @@ static void sm501_2d_operation(SM501State *s)
switch (cmd) {
case 0: /* BitBlt */
{
- static uint32_t tmp_buf[16384];
unsigned int src_x = (s->twoD_source >> 16) & 0x01FFF;
unsigned int src_y = s->twoD_source & 0xFFFF;
uint32_t src_base = s->twoD_source_base & 0x03FFFFFF;
@@ -828,9 +833,11 @@ static void sm501_2d_operation(SM501State *s)
de = db + (width + (height - 1) * dst_pitch) * bypp;
overlap = (db < se && sb < de);
}
+#ifdef CONFIG_PIXMAN
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));
+ static uint32_t tmp_buf[16384];
uint32_t *tmp = tmp_buf;
if (tmp_stride * sizeof(uint32_t) * height > sizeof(tmp_buf)) {
@@ -860,9 +867,12 @@ static void sm501_2d_operation(SM501State *s)
dst_pitch * bypp / sizeof(uint32_t),
8 * bypp, 8 * bypp, src_x, src_y,
dst_x, dst_y, width, height);
- } else {
+ } else
+#else
+ {
fallback = true;
}
+#endif
if (fallback) {
uint8_t *sp = s->local_mem + src_base;
uint8_t *d = s->local_mem + dst_base;
@@ -894,10 +904,13 @@ static void sm501_2d_operation(SM501State *s)
color = cpu_to_le16(color);
}
+#ifdef CONFIG_PIXMAN
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)) {
+ dst_x, dst_y, width, height, color))
+#endif
+ {
/* fallback when pixman failed or we don't want to call it */
uint8_t *d = s->local_mem + dst_base;
unsigned int x, y, i;
@@ -1875,9 +1888,16 @@ static void sm501_reset(SM501State *s)
s->twoD_wrap = 0;
}
-static void sm501_init(SM501State *s, DeviceState *dev,
- uint32_t local_mem_bytes)
+static bool sm501_init(SM501State *s, DeviceState *dev,
+ uint32_t local_mem_bytes, Error **errp)
{
+#ifndef CONFIG_PIXMAN
+ if (s->use_pixman != 0) {
+ error_setg(errp, "x-pixman!=0, however PIXMAN is not available");
+ return false;
+ }
+#endif
I think this should be just a warning saying that x-pixman != 0 is not
effective without pixman and not a hard error. The goal is to keep command
lines working the same not to annoy users unnecesarily.
+
s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes);
/* local memory */
@@ -1916,6 +1936,7 @@ static void sm501_init(SM501State *s, DeviceState *dev,
/* create qemu graphic console */
s->con = graphic_console_init(dev, 0, &sm501_ops, s);
+ return true;
}
static const VMStateDescription vmstate_sm501_state = {
@@ -2014,7 +2035,9 @@ static void sm501_realize_sysbus(DeviceState *dev, Error
**errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
MemoryRegion *mr;
- sm501_init(&s->state, dev, s->vram_size);
+ if (!sm501_init(&s->state, dev, s->vram_size, errp)) {
+ return;
+ }
if (get_local_mem_size(&s->state) != s->vram_size) {
error_setg(errp, "Invalid VRAM size, nearest valid size is %" PRIu32,
get_local_mem_size(&s->state));
@@ -2038,7 +2061,8 @@ 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),
+ /* TODO: consider PROP_BIT instead */
I don't think a comment is needed here but don't mind it you think it
helps to note this here too but I'd keep the single value instead of
separate BIT properties.
+ DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman,
DEFAULT_X_PIXMAN),
DEFINE_PROP_END_OF_LIST(),
};
@@ -2112,7 +2136,9 @@ static void sm501_realize_pci(PCIDevice *dev, Error
**errp)
{
SM501PCIState *s = PCI_SM501(dev);
- sm501_init(&s->state, DEVICE(dev), s->vram_size);
+ if (!sm501_init(&s->state, DEVICE(dev), s->vram_size, errp)) {
+ return;
+ }
if (get_local_mem_size(&s->state) != s->vram_size) {
error_setg(errp, "Invalid VRAM size, nearest valid size is %" PRIu32,
get_local_mem_size(&s->state));
@@ -2126,7 +2152,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_UINT8("x-pixman", SM501PCIState, state.use_pixman,
DEFAULT_X_PIXMAN),
DEFINE_PROP_END_OF_LIST(),
};
@@ -2169,8 +2195,10 @@ static void sm501_pci_class_init(ObjectClass *klass,
void *data)
static void sm501_pci_init(Object *o)
{
+#ifdef CONFIG_PIXMAN
Do you still need the ifdef here if we keep the property even with
!PIXMAN?
Regards,
BALATON Zoltan
object_property_set_description(o, "x-pixman", "Use pixman for: "
"1: fill, 2: blit, 4: overlap blit");
+#endif
}
static const TypeInfo sm501_pci_info = {
