Virtualization - Feb 2023 - [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

Hi

Am 16.02.23 um 13:52 schrieb Ville Syrj?l?:
> On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
>> Hi,
>>
>> thanks for taking a look at the patches.
>>
>> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
>>> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
>>>> Set the VGA bit for unblanking with macro constants instead of
magic
>>>> values. No functional changes.
>>>
>>> blank/unblank should work simliar to bochs (see commit
250e743915d4),
>>> that is maybe a nice thing to add of you modernize the driver
anyway.
>> Yeah, it's the VGA PAS field. [1] But is it really called blanking?
PAS
>> controls palette access, but blanking is sounds more like DPMS.
> 
> Why aren't people just using the normal way of flipping the
> screen off bit in sequencer register 01?
Setting the SD bit in SR01 isn't a bad idea. We can do this as part of 
enabling/disabling the plane.

But for PAS, we don't have a choice. It's one of the bazillion obscure 
VGA settings and (according to a comment in the source code) we need to 
update it for compatibility.

Best regards
Thomas
> 
> 
>>
>> The PAS setting is actually part of the primary plane, so it's
current
>> location in the CRTC code is misleading. I didn't want to change
the
>> driver logic too much, but I guess I'll fix that in the next
iteration.
>>
>> Best regards
>> Thomas
>>
>> [1]
>>
https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/attrreg.htm#3C0
>>
>>>
>>> take care,
>>>     Gerd
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 N?rnberg, Germany
>> (HRB 36809, AG N?rnberg)
>> Gesch?ftsf?hrer: Ivo Totev
> 
> 
> 
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 N?rnberg, Germany
(HRB 36809, AG N?rnberg)
Gesch?ftsf?hrer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230216/47cbde35/attachment.sig>

On Thu, Feb 16, 2023 at 02:21:43PM +0100, Thomas Zimmermann
wrote:
> Hi
> 
> Am 16.02.23 um 13:52 schrieb Ville Syrj?l?:
> > On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
> >> Hi,
> >>
> >> thanks for taking a look at the patches.
> >>
> >> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> >>> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann
wrote:
> >>>> Set the VGA bit for unblanking with macro constants
instead of magic
> >>>> values. No functional changes.
> >>>
> >>> blank/unblank should work simliar to bochs (see commit
250e743915d4),
> >>> that is maybe a nice thing to add of you modernize the driver
anyway.
> >> Yeah, it's the VGA PAS field. [1] But is it really called
blanking? PAS
> >> controls palette access, but blanking is sounds more like DPMS.
> > 
> > Why aren't people just using the normal way of flipping the
> > screen off bit in sequencer register 01?
> 
> Setting the SD bit in SR01 isn't a bad idea. We can do this as part of 
> enabling/disabling the plane.
> 
> But for PAS, we don't have a choice. It's one of the bazillion
obscure
> VGA settings and (according to a comment in the source code) we need to 
> update it for compatibility.
Well, you do need to enable the palette to see something
other that border color. Not sure tha't a very obscure thing :P

On a related note, the code looks pretty sketchy. It just
blindly writes to 0x3c0 assuming it is the attribute controller
index register. But unless you explicitly reset the flip-flop
it could actually be the data write register instead. That could
easily happen if the previous access to the attribute controller
was a read since reads do not toggle the register role.

-- 
Ville Syrj?l?
Intel