Hello Hoan, On Tuesday, 23 October 2018 04:01:19 EET Hoan wrote: > On 2018/10/22 20:23, Laurent Pinchart wrote: > > On Monday, 22 October 2018 09:30:54 EEST Nguyen An Hoan wrote: > >> From: Hoan Nguyen An <na-hoan@xxxxxxxxxxx> > >> > >> From previous commit 0521ccb "drm: rcar-du: Cache DSYSR value to ensure > >> > >> known initial value" > > > > What exact commit are you referring to ? The mainline commit that has this > > subject is 9144adc5e5a99577bce0d4ee2ca3615f53b9d296. > > Seems I have cited the wrong Commit-ID、it is > > 9144adc5e5a99577bce0d4ee2ca3615f53b9d296 > drm: rcar-du: Cache DSYSR value to ensure known initial value > > >> We only need to update DSYSR0, DSYSR2 for start/stop. > >> So using rgrp-> mmio_offset is enough, the change back from rcar_du_crtc > >> ->rcar_du_group -> rcar_du_crtc leading to mmio addresses for DSYSR may > >> be different. > > > > Is this fixing an actual problem ? If you look at the code, the line > > > > struct rcar_du_crtc *rcrtc = &rgrp->dev->crtcs[rgrp->index * 2]; > > > > makes sure that we select DU0 or DU2 only, so the register write > > > > rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_DRES | DSYSR_DEN, > > start ? DSYSR_DEN : DSYSR_DRES); > > > > should only access DSYSR0 and DSYSR2. > > This seems I think to fix the rcar-du problem with M3N-r8a77965, > > With M3N-R8a77965 we have DU0, DU1, DU3 > So, when Laurent-san divide objetcs into rcar_du_group, rcar_du_crtc. > > DU0, DU1 -> du_group[0] rgrp-> mmio_offset = DU0_REG_OFFSET > DU3->du_group[1] and rgrp-> mmio_offset = DU2_REG_OFFSET, but > rcrtc->mmio_offset=DU3_REG_OFFSET (with M3N) > > M3N-R8a77965 not have DU2, So after the command: > > struct rcar_du_crtc *rcrtc = &rgrp->dev->crtcs[rgrp->index * 2]; > > So in fact, with M3N we are updating DSYSR3 (In this my TC, this > reference to start/stop DU3-RGB) > > This will not affect H3, since the H3 lines always have enough DU0, > DU1,DU2,DU3. You're absolutely right. I'm sorry for introducing the bug in the first place, and for failing to understand your patch. I would however prefer a different fix, as switching back to rcar_du_group_write() defeats the purpose of the "drm: rcar-du: Cache DSYSR value to ensure known initial value" patch. I will submit a patch and CC you. -- Regards, Laurent Pinchart