On 1/23/19 11:08 AM, Grodzovsky, Andrey wrote:
>
>
> On 01/22/2019 01:28 PM, sunpeng.li@xxxxxxx wrote:
>> From: David Francis <David.Francis@xxxxxxx>
>>
>> [Why]
>> We were assuming that any commit with allow_modeset == false
>> was a pageflip. This was against drm intention and only
>> worked by sheer luck
>>
>> [How]
>> A pageflip is the change from one framebuffer to another
>
> What about other references to state->allow_modeset in DM code ? You
> have one in dm_determine_update_type_for_commit which will actually
> cause you to assume fast update because of state->allow_modeset == false.
> Also - what about other types of plane updates which are more drastic
> (require full update) but will also have different FB, can't you wrongly
> assume they require page flip while it's actually needs
> commit_planes_to_stream ?
>
> Andrey
Other references (in atomic check) will still need to be addressed after
this patch. This patch only fixes page-flips not working when
state->allow_modeset = true. Everything else is mostly just about
enabling us to do fast updates more frequently for better performance.
You're right about the fast vs full update bit (since we won't be
waiting for vblank) but this gets addressed with another patch in this
series, "drm/amd/display: Call into DC once per multiplane flip".
I don't remember the ordering of which these patches were originally
applied but they may have changed when sent out to the mailing list.
For bisection purposes I think the ordering of:
[PATCH 06/20] drm/amd/display: Let updates with no scaling changes be fast
[PATCH 03/20] drm/amd/display: Simplify underscan and ABM commit
[PATCH 05/20] drm/amd/display: Call into DC once per multiplane flip
[PATCH 07/20] drm/amd/display: Perform plane updates only when needed
[PATCH 04/20] drm/amd/display: Know what a pageflip is
...probably makes the most logical sense.
Nicholas Kazlauskas
>
>>
>> Signed-off-by: David Francis <David.Francis@xxxxxxx>
>> Reviewed-by: Harry Wentland <Harry.Wentland@xxxxxxx>
>> Reviewed-by: Nicholas Kazlauskas <Nicholas.Kazlauskas@xxxxxxx>
>> Acked-by: Leo Li <sunpeng.li@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 405c263..db060da 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4995,6 +4995,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>> struct drm_crtc *crtc = new_plane_state->crtc;
>> struct drm_crtc_state *new_crtc_state;
>> struct drm_framebuffer *fb = new_plane_state->fb;
>> + struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(fb);
>> + struct amdgpu_framebuffer *old_afb = to_amdgpu_framebuffer(old_plane_state->fb);
>> bool pflip_needed;
>> struct dm_plane_state *dm_new_plane_state = to_dm_plane_state(new_plane_state);
>>
>> @@ -5010,7 +5012,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>> if (!new_crtc_state->active)
>> continue;
>>
>> - pflip_needed = !state->allow_modeset;
>> + pflip_needed = old_plane_state->fb &&
>> + (old_plane_state->fb != new_plane_state->fb || afb->address != old_afb->address);
>>
>> spin_lock_irqsave(&crtc->dev->event_lock, flags);
>> if (acrtc_attach->pflip_status != AMDGPU_FLIP_NONE) {
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
