[PATCH 56/73] drm/amd/display: Remove dangling planes on dc commit state

[Andrey Grodzovsky]

On 11/13/2017 04:53 PM, Leo Li wrote:
>
>
> On 2017-11-10 02:00 PM, Andrey Grodzovsky wrote:
>>
>>
>> On 11/09/2017 03:05 PM, Harry Wentland wrote:
>>> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
>>>
>>> When disabling pipe splitting, we need to make sure we disable both
>>> planes used.
>>>
>>> This should be done for Linux as well.
>>>
>>> Change-Id: I79f5416a55bd26c19ca3cfb346a943d69872a8ce
>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
>>> Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
>>> Acked-by: Harry Wentland <harry.wentland at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/display/dc/core/dc.c | 39 
>>> ++++++++++++++++++++++++++++----
>>>   1 file changed, 35 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
>>> b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> index 56df1304e49c..d70dbc102123 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> @@ -629,6 +629,39 @@ static bool construct(struct dc *dc,
>>>       return false;
>>>   }
>>> +static void disable_dangling_plane(struct dc *dc, struct dc_state 
>>> *context)
>>> +{
>>> +    int i, j;
>>> +    struct dc_state *dangling_context = dc_create_state();
>>> +    struct dc_state *current_ctx;
>>> +
>>> +    if (dangling_context == NULL)
>>> +        return;
>>> +
>>> +    dc_resource_state_copy_construct(dc->current_state, 
>>> dangling_context);
>>> +
>>> +    for (i = 0; i < dc->res_pool->pipe_count; i++) {
>>> +        struct dc_stream_state *old_stream =
>>> + dc->current_state->res_ctx.pipe_ctx[i].stream;
>>> +        bool should_disable = true;
>>> +
>>> +        for (j = 0; j < context->stream_count; j++) {
>>> +            if (old_stream == context->streams[j]) {
>>> +                should_disable = false;
>>> +                break;
>>> +            }
>>> +        }
>>> +        if (should_disable && old_stream) {
>>> +            dc_rem_all_planes_for_stream(dc, old_stream, 
>>> dangling_context);
>>
>> Why this is not happening in atomic_check during 
>> dm_update_planes_state with enable set to false ? Since the old 
>> stream is present
>> I assume old crtc_state for planes to disable is present and as I see 
>> from the code it should happen in that function
>>
>> Thanks,
>> Andrey
> You're correct, the above logic should be done in atomic check. However,
> the apply_ctx_for_surface call below is the reason why this code (or
> rather, this entire function) is here.
>
> Due to some refactoring efforts that rearranged the front-end and
> back-end programming sequence, the logic for disabling surfaces for
> streams have been moved here. disable_dangling_planes is responsible for
> disabling the front end of streams that do not exist in the new dc_state
> (i.e. streams that are disabled). See one of the relevant patches here:
> https://lists.freedesktop.org/archives/amd-gfx/2017-October/015201.html
>
> This cannot be done in atomic check, because there is currently no
> support in DC to explicitly mark a stream or surface as disabled. We
> infer it by comparing the current and new dc_states, then call apply_ctx
> to program it.
>
> Leo

I see, at least try to get rid of dangling_context I don't see why 
intermediate state is needed. Adding Tony
and Jonathan, maybe they can explain.
Also please rename  struct dc_state *context in the argument list to 
state, context is obsolete naming.

Thanks,
Andrey

>
>>
>>> + dc->hwss.apply_ctx_for_surface(dc, old_stream, 0, dangling_context);
>>> +        }
>>> +    }
>>> +
>>> +    current_ctx = dc->current_state;
>>> +    dc->current_state = dangling_context;
>>> +    dc_release_state(current_ctx);
>>> +}
>>> +
>>> /******************************************************************************* 
>>>
>>>    * Public functions
>>> ******************************************************************************/ 
>>>
>>> @@ -833,14 +866,14 @@ static enum dc_status 
>>> dc_commit_state_no_check(struct dc *dc, struct dc_state *c
>>>       int i, k, l;
>>>       struct dc_stream_state *dc_streams[MAX_STREAMS] = {0};
>>> +    disable_dangling_plane(dc, context);
>>> +
>>>       for (i = 0; i < context->stream_count; i++)
>>>           dc_streams[i] =  context->streams[i];
>>>       if (!dcb->funcs->is_accelerated_mode(dcb))
>>>           dc->hwss.enable_accelerated_mode(dc);
>>> -
>>> -
>>>       for (i = 0; i < context->stream_count; i++) {
>>>           const struct dc_sink *sink = context->streams[i]->sink;
>>> @@ -864,8 +897,6 @@ static enum dc_status 
>>> dc_commit_state_no_check(struct dc *dc, struct dc_state *c
>>>               }
>>>           }
>>> -
>>> -
>>>           CONN_MSG_MODE(sink->link, "{%dx%d, %dx%d@%dKhz}",
>>> context->streams[i]->timing.h_addressable,
>>> context->streams[i]->timing.v_addressable,
>>