> On September 14, 2018 at 11:10 AM Laurent Pinchart > <laurent.pinchart+rene...@ideasonboard.com> wrote: > > > DSYSR is a DU channel register that also contains group fields. It is > thus written to by both the group and CRTC code, using read-update-write > sequences. As the register isn't initialized explicitly at startup time, > this can lead to invalid or otherwise unexpected values being written to > some of the fields if they have been modified by the firmware or just > not reset properly. > > To fix this we can write a fully known value to the DSYSR register when > turning a channel's functional clock on. However, the mix of group and > channel fields complicate this. A simpler solution is to cache the > register and initialize the cached value to the desired hardware > defaults. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com> > Tested-by: Jacopo Mondi <jacopo+rene...@jmondi.org> > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 16 ++++++++-------- > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 5 +++++ > drivers/gpu/drm/rcar-du/rcar_du_group.c | 7 ++++--- > 3 files changed, 17 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 2f8776c1ec8f..f827fccf6416 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -57,13 +57,12 @@ static void rcar_du_crtc_set(struct rcar_du_crtc *rcrtc, > u32 reg, u32 set) > rcar_du_read(rcdu, rcrtc->mmio_offset + reg) | set); > } > > -static void rcar_du_crtc_clr_set(struct rcar_du_crtc *rcrtc, u32 reg, > - u32 clr, u32 set) > +void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 set) > { > struct rcar_du_device *rcdu = rcrtc->group->dev; > - u32 value = rcar_du_read(rcdu, rcrtc->mmio_offset + reg); > > - rcar_du_write(rcdu, rcrtc->mmio_offset + reg, (value & ~clr) | set); > + rcrtc->dsysr = (rcrtc->dsysr & ~clr) | set; > + rcar_du_write(rcdu, rcrtc->mmio_offset + DSYSR, rcrtc->dsysr); > } > > /* > ----------------------------------------------------------------------------- > @@ -576,9 +575,9 @@ static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) > * actively driven). > */ > interlaced = rcrtc->crtc.mode.flags & DRM_MODE_FLAG_INTERLACE; > - rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK | DSYSR_SCM_MASK, > - (interlaced ? DSYSR_SCM_INT_VIDEO : 0) | > - DSYSR_TVM_MASTER); > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK | DSYSR_SCM_MASK, > + (interlaced ? DSYSR_SCM_INT_VIDEO : 0) | > + DSYSR_TVM_MASTER); > > rcar_du_group_start_stop(rcrtc->group, true); > } > @@ -645,7 +644,7 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) > * Select switch sync mode. This stops display operation and configures > * the HSYNC and VSYNC signals as inputs. > */ > - rcar_du_crtc_clr_set(rcrtc, DSYSR, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK, DSYSR_TVM_SWITCH); > > rcar_du_group_start_stop(rcrtc->group, false); > } > @@ -1121,6 +1120,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, > unsigned int swindex, > rcrtc->group = rgrp; > rcrtc->mmio_offset = mmio_offsets[hwindex]; > rcrtc->index = hwindex; > + rcrtc->dsysr = (rcrtc->index % 2 ? 0 : DSYSR_DRES) | DSYSR_TVM_TVSYNC; > > if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) > primary = &rcrtc->vsp->planes[rcrtc->vsp_pipe].plane; > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > index 4990bbe9ba26..59ac6e7d22c9 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h > @@ -30,6 +30,7 @@ struct rcar_du_vsp; > * @mmio_offset: offset of the CRTC registers in the DU MMIO block > * @index: CRTC software and hardware index > * @initialized: whether the CRTC has been initialized and clocks enabled > + * @dsysr: cached value of the DSYSR register > * @vblank_enable: whether vblank events are enabled on this CRTC > * @event: event to post when the pending page flip completes > * @flip_wait: wait queue used to signal page flip completion > @@ -50,6 +51,8 @@ struct rcar_du_crtc { > unsigned int index; > bool initialized; > > + u32 dsysr; > + > bool vblank_enable; > struct drm_pending_vblank_event *event; > wait_queue_head_t flip_wait; > @@ -103,4 +106,6 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc, > enum rcar_du_output output); > void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc); > > +void rcar_du_crtc_dsysr_clr_set(struct rcar_du_crtc *rcrtc, u32 clr, u32 > set); > + > #endif /* __RCAR_DU_CRTC_H__ */ > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c > b/drivers/gpu/drm/rcar-du/rcar_du_group.c > index f38703e7a10d..d85f0a1c1581 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -202,9 +202,10 @@ void rcar_du_group_put(struct rcar_du_group *rgrp) > > static void __rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool > start) > { > - rcar_du_group_write(rgrp, DSYSR, > - (rcar_du_group_read(rgrp, DSYSR) & ~(DSYSR_DRES | DSYSR_DEN)) | > - (start ? DSYSR_DEN : DSYSR_DRES)); > + struct rcar_du_crtc *rcrtc = &rgrp->dev->crtcs[rgrp->index * 2]; > + > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_DRES | DSYSR_DEN, > + start ? DSYSR_DEN : DSYSR_DRES); > } > > void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start) > --
Reviewed-by: Ulrich Hecht <uli+rene...@fpond.eu> CU Uli _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel