diff mbox series

[v3] drm/ast: Fix start address computation

Message ID 20230209094417.21630-1-jfalempe@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3] drm/ast: Fix start address computation | expand

Commit Message

Jocelyn Falempe Feb. 9, 2023, 9:44 a.m. UTC
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.

 * v2: rewrite the patch to set the offset to 0. (Thomas Zimmermann)
 * v3: move the change to plane_init() and also fix the cursor plane.
       (Jammy Huang)

Tested on a sr645 affected by this bug.

Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/ast/ast_mode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Zimmermann Feb. 9, 2023, 9:55 a.m. UTC | #1
Am 09.02.23 um 10:44 schrieb Jocelyn Falempe:
> 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

'uninitialized'

> memory is displayed on the BMC.
> With shmem, the primary plane is always at offset 0 in GPU memory.
> 
>   * v2: rewrite the patch to set the offset to 0. (Thomas Zimmermann)
>   * v3: move the change to plane_init() and also fix the cursor plane.
>         (Jammy Huang)
> 
> Tested on a sr645 affected by this bug.
> 
> Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/ast/ast_mode.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index c7443317c747..66a4a41c3fe9 100644
> --- 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; /* with shmem, the primary plane is always at 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 = ast->vram_fb_available - size;
>   
>   	ret = ast_plane_init(dev, ast_cursor_plane, vaddr, offset, size,
>   			     0x01, &ast_cursor_plane_funcs,
Jammy Huang Feb. 10, 2023, 12:33 a.m. UTC | #2
On 2023/2/9 下午 05:55, Thomas Zimmermann wrote:
>
>
> Am 09.02.23 um 10:44 schrieb Jocelyn Falempe:
>> 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
>
> 'uninitialized'
>
>> memory is displayed on the BMC.
>> With shmem, the primary plane is always at offset 0 in GPU memory.
>>
>>   * v2: rewrite the patch to set the offset to 0. (Thomas Zimmermann)
>>   * v3: move the change to plane_init() and also fix the cursor plane.
>>         (Jammy Huang)
>>
>> Tested on a sr645 affected by this bug.
>>
>> Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Jammy Huang <jammy_huang@aspeedtech.com>

>
>> ---
>>   drivers/gpu/drm/ast/ast_mode.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index c7443317c747..66a4a41c3fe9 100644
>> --- 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; /* with shmem, the primary plane is always at 
>> 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 = ast->vram_fb_available - size;
>>         ret = ast_plane_init(dev, ast_cursor_plane, vaddr, offset, size,
>>                    0x01, &ast_cursor_plane_funcs,
>
Jocelyn Falempe Feb. 10, 2023, 3:02 p.m. UTC | #3
On 10/02/2023 01:33, Jammy Huang wrote:
> 
> On 2023/2/9 下午 05:55, Thomas Zimmermann wrote:
>>
>>
>> Am 09.02.23 um 10:44 schrieb Jocelyn Falempe:
>>> 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
>>
>> 'uninitialized'
>>
>>> memory is displayed on the BMC.
>>> With shmem, the primary plane is always at offset 0 in GPU memory.
>>>
>>>   * v2: rewrite the patch to set the offset to 0. (Thomas Zimmermann)
>>>   * v3: move the change to plane_init() and also fix the cursor plane.
>>>         (Jammy Huang)
>>>
>>> Tested on a sr645 affected by this bug.
>>>
>>> Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Reviewed-by: Jammy Huang <jammy_huang@aspeedtech.com>

I just pushed it to drm-misc-fixes.

Thanks a lot,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c7443317c747..66a4a41c3fe9 100644
--- 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; /* with shmem, the primary plane is always at 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 = ast->vram_fb_available - size;
 
 	ret = ast_plane_init(dev, ast_cursor_plane, vaddr, offset, size,
 			     0x01, &ast_cursor_plane_funcs,