Hi,
On Thu 13 Apr 23, 10:27, Chris Morgan wrote:
> On Thu, Apr 13, 2023 at 04:43:47PM +0200, Sascha Hauer wrote:
> > During a suspend/resume cycle the VO power domain will be disabled and
> > the VOP2 registers will reset to their default values. After that the
> > cached register values will be out of sync and the read/modify/write
> > operations we do on the window registers will result in bogus values
> > written. Fix this by re-initializing the register cache each time we
> > enable the VOP2. With this the VOP2 will show a picture after a
> > suspend/resume cycle whereas without this the screen stays dark.
I was actually tracking the very same bug this week!
Thanks a lot for fixing this, it would certainly have taken me a while to
think about regmap cache maintenance. Good thinking :)
Your patch fixes the issue on my side but I have a suggestion below:
> > Fixes: 604be85547ce4 ("drm/rockchip: Add VOP2 driver")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index ba3b817895091..d9daa686b014d 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -215,6 +215,8 @@ struct vop2 {
> > struct vop2_win win[];
> > };
> >
> > +static const struct regmap_config vop2_regmap_config;
> > +
> > static struct vop2_video_port *to_vop2_video_port(struct drm_crtc *crtc)
> > {
> > return container_of(crtc, struct vop2_video_port, crtc);
> > @@ -839,6 +841,12 @@ static void vop2_enable(struct vop2 *vop2)
> > return;
> > }
> >
> > + ret = regmap_reinit_cache(vop2->map, &vop2_regmap_config);
> > + if (ret) {
> > + drm_err(vop2->drm, "failed to reinit cache: %d\n", ret);
> > + return;
> > + }
It seems that regmap has regcache_mark_dirty() for this purpose, which is
perhaps more adapted than reinitializing cache (unless I'm missing something).
Note that I haven't tested it at this point.
Cheers,
Paul
> > if (vop2->data->soc_id == 3566)
> > vop2_writel(vop2, RK3568_OTP_WIN_EN, 1);
> >
> > --
> > 2.39.2
> >
>
> I confirmed this works on my Anbernic RG353P which uses the rk3566 SOC.
> Before applying the patch I displayed a color pattern with modetest
> before suspend and it appeared correctly. Then I suspended and resumed
> the device, attempted to display the same color pattern, and only got
> a single pixel on an otherwise blank display. After applying the patch
> I performed the same test and the color pattern appeared correctly
> both before and after suspend (and the display was no longer blank
> after resume from suspend).
>
> Tested-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
