Skip to contentSkip to conversation

Misc sm501 improvements

PATCH In review BALATON Zoltan 3 participants 9 messages Jul 4, 2018

lore
Share & tools
lore

BALATON Zoltan

author
#0 lore raw

v3 cover Misc sm501 improvements

Version 3 with changes to 1/7 suggested by Philippe Mathieu-Daudé. Hopefully this haven't missed 3.0 yet.

BALATON Zoltan (3):
sm501: Implement i2c part for reading monitor EDID
sm501: Fix support for non-zero frame buffer start address
sm501: Set updated region dirty after 2D operation

Sebastian Bauer (4):
sm501: Perform a full update after palette change
sm501: Use values from the pitch register for 2D operations
sm501: Implement negated destination raster operation mode
sm501: Log unimplemented raster operation modes

 default-configs/ppc-softmmu.mak    |   1 +
 default-configs/ppcemb-softmmu.mak |   1 +
 default-configs/sh4-softmmu.mak    |   2 +
 default-configs/sh4eb-softmmu.mak  |   2 +
 hw/display/sm501.c                 | 230 +++++++++++++++++++++++++++++++++++--
 5 files changed, 224 insertions(+), 12 deletions(-)
Signature · 1 line
2.7.6
#1 lore raw

PATCH v3 2/7 sm501: Perform a full update after palette change

From: Sebastian Bauer <[email protected]>

Changing the palette of a color index has as an immediate effect on all pixels with the corresponding index on real hardware. Performing a full update after a palette change is a simple way to emulate this effect.

Signed-off-by Sebastian Bauer <[email protected]>
Signed-off-by BALATON Zoltan <[email protected]>

---
v2: change type to bool

 hw/display/sm501.c | 8 ++++++++
 1 file changed, 8 insertions(+)
hw/display/sm501.c +8 -0
index 05c964b..b095134 100644
@@ -487,6 +487,7 @@ typedef struct SM501State {
     MemoryRegion twoD_engine_region;
     uint32_t last_width;
     uint32_t last_height;
+    bool do_full_update; /* perform a full update next time */
     I2CBus *i2c_bus;
 
     /* mmio registers */

@@ -1042,6 +1043,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr,
 
     assert(range_covers_byte(0, 0x400 * 3, addr));
     *(uint32_t *)&s->dc_palette[addr] = value;
+    s->do_full_update = true;
 }
 
 static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr,

@@ -1630,6 +1632,12 @@ static void sm501_update_display(void *opaque)
         full_update = 1;
     }
 
+    /* someone else requested a full update */
+    if (s->do_full_update) {
+        s->do_full_update = false;
+        full_update = 1;
+    }
+
     /* draw each line according to conditions */
     snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region,
               offset, width * height * src_bpp, DIRTY_MEMORY_VGA);
Signature · 1 line
2.7.6
author
#2 lore raw

PATCH v3 1/7 sm501: Implement i2c part for reading monitor EDID

In reply to BALATON Zoltan

Emulate the i2c part of SM501 which is used to access the EDID info from a monitor.

The vmstate structure is changed and its version is increased but SM501 is only used on SH and PPC sam460ex machines that don't support cross-version migration.

Signed-off-by BALATON Zoltan <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>

---
v3:
- Fixed resetting error bit in status reg
- Add comment for ddc
v2:
- added constants for register bits
- fix clearing error bit in reset reg
- set max access size to 1
v1: fixed build with SH

 default-configs/ppc-softmmu.mak    |   1 +
 default-configs/ppcemb-softmmu.mak |   1 +
 default-configs/sh4-softmmu.mak    |   2 +
 default-configs/sh4eb-softmmu.mak  |   2 +
 hw/display/sm501.c                 | 147 ++++++++++++++++++++++++++++++++++++-
 5 files changed, 149 insertions(+), 4 deletions(-)
default-configs/ppc-softmmu.mak +1 -0
index 6f12bf8..3181bbf 100644
@@ -25,6 +25,7 @@ CONFIG_ETSEC=y
 CONFIG_SAM460EX=y
 CONFIG_USB_EHCI_SYSBUS=y
 CONFIG_SM501=y
+CONFIG_DDC=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
 CONFIG_BITBANG_I2C=y
default-configs/ppcemb-softmmu.mak +1 -0
index 37af193..ac44f15 100644
@@ -17,6 +17,7 @@ CONFIG_XILINX=y
 CONFIG_XILINX_ETHLITE=y
 CONFIG_USB_EHCI_SYSBUS=y
 CONFIG_SM501=y
+CONFIG_DDC=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
 CONFIG_BITBANG_I2C=y
default-configs/sh4-softmmu.mak +2 -0
index 546d855..caeccd5 100644
@@ -9,6 +9,8 @@ CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
+CONFIG_I2C=y
+CONFIG_DDC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_I82378=y
 CONFIG_I8259=y
default-configs/sh4eb-softmmu.mak +2 -0
index 2d3fd49..53b9cd7 100644
@@ -9,6 +9,8 @@ CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
 CONFIG_IDE_MMIO=y
 CONFIG_SM501=y
+CONFIG_I2C=y
+CONFIG_DDC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_I82378=y
 CONFIG_I8259=y
hw/display/sm501.c +143 -4

223 lines not on this page · raw message

Signature · 1 line
2.7.6
author
#3 lore raw

PATCH v3 6/7 sm501: Fix support for non-zero frame buffer start address

In reply to BALATON Zoltan

Display updates and drawing hardware cursor did not work when frame buffer address was non-zero. Fix this by taking the frame buffer address into account in these cases. This fixes screen dragging on AmigaOS. Based on patch by Sebastian Bauer.

Signed-off-by Sebastian Bauer <[email protected]>
Signed-off-by BALATON Zoltan <[email protected]>

---
v2: fixed crash with Linux setting extra bits and log unimplemented case

 hw/display/sm501.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
hw/display/sm501.c +16 -3
index ebd1390..10ee008 100644
@@ -586,6 +586,11 @@ static uint32_t get_local_mem_size_index(uint32_t size)
     return index;
 }
 
+static ram_addr_t get_fb_addr(SM501State *s, int crt)
+{
+    return (crt ? s->dc_crt_fb_addr : s->dc_panel_fb_addr) & 0x3FFFFF0;
+}
+
 static inline int get_width(SM501State *s, int crt)
 {
     int width = crt ? s->dc_crt_h_total : s->dc_panel_h_total;

@@ -688,7 +693,8 @@ static inline void hwc_invalidate(SM501State *s, int crt)
     start *= w * bpp;
     end *= w * bpp;
 
-    memory_region_set_dirty(&s->local_mem_region, start, end - start);
+    memory_region_set_dirty(&s->local_mem_region,
+                            get_fb_addr(s, crt) + start, end - start);
 }
 
 static void sm501_2d_operation(SM501State *s)

@@ -1213,6 +1219,9 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr,
         break;
     case SM501_DC_PANEL_FB_ADDR:
         s->dc_panel_fb_addr = value & 0x8FFFFFF0;
+        if (value & 0x8000000) {
+            qemu_log_mask(LOG_UNIMP, "Panel external memory not supported\n");
+        }
         break;
     case SM501_DC_PANEL_FB_OFFSET:
         s->dc_panel_fb_offset = value & 0x3FF03FF0;

@@ -1273,6 +1282,9 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr addr,
         break;
     case SM501_DC_CRT_FB_ADDR:
         s->dc_crt_fb_addr = value & 0x8FFFFFF0;
+        if (value & 0x8000000) {
+            qemu_log_mask(LOG_UNIMP, "CRT external memory not supported\n");
+        }
         break;
     case SM501_DC_CRT_FB_OFFSET:
         s->dc_crt_fb_offset = value & 0x3FF03FF0;

@@ -1615,7 +1627,7 @@ static void sm501_update_display(void *opaque)
     draw_hwc_line_func *draw_hwc_line = NULL;
     int full_update = 0;
     int y_start = -1;
-    ram_addr_t offset = 0;
+    ram_addr_t offset;
     uint32_t *palette;
     uint8_t hwc_palette[3 * 3];
     uint8_t *hwc_src = NULL;

@@ -1672,9 +1684,10 @@ static void sm501_update_display(void *opaque)
     }
 
     /* draw each line according to conditions */
+    offset = get_fb_addr(s, crt);
     snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region,
               offset, width * height * src_bpp, DIRTY_MEMORY_VGA);
-    for (y = 0, offset = 0; y < height; y++, offset += width * src_bpp) {
+    for (y = 0; y < height; y++, offset += width * src_bpp) {
         int update, update_hwc;
 
         /* check if hardware cursor is enabled and we're within its range */
Signature · 1 line
2.7.6
#4 lore raw

PATCH v3 3/7 sm501: Use values from the pitch register for 2D operations

In reply to BALATON Zoltan

Before, crt_h_total was used for src_width and dst_width. This is a property of the current display setting and not relevant for the 2D operation that also can be done off-screen. The pitch register's purpose is to describe line pitch relevant of the 2D operation.

Signed-off-by Sebastian Bauer <[email protected]>
Signed-off-by BALATON Zoltan <[email protected]>
---
 hw/display/sm501.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
hw/display/sm501.c +2 -2
index b095134..d4878f0 100644
@@ -709,8 +709,8 @@ static void sm501_2d_operation(SM501State *s)
     /* get frame buffer info */
     uint8_t *src = s->local_mem + (s->twoD_source_base & 0x03FFFFFF);
     uint8_t *dst = s->local_mem + (s->twoD_destination_base & 0x03FFFFFF);
-    int src_width = (s->dc_crt_h_total & 0x00000FFF) + 1;
-    int dst_width = (s->dc_crt_h_total & 0x00000FFF) + 1;
+    int src_width = s->twoD_pitch & 0x1FFF;
+    int dst_width = (s->twoD_pitch >> 16) & 0x1FFF;
 
     if (addressing != 0x0) {
         printf("%s: only XY addressing is supported.\n", __func__);
Signature · 1 line
2.7.6
#5 lore raw

PATCH v3 4/7 sm501: Implement negated destination raster operation mode

In reply to BALATON Zoltan

Add support for the negated destination operation mode. This is used e.g. by AmigaOS for the INVERSEVID drawing mode. With this change, the cursor in the shell and non-immediate window adjustment are working now.

Signed-off-by BALATON Zoltan <[email protected]>
---
 hw/display/sm501.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
hw/display/sm501.c +11 -1
index d4878f0..0b44d95 100644
@@ -705,6 +705,8 @@ static void sm501_2d_operation(SM501State *s)
     uint32_t color = s->twoD_foreground;
     int format_flags = (s->twoD_stretch >> 20) & 0x3;
     int addressing = (s->twoD_stretch >> 16) & 0xF;
+    int rop_mode = (s->twoD_control >> 15) & 0x1; /* 1 for rop2, else rop3 */
+    int rop = s->twoD_control & 0xFF;
 
     /* get frame buffer info */
     uint8_t *src = s->local_mem + (s->twoD_source_base & 0x03FFFFFF);

@@ -729,6 +731,8 @@ static void sm501_2d_operation(SM501State *s)
         int y, x, index_d, index_s;                                           \
         for (y = 0; y < operation_height; y++) {                              \
             for (x = 0; x < operation_width; x++) {                           \
+                _pixel_type val;                                              \
+                                                                              \
                 if (rtl) {                                                    \
                     index_s = ((src_y - y) * src_width + src_x - x) * _bpp;   \
                     index_d = ((dst_y - y) * dst_width + dst_x - x) * _bpp;   \

@@ -736,7 +740,13 @@ static void sm501_2d_operation(SM501State *s)
                     index_s = ((src_y + y) * src_width + src_x + x) * _bpp;   \
                     index_d = ((dst_y + y) * dst_width + dst_x + x) * _bpp;   \
                 }                                                             \
-                *(_pixel_type *)&dst[index_d] = *(_pixel_type *)&src[index_s];\
+                if (rop_mode == 1 && rop == 5) {                              \
+                    /* Invert dest */                                         \
+                    val = ~*(_pixel_type *)&dst[index_d];                     \
+                } else {                                                      \
+                    val = *(_pixel_type *)&src[index_s];                      \
+                }                                                             \
+                *(_pixel_type *)&dst[index_d] = val;                          \
             }                                                                 \
         }                                                                     \
     }
Signature · 1 line
2.7.6
#6 lore raw

PATCH v3 5/7 sm501: Log unimplemented raster operation modes

In reply to BALATON Zoltan

The sm501 currently implements only a very limited set of raster operation modes. After this change, unknown raster operation modes are logged so these can be easily spotted.

Signed-off-by BALATON Zoltan <[email protected]>
---
 hw/display/sm501.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
hw/display/sm501.c +23 -0
index 0b44d95..ebd1390 100644
@@ -706,6 +706,8 @@ static void sm501_2d_operation(SM501State *s)
     int format_flags = (s->twoD_stretch >> 20) & 0x3;
     int addressing = (s->twoD_stretch >> 16) & 0xF;
     int rop_mode = (s->twoD_control >> 15) & 0x1; /* 1 for rop2, else rop3 */
+    /* 1 if rop2 source is the pattern, otherwise the source is the bitmap */
+    int rop2_source_is_pattern = (s->twoD_control >> 14) & 0x1;
     int rop = s->twoD_control & 0xFF;
 
     /* get frame buffer info */

@@ -719,6 +721,27 @@ static void sm501_2d_operation(SM501State *s)
         abort();
     }
 
+    if (rop_mode == 0) {
+        if (rop != 0xcc) {
+            /* Anything other than plain copies are not supported */
+            qemu_log_mask(LOG_UNIMP, "sm501: rop3 mode with rop %x is not "
+                          "supported.\n", rop);
+        }
+    } else {
+        if (rop2_source_is_pattern && rop != 0x5) {
+            /* For pattern source, we support only inverse dest */
+            qemu_log_mask(LOG_UNIMP, "sm501: rop2 source being the pattern and "
+                          "rop %x is not supported.\n", rop);
+        } else {
+            if (rop != 0x5 && rop != 0xc) {
+                /* Anything other than plain copies or inverse dest is not
+                 * supported */
+                qemu_log_mask(LOG_UNIMP, "sm501: rop mode %x is not "
+                              "supported.\n", rop);
+            }
+        }
+    }
+
     if ((s->twoD_source_base & 0x08000000) ||
         (s->twoD_destination_base & 0x08000000)) {
         printf("%s: only local memory is supported.\n", __func__);
Signature · 1 line
2.7.6
author
#7 lore raw

PATCH v3 7/7 sm501: Set updated region dirty after 2D operation

In reply to BALATON Zoltan

Set the changed memory region dirty after performed a 2D operation to ensure that the screen is updated properly.

Signed-off-by BALATON Zoltan <[email protected]>

---
v2: fixed to work with non-zero fb_addr

 hw/display/sm501.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
hw/display/sm501.c +15 -2
index 10ee008..3661a89 100644
@@ -715,12 +715,16 @@ static void sm501_2d_operation(SM501State *s)
     /* 1 if rop2 source is the pattern, otherwise the source is the bitmap */
     int rop2_source_is_pattern = (s->twoD_control >> 14) & 0x1;
     int rop = s->twoD_control & 0xFF;
+    uint32_t src_base = s->twoD_source_base & 0x03FFFFFF;
+    uint32_t dst_base = s->twoD_destination_base & 0x03FFFFFF;
 
     /* get frame buffer info */
-    uint8_t *src = s->local_mem + (s->twoD_source_base & 0x03FFFFFF);
-    uint8_t *dst = s->local_mem + (s->twoD_destination_base & 0x03FFFFFF);
+    uint8_t *src = s->local_mem + src_base;
+    uint8_t *dst = s->local_mem + dst_base;
     int src_width = s->twoD_pitch & 0x1FFF;
     int dst_width = (s->twoD_pitch >> 16) & 0x1FFF;
+    int crt = (s->dc_crt_control & SM501_DC_CRT_CONTROL_SEL) ? 1 : 0;
+    int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt);
 
     if (addressing != 0x0) {
         printf("%s: only XY addressing is supported.\n", __func__);

@@ -821,6 +825,15 @@ static void sm501_2d_operation(SM501State *s)
         abort();
         break;
     }
+
+    if (dst_base >= get_fb_addr(s, crt) &&
+        dst_base <= get_fb_addr(s, crt) + fb_len) {
+        int dst_len = MIN(fb_len, ((dst_y + operation_height - 1) * dst_width +
+                           dst_x + operation_width) * (1 << format_flags));
+        if (dst_len) {
+            memory_region_set_dirty(&s->local_mem_region, dst_base, dst_len);
+        }
+    }
 }
 
 static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,
Signature · 1 line
2.7.6
#8 lore raw
In reply to BALATON Zoltan
Quoted 2 lines · BALATON Zoltan

Version 3 with changes to 1/7 suggested by Philippe Mathieu-Daudé. Hopefully this haven't missed 3.0 yet.

It has, sorry. Applied to ppc-for-3.1.

Quoted 19 lines

BALATON Zoltan (3):
sm501: Implement i2c part for reading monitor EDID
sm501: Fix support for non-zero frame buffer start address
sm501: Set updated region dirty after 2D operation

Sebastian Bauer (4):
sm501: Perform a full update after palette change
sm501: Use values from the pitch register for 2D operations
sm501: Implement negated destination raster operation mode
sm501: Log unimplemented raster operation modes

 default-configs/ppc-softmmu.mak    |   1 +
 default-configs/ppcemb-softmmu.mak |   1 +
 default-configs/sh4-softmmu.mak    |   2 +
 default-configs/sh4eb-softmmu.mak  |   2 +
 hw/display/sm501.c                 | 230 +++++++++++++++++++++++++++++++++++--
 5 files changed, 224 insertions(+), 12 deletions(-)
Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson