Re: [PATCH v2 08/12] macfb: add common monitor modes supported by the MacOS toolbox ROM

[Laurent Vivier]

Le 05/10/2021 à 13:38, Mark Cave-Ayland a écrit :
> On 05/10/2021 10:50, Laurent Vivier wrote:
> 
>> Le 04/10/2021 à 23:19, Mark Cave-Ayland a écrit :
>>> The monitor modes table is found by experimenting with the Monitors Control
>>> Panel in MacOS and analysing the reads/writes. From this it can be found 
>>> that
>>> the mode is controlled by writes to the DAFB_MODE_CTRL1 and DAFB_MODE_CTRL2
>>> registers.
>>>
>>> Implement the first block of DAFB registers as a register array including 
>>> the
>>> existing sense register, the newly discovered control registers above, and 
>>> also
>>> the DAFB_MODE_VADDR1 and DAFB_MODE_VADDR2 registers which are used by 
>>> NetBSD to
>>> determine the current video mode.
>>>
>>> These experiments also show that the offset of the start of video RAM and 
>>> the
>>> stride can change depending upon the monitor mode, so update 
>>> macfb_draw_graphic()
>>> and both the BI_MAC_VADDR and BI_MAC_VROW bootinfo for the q800 machine
>>> accordingly.
>>>
>>> Finally update macfb_common_realize() so that only the resolution and depth
>>> supported by the display type can be specified on the command line.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>   hw/display/macfb.c         | 124 ++++++++++++++++++++++++++++++++-----
>>>   hw/display/trace-events    |   1 +
>>>   hw/m68k/q800.c             |  11 ++--
>>>   include/hw/display/macfb.h |  16 ++++-
>>>   4 files changed, 131 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
>>> index f98bcdec2d..357fe18be5 100644
>>> --- a/hw/display/macfb.c
>>> +++ b/hw/display/macfb.c
>>>
>> ...
>>> +static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
>>> +                                  uint16_t width, uint16_t height,
>>> +                                  uint8_t depth)
>>> +{
>>> +    MacFbMode *macfb_mode;
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
>>> +        macfb_mode = &macfb_mode_table[i];
>>> +
>>> +        if (display_type == macfb_mode->type && width == macfb_mode->width 
>>> &&
>>> +                height == macfb_mode->height && depth == 
>>> macfb_mode->depth) {
>>> +            return macfb_mode;
>>> +        }
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>
>> I misunderstood this part when I reviewed v1...
>>
>> It means you have to provide the monitor type to QEMU to switch from the 
>> default mode?
> 
> Not as such: both the MacOS toolbox ROM and MacOS itself offer a fixed set of 
> resolutions and depths
> based upon the display type. What I've done for now is default the display 
> type to VGA since it
> offers both 640x480 and 800x600 in 1, 2, 4, 8, 16 and 24-bit colour which 
> should cover the most
> common use of cases of people wanting to boot using the MacOS toolbox ROM.
> 
> Even if you specify a default on the command line, MacOS still only cares 
> about the display type and
> will allow you to change the resolution and depth dynamically, remembering 
> the last resolution and
> depth across reboots.
> 
> During testing I found that having access to the 1152x870 resolution offered 
> by the Apple 21"
> monitor display type was useful to allow larger screen sizes, although only 
> up to 8-bit depth so I
> added a bit of code that will switch from a VGA display type to a 21" display 
> type if the graphics
> resolution is set to 1152x870x8.
> 
> Finally if you boot a Linux kernel directly using -kernel then the provided 
> XxYxD is placed directly
> into the relevant bootinfo fields with a VGA display type, unless a 
> resolution of 1152x870x8 is
> specified in which case the 21" display type is used as above.
> 
>> But, as a user, how do we know which modes are allowed with which resolution?
>>
>> Is possible to try to set internally the type here according to the 
>> resolution?
>>
>> Could you provide an command line example how to start the q800 with the 
>> 1152x870 resolution?
> 
> Sure - simply add "-g 1152x870x8" to your command line. If the -g parameter 
> is omitted then the
> display type will default to VGA.
> 

Thank you for the explanation.

Perhaps you can add in the error message the list of the available mode and 
depth?
(it's not a blocker for the series, it can be added later)

As an user, it's hard to know what are the allowed values.

Thanks,
Laurent