[PATCH v2] drm/ast: Fix start address computation

[Thomas Zimmermann]

Hi

Am 09.02.23 um 10:22 schrieb Jammy Huang:
> Hello,
> 
> The offset given to ast_set_start_address_crt1() should be offset in 
> vram. It should 0 now as your patch.
> 
> I think it is better to correct it in ast_primary_plane_init() and 
> ast_cursor_plane_init() as below.

I was about to write the same. Thanks for the review.

> 
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -714,7 +714,7 @@ static int ast_primary_plane_init(struct ast_private 
> *ast)
>          struct ast_plane *ast_primary_plane = &ast->primary_plane;
>          struct drm_plane *primary_plane = &ast_primary_plane->base;
>          void __iomem *vaddr = ast->vram;
> -       u64 offset = ast->vram_base;
> +       u64 offset = 0;
>          unsigned long cursor_size = roundup(AST_HWC_SIZE + 
> AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
>          unsigned long size = ast->vram_fb_available - cursor_size;
>          int ret;
> @@ -972,7 +972,7 @@ static int ast_cursor_plane_init(struct ast_private 
> *ast)
>                  return -ENOMEM;
> 
>          vaddr = ast->vram + ast->vram_fb_available - size;
> -       offset = ast->vram_base + ast->vram_fb_available - size;
> +       offset = st->vram_fb_available - size;

This is confusing me, because in my tests I still saw a cursor, even 
though the address is currently wrong.

Best regard
Thomas

> 
> On 2023/2/9 下午 05:12, Jocelyn Falempe wrote:
>> During the driver conversion to shmem, the start address for the
>> scanout buffer was set to the base PCI address.
>> In most cases it works because only the lower 24bits are used, and
>> due to alignment it was almost always 0.
>> But on some unlucky hardware, it's not the case, and some unitilized
>> memory is displayed on the BMC.
>> With shmem, the primary plane is always at offset 0 in GPU memory.
>>
>> Tested on a sr645 affected by this bug.
>>
>> Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
>> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
>> ---
>>   drivers/gpu/drm/ast/ast_mode.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index c7443317c747..54deb29bfeb3 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -681,7 +681,8 @@ static void 
>> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>>       if (!old_fb || old_fb->pitches[0] != fb->pitches[0])
>>           ast_set_offset_reg(ast, fb);
>>       if (!old_fb) {
>> -        ast_set_start_address_crt1(ast, (u32)ast_plane->offset);
>> +        /* with shmem, the primary plane is always at offset 0 */
>> +        ast_set_start_address_crt1(ast, 0);
>>           ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x00);
>>       }
>>   }
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230209/f0738335/attachment-0001.sig>