Re: [PATCH 8/8] drm/ast: Avoid reprogramming primary-plane scanout address

[Jocelyn Falempe <jfalempe@redhat.com>]

On 11/10/2022 16:59, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.10.22 um 16:21 schrieb Jocelyn Falempe:
>> On 10/10/2022 12:36, Thomas Zimmermann wrote:
>>> Some AST-based BMCs stop display output for up to 5 seconds after
>>> reprogramming the scanout address. As the address is fixed, avoid
>>> re-setting the address' value.
>>>
>>> Reported-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/gpu/drm/ast/ast_mode.c | 12 ++++++++++--
>>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>>> b/drivers/gpu/drm/ast/ast_mode.c
>>> index 1b991658290b..54a9643d86ce 100644
>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>> @@ -672,9 +672,17 @@ static void 
>>> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>>       }
>>>       ast_set_offset_reg(ast, fb);
>>> -    ast_set_start_address_crt1(ast, (u32)ast_plane->offset);
>>> -    ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x00);
>>> +    /*
>>> +     * Some BMCs stop scanning out the video signal after the driver
>>> +     * reprogrammed the scanout address. This stalls display output
>>> +     * for several seconds and makes the display unusable. Therefore
>>> +     * only reprogram the address after enabling the plane.
>>> +     */
>>> +    if (!old_fb && fb) {
>>> +        ast_set_start_address_crt1(ast, (u32)ast_plane->offset);
>>> +        ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x00);
>>> +    }
>>>   }
>>
>> I've tested the series, and BMC is still very slow with Gnome/Wayland.
>>
>> It's because ast_set_offset_reg() also trigger a 5s freeze of the BMC.
>>
>> I added this, and it works well:
>>
>> if (!old_fb || old_fb->pitches[0] != fb->pitches[0])
>>      ast_set_offset_reg(ast, fb);
> 
> Great thanks for testing. I'll add this to the next version.
> 
> I wonder if that problem is in all ast chips or just this one. :/

My test machine has an AST2600, but I think it depends mostly on the BMC 
firmware.
Anyway it's not useful to reprogram the same value on the registers for 
each frame. And that's the good thing about atomic interface, because we 
have previous and next state, it's easy to check if the registers need 
to be updated.

> 
> Best regards
> Thomas
> 
>>
>>
>>>   static void ast_primary_plane_helper_atomic_disable(struct 
>>> drm_plane *plane,
>>
>>
>>
>>
> 

-- 

Jocelyn