Re: [PATCH 2/7] drm/msm/dpu: drop dpu_encoder_early_unregister

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 5/2/2023 1:54 PM, Dmitry Baryshkov wrote:
> On Tue, 2 May 2023 at 23:45, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote:
>>> There is no need to clean up debugfs manually, it will be done by the
>>> DRM core on device deregistration.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>
>> There are two reasons to have the debugfs removed in the early_unregister:
>>
>> 1) Today, registration happens in late_register(), hence to balance the
>> the call in _dpu_encoder_init_debugfs(), this one is present.
>>
>> 2) In drm_modeset_register_all(), if drm_connector_register_all() fails,
>> it calls drm_encoder_unregister_all() first which calls early_unregister().
>>
>> So to balance these out, dont we need to keep it?
> 
> Please correct me if I'm wrong, drm_debugfs_cleanup() should take care of that.
> 

Not from what I am seeing, drm_debugfs_cleanup() is getting called from 
the error handling path of drm_dev_register() and separately from 
drm_dev_unregister() but not from the error handling path of 
drm_modeset_register_all().

So that case will be unbalanced with this change.

>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 --------
>>>    1 file changed, 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 32785cb1b079..8c45c949ec39 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -2154,13 +2154,6 @@ static int dpu_encoder_late_register(struct drm_encoder *encoder)
>>>        return _dpu_encoder_init_debugfs(encoder);
>>>    }
>>>
>>> -static void dpu_encoder_early_unregister(struct drm_encoder *encoder)
>>> -{
>>> -     struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder);
>>> -
>>> -     debugfs_remove_recursive(dpu_enc->debugfs_root);
>>> -}
>>> -
>>>    static int dpu_encoder_virt_add_phys_encs(
>>>                struct msm_display_info *disp_info,
>>>                struct dpu_encoder_virt *dpu_enc,
>>> @@ -2374,7 +2367,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = {
>>>    static const struct drm_encoder_funcs dpu_encoder_funcs = {
>>>                .destroy = dpu_encoder_destroy,
>>>                .late_register = dpu_encoder_late_register,
>>> -             .early_unregister = dpu_encoder_early_unregister,
>>>    };
>>>
>>>    struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
>