Re: [Freedreno] [PATCH v1 1/3] drm/msm/dpu: set dirty_fb flag while in self refresh mode
Hi, On Fri, Mar 31, 2023 at 6:59 AM Vinod Polimera wrote: > > While in virtual terminal mode with PSR enabled, there will be > no atomic commits triggered without dirty_fb being set. This > will create a notion of no screen update. Allow atomic commit > when dirty_fb ioctl is issued, so that it can trigger a PSR exit > and shows update on the screen. > > Reported-by: Bjorn Andersson > Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/ > Signed-off-by: Vinod Polimera > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +++ > 1 file changed, 3 insertions(+) I can confirm that this patch plus patch #2 fixes the typing problems seen on VT2 on a Chromebook using PSR. Tested-by: Douglas Anderson -Doug
Re: [Freedreno] [PATCH v1 1/3] drm/msm/dpu: set dirty_fb flag while in self refresh mode
On 31/03/2023 17:45, Dmitry Baryshkov wrote:
On Fri, 31 Mar 2023 at 16:59, Vinod Polimera wrote:
While in virtual terminal mode with PSR enabled, there will be
no atomic commits triggered without dirty_fb being set. This
will create a notion of no screen update. Allow atomic commit
when dirty_fb ioctl is issued, so that it can trigger a PSR exit
and shows update on the screen.
Will this impact non-VT workloads? If I remember correctly, we added
dirty_fb handling to prevent the framework from limiting the page
flips to vblank events (in DSI video mode).
Actually, this is kind of stupid. If we care about the workload of this
pipe, then it is being updated, which means it is not in SR mode,
self_refresh_active = false.
Reviewed-by: Dmitry Baryshkov
Reported-by: Bjorn Andersson
Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
Signed-off-by: Vinod Polimera
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index ab636da..96f645e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1158,6 +1158,9 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state
*cstate)
struct drm_crtc *crtc = cstate->crtc;
struct drm_encoder *encoder;
+ if (cstate->self_refresh_active)
+ return true;
+
drm_for_each_encoder_mask (encoder, crtc->dev, cstate->encoder_mask) {
if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) {
return true;
--
2.7.4
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v1 1/3] drm/msm/dpu: set dirty_fb flag while in self refresh mode
> On Fri, 31 Mar 2023 at 16:59, Vinod Polimera
> wrote:
> >
> > While in virtual terminal mode with PSR enabled, there will be
> > no atomic commits triggered without dirty_fb being set. This
> > will create a notion of no screen update. Allow atomic commit
> > when dirty_fb ioctl is issued, so that it can trigger a PSR exit
> > and shows update on the screen.
>
> Will this impact non-VT workloads? If I remember correctly, we added
> dirty_fb handling to prevent the framework from limiting the page
> flips to vblank events (in DSI video mode).
>
From the use case referred in the commit text ( 9e4dde28e drm/msm: Avoid
dirtyfb stalls on video mode displays (v2)).
There can be an impact on the workload. I can think of two solutions:
1) Add modparam to configure PSR wait time (SELF_REFRESH_AVG_SEED_MS 200) and
application can set it to "-1" if they are operating on dirty_fb
2) In msm drivers, disable psr if dirty_fb_ioctl is requested from the
framework and re-enable it during regular commit.
This will involve copying dirtyflags from drm fb ->msm_fb->dpu_plane and
add atomic_check failure if ( dirty_flags && self_refresh_active).
Please let me know your thoughts on the above two.
Thanks,
Vinod.
> >
> > Reported-by: Bjorn Andersson
> > Link:
> https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
> > Signed-off-by: Vinod Polimera
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index ab636da..96f645e 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -1158,6 +1158,9 @@ static bool dpu_crtc_needs_dirtyfb(struct
> drm_crtc_state *cstate)
> > struct drm_crtc *crtc = cstate->crtc;
> > struct drm_encoder *encoder;
> >
> > + if (cstate->self_refresh_active)
> > + return true;
> > +
> > drm_for_each_encoder_mask (encoder, crtc->dev, cstate-
> >encoder_mask) {
> > if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) {
> > return true;
> > --
> > 2.7.4
> >
>
>
> --
> With best wishes
> Dmitry
Re: [Freedreno] [PATCH v1 1/3] drm/msm/dpu: set dirty_fb flag while in self refresh mode
On Fri, 31 Mar 2023 at 16:59, Vinod Polimera wrote:
>
> While in virtual terminal mode with PSR enabled, there will be
> no atomic commits triggered without dirty_fb being set. This
> will create a notion of no screen update. Allow atomic commit
> when dirty_fb ioctl is issued, so that it can trigger a PSR exit
> and shows update on the screen.
Will this impact non-VT workloads? If I remember correctly, we added
dirty_fb handling to prevent the framework from limiting the page
flips to vblank events (in DSI video mode).
>
> Reported-by: Bjorn Andersson
> Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
> Signed-off-by: Vinod Polimera
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index ab636da..96f645e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1158,6 +1158,9 @@ static bool dpu_crtc_needs_dirtyfb(struct
> drm_crtc_state *cstate)
> struct drm_crtc *crtc = cstate->crtc;
> struct drm_encoder *encoder;
>
> + if (cstate->self_refresh_active)
> + return true;
> +
> drm_for_each_encoder_mask (encoder, crtc->dev, cstate->encoder_mask) {
> if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) {
> return true;
> --
> 2.7.4
>
--
With best wishes
Dmitry
[Freedreno] [PATCH v1 1/3] drm/msm/dpu: set dirty_fb flag while in self refresh mode
While in virtual terminal mode with PSR enabled, there will be
no atomic commits triggered without dirty_fb being set. This
will create a notion of no screen update. Allow atomic commit
when dirty_fb ioctl is issued, so that it can trigger a PSR exit
and shows update on the screen.
Reported-by: Bjorn Andersson
Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
Signed-off-by: Vinod Polimera
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index ab636da..96f645e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1158,6 +1158,9 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state
*cstate)
struct drm_crtc *crtc = cstate->crtc;
struct drm_encoder *encoder;
+ if (cstate->self_refresh_active)
+ return true;
+
drm_for_each_encoder_mask (encoder, crtc->dev, cstate->encoder_mask) {
if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) {
return true;
--
2.7.4
