Hi Neil, On Wed, Jan 9, 2019 at 2:36 PM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: > > Since commit 2bcd3ecab773 when switching mode from X11 (ubuntu mate for > example) the display gets blurry, looking like an invalid framebuffer width. > > This commit fixed atomic crtc modesetting but didn't update the display > parameters when changing mode, but only when starting a mode setting after > a crtc disable. > > This commit setups the crctc parameter in _begin() and _enable() to > take in account the current ctrc parameters. > > Reported-by: Tony McKahan <tonymckahan@xxxxxxxxx> > Fixes: 2bcd3ecab773 ("drm/meson: Fixes for drm_crtc_vblank_on/off support") > Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > --- > drivers/gpu/drm/meson/meson_crtc.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c > index 75d97f1b2e8f..5bb432021caf 100644 > --- a/drivers/gpu/drm/meson/meson_crtc.c > +++ b/drivers/gpu/drm/meson/meson_crtc.c > @@ -82,14 +82,12 @@ static const struct drm_crtc_funcs meson_crtc_funcs = { > > }; > > -static void meson_crtc_enable(struct drm_crtc *crtc) > +static void meson_crtc_setup(struct drm_crtc *crtc) > { > struct meson_crtc *meson_crtc = to_meson_crtc(crtc); > struct drm_crtc_state *crtc_state = crtc->state; > struct meson_drm *priv = meson_crtc->priv; > > - DRM_DEBUG_DRIVER("\n"); > - > if (!crtc_state) { > DRM_ERROR("Invalid crtc_state\n"); > return; > @@ -98,6 +96,16 @@ static void meson_crtc_enable(struct drm_crtc *crtc) > /* Enable VPP Postblend */ nit-pick: this "enable" comment is now in meson_crtc_setup(). I would drop it because my interpretation of the following lines is now "setting VPP_POSTBLEND_H_SIZE enables the VPP postblend" > writel(crtc_state->mode.hdisplay, > priv->io_base + _REG(VPP_POSTBLEND_H_SIZE)); > +} > + > +static void meson_crtc_enable(struct drm_crtc *crtc) > +{ > + struct meson_crtc *meson_crtc = to_meson_crtc(crtc); > + struct meson_drm *priv = meson_crtc->priv; > + > + DRM_DEBUG_DRIVER("\n"); > + > + meson_crtc_setup(crtc); > > /* VD1 Preblend vertical start/end */ > writel(FIELD_PREP(GENMASK(11, 0), 2303), > @@ -121,6 +129,8 @@ static void meson_crtc_atomic_enable(struct drm_crtc *crtc, > > if (!meson_crtc->enabled) > meson_crtc_enable(crtc); > + else > + meson_crtc_setup(crtc); it's probably only personal preference, but have you thought about re-ordering this: meson_crtc_setup(crtc); if (!meson_crtc->enabled) meson_crtc_enable(crtc); with that you could get rid of the meson_crtc_setup() call in meson_crtc_enable(), leaving only one code-path (instead of two) which calls meson_crtc_setup() Regards Martin _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel