[PATCH 00/14] drm/mgag200: Move model-specific code into separate functions

Thomas Zimmermann tzimmermann at suse.de
Mon Jul 11 07:29:30 UTC 2022


Hi Sam,

thanks for reviewing.

Am 08.07.22 um 19:22 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Jul 08, 2022 at 11:39:15AM +0200, Thomas Zimmermann wrote:
>> Mgag200 still mixes model-specific code and generic code in the same
>> functions. Separate it into distinct helpers.
>>
>> As part of this effort, convert the driver from simple-KMS helpers
>> to regular atomic helpers. The latter are way more flexible and can
>> be adapted easily for each hardware model.
>>
>> Tested on Matrox G200 and G200EH hardware.
>>
>> Thomas Zimmermann (14):
>>    drm/mgag200: Split mgag200_modeset_init()
>>    drm/mgag200: Move DAC-register setup into model-specific code
>>    dmr/mgag200: Move ER/EW3 register initializatino to per-model code
>>    drm/mgag200: Acquire I/O-register lock in atomic_commit_tail function
>>    drm/mgag200: Store primary plane's color format in CRTC state
>>    drm/mgag200: Reorganize before dropping simple-KMS helpers
>>    drm/mgag200: Replace simple-KMS with regular atomic helpers
>>    drm/mgag200: Set SCROFF in primary-plane code
>>    drm/mgag200: Add per-device callbacks
>>    drm/mgag200: Provide per-device callbacks for BMC synchronization
>>    drm/mgag200: Provide per-device callbacks for PIXPLLC
>>    drm/mgag200: Move mode-config to model-specific code
>>    drm/mgag200: Move CRTC atomic_enable to model-specfic code
>>    drm/mgag200: Remove type field from struct mga_device
> 
> I have browsed the patches and they looked good.
> I was about to complain about several things, like bmc init, but then
> later this is all nicely cleaned up.
> Especially the pll setup stuff did not get much scrunity, as it like
> most of the patch looked like code movements.

Indeed. The patch moves code and adapts the functions' interfaces to the 
new callbacks. But the implementation remains the same.

> 
> All patches except "Move DAC-register setup into model-specific code"
> are:
> Acked-by: Sam Ravnborg <sam at ravnborg.org>
> 
> 
>>   14 files changed, 2804 insertions(+), 1586 deletions(-)
> This is not a diffstat that makes one go - yyipeee.
> But the improvements makes it worthwhile.

Yeah. I accept a grow in driver size in exchange for the more cleanly 
structured code base. In my reply to the DAC review, I outline ways to 
re-share some of the duplicated code.

Best regards
Thomas

> 
> 	Sam

-- 
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: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220711/8898f909/attachment.sig>


More information about the dri-devel mailing list