Re: [PATCH 1/2] drm/msm: provide fb_dirty implemenation

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 12/06/2023 11:21, Thomas Zimmermann wrote:
> Hi
> 
> Am 12.06.23 um 05:16 schrieb Dmitry Baryshkov:
>> Since commit 93e81e38e197 ("drm/fb_helper: Minimize damage-helper
>> overhead") the drm_fb_helper_funcs::fb_dirty helper is required for
>> proper dirty/damage processing. The drm/msm driver requires that to
>> function to let CMD panels to work. Use simplified version of
>> drm_fbdev_generic_helper_fb_dirty() to fix support for CMD mode panels.
>>
>> Reported-by: Degdag Mohamed <degdagmohamed@gmail.com>
>> Fixes: 93e81e38e197 ("drm/fb_helper: Minimize damage-helper overhead")
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> To make mmap work correctly, you'll also need deferred I/O in the fbdev 
> code. AFAICT the driver never supported that.

We do not use the deferred I/O. The damage/dirty tracking is used to 
check whether we should ping the CMD panel or not. See [1]

[1] https://lore.kernel.org/all/20220223191118.881321-1-robdclark@gmail.com/

> 
> Best regards
> Thomas
> 
>> ---
>>   drivers/gpu/drm/msm/msm_fbdev.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c 
>> b/drivers/gpu/drm/msm/msm_fbdev.c
>> index fa9c1cbffae3..b933a85420f6 100644
>> --- a/drivers/gpu/drm/msm/msm_fbdev.c
>> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
>> @@ -139,8 +139,28 @@ static int msm_fbdev_create(struct drm_fb_helper 
>> *helper,
>>       return ret;
>>   }
>> +static int msm_fbdev_fb_dirty(struct drm_fb_helper *helper,
>> +                  struct drm_clip_rect *clip)
>> +{
>> +    struct drm_device *dev = helper->dev;
>> +    int ret;
>> +
>> +    /* Call damage handlers only if necessary */
>> +    if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2))
>> +        return 0;
>> +
>> +    if (helper->fb->funcs->dirty) {
>> +        ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1);
>> +        if (drm_WARN_ONCE(dev, ret, "Dirty helper failed: ret=%d\n", 
>> ret))
>> +            return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
>>       .fb_probe = msm_fbdev_create,
>> +    .fb_dirty = msm_fbdev_fb_dirty,
>>   };
>>   /*
> 

-- 
With best wishes
Dmitry