Quoting Laurent Pinchart (2021-11-29 22:28:13) > When the CMM is enabled, an offset of 25 pixels must be subtracted from > the HDS (horizontal display start) and HDE (horizontal display end) > registers. Fix the timings calculation, and take this into account in > the mode validation. > > This fixes a visible horizontal offset in the image with VGA monitors. > HDMI monitors seem to be generally more tolerant to incorrect timings, > but may be affected too. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 5672830ca184..ee6ba74627a2 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -215,6 +215,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) > const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode; > struct rcar_du_device *rcdu = rcrtc->dev; > unsigned long mode_clock = mode->clock * 1000; > + unsigned int hdse_offset; > u32 dsmr; > u32 escr; > > @@ -298,10 +299,15 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) > | DSMR_DIPM_DISP | DSMR_CSPM; > rcar_du_crtc_write(rcrtc, DSMR, dsmr); > This looks like the kind of place that could do with a comment explaining what is going on. > + hdse_offset = 19; > + if (rcrtc->group->cmms_mask & BIT(rcrtc->index % 2)) > + hdse_offset += 25; > + > /* Display timings */ > - rcar_du_crtc_write(rcrtc, HDSR, mode->htotal - mode->hsync_start - 19); > + rcar_du_crtc_write(rcrtc, HDSR, mode->htotal - mode->hsync_start - > + hdse_offset); > rcar_du_crtc_write(rcrtc, HDER, mode->htotal - mode->hsync_start + > - mode->hdisplay - 19); > + mode->hdisplay - hdse_offset); > rcar_du_crtc_write(rcrtc, HSWR, mode->hsync_end - > mode->hsync_start - 1); > rcar_du_crtc_write(rcrtc, HCR, mode->htotal - 1); > @@ -836,6 +842,7 @@ rcar_du_crtc_mode_valid(struct drm_crtc *crtc, > struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); > struct rcar_du_device *rcdu = rcrtc->dev; > bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE; > + unsigned int min_sync_porch; > unsigned int vbp; > > if (interlaced && !rcar_du_has(rcdu, RCAR_DU_FEATURE_INTERLACED)) > @@ -843,9 +850,14 @@ rcar_du_crtc_mode_valid(struct drm_crtc *crtc, > > /* > * The hardware requires a minimum combined horizontal sync and back > - * porch of 20 pixels and a minimum vertical back porch of 3 lines. > + * porch of 20 pixels (when CMM isn't used) or 45 pixels (when CMM is > + * used), and a minimum vertical back porch of 3 lines. > */ > - if (mode->htotal - mode->hsync_start < 20) > + min_sync_porch = 20; > + if (rcrtc->group->cmms_mask & BIT(rcrtc->index % 2)) > + min_sync_porch += 25; > + > + if (mode->htotal - mode->hsync_start < min_sync_porch) > return MODE_HBLANK_NARROW; Is the '19' in the hdse offset, this min_sync_port - 1 for position correction? It looks something like that. And the rest seems ok. With or without the additional optional comment suggestion above: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > > vbp = (mode->vtotal - mode->vsync_end) / (interlaced ? 2 : 1); > > base-commit: c18c8891111bb5e014e144716044991112f16833 > prerequisite-patch-id: dc9121a1b85ea05bf3eae2b0ac2168d47101ee87 > -- > Regards, > > Laurent Pinchart >
