Re: [PATCH 1/3] drm/amdgpu: Add empty HDP flush function to JPEG v4.0.3

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 15.07.24 um 17:08 schrieb Lazar, Lijo:
>
> On 7/15/2024 8:28 PM, Christian König wrote:
>>
>> Am 15.07.24 um 16:47 schrieb Jane Jian:
>>> From: Lijo Lazar <lijo.lazar@amd.com>
>>>
>>> JPEG v4.0.3 doesn't support HDP flush when RRMT is enabled. Instead,
>>> mmsch fw will do the flush.
>>>
>>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>>> Signed-off-by: Jane Jian <Jane.Jian@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>>> b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>>> index 04d8966423de..ea601047dab0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
>>> @@ -621,6 +621,14 @@ static uint64_t
>>> jpeg_v4_0_3_dec_ring_get_wptr(struct amdgpu_ring *ring)
>>>                ring->pipe ? (0x40 * ring->pipe - 0xc80) : 0);
>>>    }
>>>    +static void jpeg_v4_0_3_ring_emit_hdp_flush(struct amdgpu_ring *ring)
>>> +{
>>> +    /* VCN engine access for HDP flush doesn't work when RRMT is
>>> enabled.
>>> +     * This is a workaround to avoid any HDP flush through VCN ring.
>>> Instead
>>> +     * HDP flush will be done by driver while submitting doorbell.
>> I think that should read "HDP flush will be done by firmware ....".
>>
>> Or is it really the driver which should do this? In this case the patch
>> here would be wrong.
>>
> That's a copy-paste mistake. This comment was originally in the initial
> version of the patch.

Yeah thought so.

> Discussed with Jane and she'll be sending a revised version. Also, there
> is a third patch expected which does normalization of register offsets
> when submitted through ring.

Another question is if we have ever released JPEG/VCN firmware which 
doesn't do the flush?

If yes then we need a version check here to provide backward 
compatibility with already released firmware.

Regards,
Christian.

>
> Thanks,
> Lijo
>
>> Regards,
>> Christian.
>>
>>> +     */
>>> +}
>>> +
>>>    /**
>>>     * jpeg_v4_0_3_dec_ring_set_wptr - set write pointer
>>>     *
>>> @@ -1072,6 +1080,7 @@ static const struct amdgpu_ring_funcs
>>> jpeg_v4_0_3_dec_ring_vm_funcs = {
>>>        .emit_ib = jpeg_v4_0_3_dec_ring_emit_ib,
>>>        .emit_fence = jpeg_v4_0_3_dec_ring_emit_fence,
>>>        .emit_vm_flush = jpeg_v4_0_3_dec_ring_emit_vm_flush,
>>> +    .emit_hdp_flush = jpeg_v4_0_3_ring_emit_hdp_flush,
>>>        .test_ring = amdgpu_jpeg_dec_ring_test_ring,
>>>        .test_ib = amdgpu_jpeg_dec_ring_test_ib,
>>>        .insert_nop = jpeg_v4_0_3_dec_ring_nop,