On 11/07/2022 09:29, Thomas Zimmermann wrote:
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.
I've read the whole serie, and it's good overall.
I agree with Sam about the duplication of the DAC-registers setup, but
that can be solved later.
I have also tested this patchset on a G200eW, and have seen no regression.
Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
Tested-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
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
