Hi Jacopo, On 30/08/18 17:00, Kieran Bingham wrote: > Hi Jacopo, > > On 22/08/18 08:21, Jacopo Mondi wrote: >> The ESCR registers offset definition is confusing, as each channel is >> equipped with an ESCR register instance, but the names suggest only ESCR and >> ESCR2 are taken into account. >> >> Rename the offsets to a name that includes the channels they apply to, and >> write them to each channel with 'rcar_du_crtc_write()'. >> >> Cosmetic patch, no functional changes intended. > > Noted, ... > >> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> >> --- >> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 +-- >> drivers/gpu/drm/rcar-du/rcar_du_regs.h | 4 ++-- >> 2 files changed, 3 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 5454884..714c1fc 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >> @@ -294,8 +294,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc) >> } >> } >> >> - rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? ESCR2 : ESCR, >> - escr); >> + rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr); >> rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? OTAR2 : OTAR, 0); >> >> /* Signal polarities */ >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h >> index 9dfd220..ebc4aea 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h >> @@ -492,8 +492,8 @@ >> * External Synchronization Control Registers >> */ >> >> -#define ESCR 0x10000 >> -#define ESCR2 0x31000 >> +#define ESCR02 0x10000 >> +#define ESCR13 0x01000 > > Assertion failed at: > ASSERT(ESCR2 == ESCR13) > > This looks intentional ? > but that makes this a bit more than a cosmetic change? Aha - sorry - I see. It looks like the '0x3' from '0x31000' was providing the offset into the group I assume. I'm surprised an equivalent offset wasn't removed from the ESCR02. But I assume this is handled by the change of "rcar_du_crtc_write() -> rcar_du_group_write()"? Regards Kieran > > > > >> #define ESCR_DCLKOINV (1 << 25) >> #define ESCR_DCLKSEL_DCLKIN (0 << 20) >> #define ESCR_DCLKSEL_CLKS (1 << 20) >> >
