Re: [PATCH 07/12] macfb: add qdev property to specify display type

[Laurent Vivier]

Le 02/10/2021 à 13:00, Mark Cave-Ayland a écrit :
> Since the available resolutions and colour depths are determined by the 
> attached
> display type, add a qdev property to allow the display type to be specified.
> 
> The main resolutions of interest are high resolution 1152x870 with 8-bit 
> colour
> and SVGA resolution up to 800x600 with 32-bit colour so update the q800 
> machine
> to allow high resolution mode if specified and otherwise fall back to SVGA.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/macfb.c         | 6 +++++-
>  hw/m68k/q800.c             | 5 +++++
>  include/hw/display/macfb.h | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 5c95aa4a11..023d1f0cd1 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -316,7 +316,7 @@ static uint32_t macfb_sense_read(MacfbState *s)
>      MacFbSense *macfb_sense;
>      uint8_t sense;
>  
> -    macfb_sense = &macfb_sense_table[MACFB_DISPLAY_VGA];
> +    macfb_sense = &macfb_sense_table[s->type];
>      if (macfb_sense->sense == 0x7) {
>          /* Extended sense */
>          sense = 0;
> @@ -545,6 +545,8 @@ static Property macfb_sysbus_properties[] = {
>      DEFINE_PROP_UINT32("width", MacfbSysBusState, macfb.width, 640),
>      DEFINE_PROP_UINT32("height", MacfbSysBusState, macfb.height, 480),
>      DEFINE_PROP_UINT8("depth", MacfbSysBusState, macfb.depth, 8),
> +    DEFINE_PROP_UINT8("display", MacfbSysBusState, macfb.type,
> +                      MACFB_DISPLAY_VGA),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -552,6 +554,8 @@ static Property macfb_nubus_properties[] = {
>      DEFINE_PROP_UINT32("width", MacfbNubusState, macfb.width, 640),
>      DEFINE_PROP_UINT32("height", MacfbNubusState, macfb.height, 480),
>      DEFINE_PROP_UINT8("depth", MacfbNubusState, macfb.depth, 8),
> +    DEFINE_PROP_UINT8("display", MacfbNubusState, macfb.type,
> +                      MACFB_DISPLAY_VGA),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 09b3366024..5223b880bc 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -421,6 +421,11 @@ static void q800_init(MachineState *machine)
>      qdev_prop_set_uint32(dev, "width", graphic_width);
>      qdev_prop_set_uint32(dev, "height", graphic_height);
>      qdev_prop_set_uint8(dev, "depth", graphic_depth);
> +    if (graphic_width == 1152 && graphic_height == 870 && graphic_depth == 
> 8) {
> +        qdev_prop_set_uint8(dev, "display", MACFB_DISPLAY_APPLE_21_COLOR);
> +    } else {
> +        qdev_prop_set_uint8(dev, "display", MACFB_DISPLAY_VGA);
> +    }
>      qdev_realize_and_unref(dev, BUS(nubus), &error_fatal);
>  
>      cs = CPU(cpu);
> diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
> index febf4ce0e8..e95a97ebdc 100644
> --- a/include/hw/display/macfb.h
> +++ b/include/hw/display/macfb.h
> @@ -46,6 +46,7 @@ typedef struct MacfbState {
>      uint8_t color_palette[256 * 3];
>      uint32_t width, height; /* in pixels */
>      uint8_t depth;
> +    uint8_t type;
>  
>      uint32_t sense;
>  } MacfbState;
> 

I think the display modes should be documentend somewhere to be directly usable 
by the user and get
ride of the graphic_XXX variables (and -g).

Perhaps it could also be merged with the previous one.

Thanks,
Laurent