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 <[email protected]>
Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
Signed-off-by: Vinod Polimera <[email protected]>
---
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
On Fri, 31 Mar 2023 at 16:59, Vinod Polimera <[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
> Signed-off-by: Vinod Polimera <[email protected]>
> ---
> 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
> On Fri, 31 Mar 2023 at 16:59, Vinod Polimera <[email protected]>
> 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 <[email protected]>
> > Link:
> https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
> > Signed-off-by: Vinod Polimera <[email protected]>
> > ---
> > 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
On 31/03/2023 17:45, Dmitry Baryshkov wrote:
> On Fri, 31 Mar 2023 at 16:59, Vinod Polimera <[email protected]> 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 <[email protected]>
>
>>
>> Reported-by: Bjorn Andersson <[email protected]>
>> Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
>> Signed-off-by: Vinod Polimera <[email protected]>
>> ---
>> 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
Hi,
On Fri, Mar 31, 2023 at 6:59 AM Vinod Polimera
<[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/all/20230326162723.3lo6pnsfdwzsvbhj@ripper/
> Signed-off-by: Vinod Polimera <[email protected]>
> ---
> 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 <[email protected]>
-Doug