| Message ID | 1458738918-32054-6-git-send-email-a.hajda@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andrzej, 2016? 03? 23? 22:15? Andrzej Hajda ?(?) ? ?: > DECON should be updated after un-protecting windows and after changing > output parameters, otherwise image is not displayed in case of HDMI path. I'm not sure why STANDALONE_UPDATE_F bit should be updated after un-protecting windows and changing output parameters. The fields with _F prefix mean that these will be applied after vsync so we use protection window in case of all registers which affect display output so that they can be updated together. Wouldn't there be other thing which affects HDMI output? In addition, we would need another patch which updates TV relevant registers only in case of DECON-TV. DECON_UPDATE is a register for DECON-TV. Thanks, Inki Dae > > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > --- > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > index ab9154e..7fec656 100644 > --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c > @@ -191,6 +191,8 @@ static void decon_commit(struct exynos_drm_crtc *crtc) > > /* enable output and display signal */ > decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0); > + > + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > } > > static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, > @@ -316,9 +318,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc, > > /* window enable */ > decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0); > - > - /* standalone update */ > - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > } > > static void decon_disable_plane(struct exynos_drm_crtc *crtc, > @@ -336,9 +335,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc, > decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); > > decon_shadow_protect_win(ctx, win, false); > - > - /* standalone update */ > - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > } > > static void decon_atomic_flush(struct exynos_drm_crtc *crtc) > @@ -352,6 +348,9 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc) > for (i = ctx->first_win; i < WINDOWS_NR; i++) > decon_shadow_protect_win(ctx, i, false); > > + /* standalone update */ > + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > + > if (ctx->out_type == IFTYPE_I80) > set_bit(BIT_WIN_UPDATED, &ctx->flags); > } > @@ -463,8 +462,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc) > decon_shadow_protect_win(ctx, win, true); > decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); > decon_shadow_protect_win(ctx, win, false); > - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > } > + > + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); > + > /* TODO: wait for possible vsync */ > msleep(50); > >
On 04/12/2016 04:25 AM, Inki Dae wrote: > Hi Andrzej, > > 2016? 03? 23? 22:15? Andrzej Hajda ?(?) ? ?: >> DECON should be updated after un-protecting windows and after changing >> output parameters, otherwise image is not displayed in case of HDMI path. > I'm not sure why STANDALONE_UPDATE_F bit should be updated after un-protecting windows and changing output parameters. > The fields with _F prefix mean that these will be applied after vsync so we use protection window in case of all registers which affect display output so that they can be updated together. > > Wouldn't there be other thing which affects HDMI output? > > In addition, we would need another patch which updates TV relevant registers only in case of DECON-TV. DECON_UPDATE is a register for DECON-TV. DECON_UPDATE is present in both DECON and DECON-TV and in both cases have the same field STANDALONE_UPDATE_F. Documentation for 5433 says: > When you modify the shadow attributed registers, set > STANDALONE_UPDATE_F. > So it should be set after setting registers with _F suffix, but it has also _F suffix - contradiction. So I guess this _F suffix should not be treated too strictly in this case. Moreover the suffix has been removed in Exynos7420 - it is called just STANDALONE_UPDATE. Anyway I am not sure what is exact purpose of this register and the changes proposed in the patch are rather results of multiple tests with hardware - documentation is rather poor on this subject. I am also not sure why DECON works without it but DECON-TV does not, anyway I set it in both variants because documentation says so. Regards Andrzej > > Thanks, > Inki Dae > >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >> --- >> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >> index ab9154e..7fec656 100644 >> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >> @@ -191,6 +191,8 @@ static void decon_commit(struct exynos_drm_crtc *crtc) >> >> /* enable output and display signal */ >> decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0); >> + >> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> } >> >> static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, >> @@ -316,9 +318,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc, >> >> /* window enable */ >> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0); >> - >> - /* standalone update */ >> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> } >> >> static void decon_disable_plane(struct exynos_drm_crtc *crtc, >> @@ -336,9 +335,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc, >> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); >> >> decon_shadow_protect_win(ctx, win, false); >> - >> - /* standalone update */ >> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> } >> >> static void decon_atomic_flush(struct exynos_drm_crtc *crtc) >> @@ -352,6 +348,9 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc) >> for (i = ctx->first_win; i < WINDOWS_NR; i++) >> decon_shadow_protect_win(ctx, i, false); >> >> + /* standalone update */ >> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> + >> if (ctx->out_type == IFTYPE_I80) >> set_bit(BIT_WIN_UPDATED, &ctx->flags); >> } >> @@ -463,8 +462,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc) >> decon_shadow_protect_win(ctx, win, true); >> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); >> decon_shadow_protect_win(ctx, win, false); >> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> } >> + >> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >> + >> /* TODO: wait for possible vsync */ >> msleep(50); >> >>
2016? 04? 12? 16:33? Andrzej Hajda ?(?) ? ?: > On 04/12/2016 04:25 AM, Inki Dae wrote: >> Hi Andrzej, >> >> 2016? 03? 23? 22:15? Andrzej Hajda ?(?) ? ?: >>> DECON should be updated after un-protecting windows and after changing >>> output parameters, otherwise image is not displayed in case of HDMI path. >> I'm not sure why STANDALONE_UPDATE_F bit should be updated after un-protecting windows and changing output parameters. >> The fields with _F prefix mean that these will be applied after vsync so we use protection window in case of all registers which affect display output so that they can be updated together. >> >> Wouldn't there be other thing which affects HDMI output? >> >> In addition, we would need another patch which updates TV relevant registers only in case of DECON-TV. DECON_UPDATE is a register for DECON-TV. > > DECON_UPDATE is present in both DECON and DECON-TV and in both cases > have the same field STANDALONE_UPDATE_F. > > Documentation for 5433 says: >> When you modify the shadow attributed registers, set >> STANDALONE_UPDATE_F. >> > So it should be set after setting registers with _F suffix, > but it has also _F suffix - contradiction. So I guess this _F suffix > should not be treated too strictly in this case. Moreover the suffix > has been removed in Exynos7420 - it is called just STANDALONE_UPDATE. Indeed. I thought the register name has TV suffix so this register is for DECON-TV. However, without the register setting, DECON didn't work correctly - display not updated. > > Anyway I am not sure what is exact purpose of this register and > the changes proposed in the patch are rather results of multiple tests > with hardware - documentation is rather poor on this subject. > > I am also not sure why DECON works without it but DECON-TV does not, > anyway I set it in both variants because documentation says so. Ok, picked them up. Thanks, Inki Dae > > Regards > Andrzej > >> >> Thanks, >> Inki Dae >> >>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >>> --- >>> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 15 ++++++++------- >>> 1 file changed, 8 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> index ab9154e..7fec656 100644 >>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c >>> @@ -191,6 +191,8 @@ static void decon_commit(struct exynos_drm_crtc *crtc) >>> >>> /* enable output and display signal */ >>> decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0); >>> + >>> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> } >>> >>> static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, >>> @@ -316,9 +318,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc, >>> >>> /* window enable */ >>> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0); >>> - >>> - /* standalone update */ >>> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> } >>> >>> static void decon_disable_plane(struct exynos_drm_crtc *crtc, >>> @@ -336,9 +335,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc, >>> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); >>> >>> decon_shadow_protect_win(ctx, win, false); >>> - >>> - /* standalone update */ >>> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> } >>> >>> static void decon_atomic_flush(struct exynos_drm_crtc *crtc) >>> @@ -352,6 +348,9 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc) >>> for (i = ctx->first_win; i < WINDOWS_NR; i++) >>> decon_shadow_protect_win(ctx, i, false); >>> >>> + /* standalone update */ >>> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> + >>> if (ctx->out_type == IFTYPE_I80) >>> set_bit(BIT_WIN_UPDATED, &ctx->flags); >>> } >>> @@ -463,8 +462,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc) >>> decon_shadow_protect_win(ctx, win, true); >>> decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); >>> decon_shadow_protect_win(ctx, win, false); >>> - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> } >>> + >>> + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); >>> + >>> /* TODO: wait for possible vsync */ >>> msleep(50); >>> >>> > >
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index ab9154e..7fec656 100644 --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c @@ -191,6 +191,8 @@ static void decon_commit(struct exynos_drm_crtc *crtc) /* enable output and display signal */ decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0); + + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); } static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, @@ -316,9 +318,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc, /* window enable */ decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0); - - /* standalone update */ - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); } static void decon_disable_plane(struct exynos_drm_crtc *crtc, @@ -336,9 +335,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc, decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); decon_shadow_protect_win(ctx, win, false); - - /* standalone update */ - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); } static void decon_atomic_flush(struct exynos_drm_crtc *crtc) @@ -352,6 +348,9 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc) for (i = ctx->first_win; i < WINDOWS_NR; i++) decon_shadow_protect_win(ctx, i, false); + /* standalone update */ + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); + if (ctx->out_type == IFTYPE_I80) set_bit(BIT_WIN_UPDATED, &ctx->flags); } @@ -463,8 +462,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc) decon_shadow_protect_win(ctx, win, true); decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0); decon_shadow_protect_win(ctx, win, false); - decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); } + + decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0); + /* TODO: wait for possible vsync */ msleep(50);
DECON should be updated after un-protecting windows and after changing output parameters, otherwise image is not displayed in case of HDMI path. Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)