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

[Dmitry Baryshkov]

On 02/05/2023 23:59, Abhinav Kumar wrote:
> 
> 
> On 5/2/2023 1:54 PM, Dmitry Baryshkov wrote:
>> On Tue, 2 May 2023 at 23:45, Abhinav Kumar <quic_abhinavk at 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 at 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.

But for the practical reasons the drm_modeset_register_all() can not 
fail. But I see your point. I'll check why drm_dev_register() doesn't 
respect an error return from drm_modeset_register_all(). Until that 
point we can drop this patch.

> 
>>>>    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,
>>

-- 
With best wishes
Dmitry