On Thu, 15 Jan 2026, Chad Jablonski wrote:
> DP_GUI_MASTER_CNTL aliases several fields from DP_DATATYPE and DP_MIX.
> These were being written correctly but not returned on a read of
> GP_GUI_MASTER_CNTL.
>
> Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
> ---
> hw/display/ati.c | 17 +++++++++++++++--
> hw/display/ati_regs.h | 5 +++++
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 6967cc0ad9..b0c87f9f80 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -459,9 +459,22 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
> case DST_Y:
> val = s->regs.dst_y;
> break;
> - case DP_GUI_MASTER_CNTL:
> - val = s->regs.dp_gui_master_cntl;
> + case DP_GUI_MASTER_CNTL: {
At other places the { if needed is put on the next line after the case
label not directly after it. But I'm not sure we need to extract values
into variables then shift them again. I'd just combine these shifts and
and get rid of the variables, that way also no braces are needed. That may
be less obvious but only needs to be checked once at review and some
values already match so no need to shift them back and forth.
Regards,
BALATON Zoltan
> + /* DP_GUI_MASTER_CNTL aliases fields from DP_MIX and DP_DATATYPE */
> + uint32_t dst_datatype = s->regs.dp_datatype & DP_DST_DATATYPE;
> + uint32_t brush_datatype = (s->regs.dp_datatype &
> + DP_BRUSH_DATATYPE) >> 8;
> + uint32_t src_datatype = (s->regs.dp_datatype & DP_SRC_DATATYPE) >> 16;
> + uint32_t src_source = (s->regs.dp_mix & DP_SRC_SOURCE) >> 8;
> + uint32_t rop3 = (s->regs.dp_mix & DP_ROP3) >> 16;
> + val = s->regs.dp_gui_master_cntl |
> + (brush_datatype << 4) |
> + (dst_datatype << 8) |
> + (src_datatype << 12) |
> + (rop3 << 16) |
> + (src_source << 24);
> break;
> + }
> case SRC_OFFSET:
> val = s->regs.src_offset;
> break;
> diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
> index d7127748ff..0a0825db04 100644
> --- a/hw/display/ati_regs.h
> +++ b/hw/display/ati_regs.h
> @@ -386,6 +386,9 @@
> #define DST_16BPP 0x00000004
> #define DST_24BPP 0x00000005
> #define DST_32BPP 0x00000006
> +#define DP_DST_DATATYPE 0x0000000f
> +#define DP_BRUSH_DATATYPE 0x00000f00
> +#define DP_SRC_DATATYPE 0x00030000
>
> #define BRUSH_SOLIDCOLOR 0x00000d00
>
> @@ -437,6 +440,8 @@
> #define DP_SRC_RECT 0x00000200
> #define DP_SRC_HOST 0x00000300
> #define DP_SRC_HOST_BYTEALIGN 0x00000400
> +#define DP_SRC_SOURCE 0x00000700
> +#define DP_ROP3 0x00ff0000
>
> /* LVDS_GEN_CNTL constants */
> #define LVDS_BL_MOD_LEVEL_MASK 0x0000ff00
>