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

[Jocelyn Falempe]

On 09/02/2023 10:22, Jammy Huang wrote:
> 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.

ok, thanks for the review, I will send a v3 with this suggestion.

-- 

Jocelyn

> 
> --- 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;
> 
> 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);
>>       }
>>   }
>