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_deviceI 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@xxxxxxxxxxxx>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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
