Hi Thomas, On Mon, Jul 05, 2021 at 02:45:04PM +0200, Thomas Zimmermann wrote: > Put the clock-selection code into each of the PLL-update functions to > make them select the correct pixel clock. > > The pixel clock for video output was not actually set before programming > the clock's values. It worked because the device had the correct clock > pre-set. > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > Fixes: db05f8d3dc87 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> > Cc: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> > Cc: Dave Airlie <airlied@xxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: <stable@xxxxxxxxxxxxxxx> # v5.9+ > --- > drivers/gpu/drm/mgag200/mgag200_mode.c | 47 ++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c > index 3b3059f471c2..482843ebb69f 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c > @@ -130,6 +130,7 @@ static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) > long ref_clk = mdev->model.g200.ref_clk; > long p_clk_min = mdev->model.g200.pclk_min; > long p_clk_max = mdev->model.g200.pclk_max; > + u8 misc; > > if (clock > p_clk_max) { > drm_err(dev, "Pixel Clock %ld too high\n", clock); > @@ -174,6 +175,11 @@ static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) > drm_dbg_kms(dev, "clock: %ld vco: %ld m: %d n: %d p: %d s: %d\n", > clock, f_vco, m, n, p, s); > > + misc = RREG8(MGA_MISC_IN); > + misc &= ~MGAREG_MISC_CLK_SEL_MASK; > + misc |= MGAREG_MISC_CLK_SEL_MGA_MSK; > + WREG8(MGA_MISC_OUT, misc); This chunk is repeated a number of times. Any good reason why this is not a small helper? Sam