Re: [PATCH v6 5/5] drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x

["Maíra Canal" <mcanal@igalia.com>]

Hi Stefan,

On 27/03/25 07:57, Stefan Wahren wrote:
> Hi Maíra,
> 
> Am 18.03.25 um 02:01 schrieb Maíra Canal:
>> In addition to the standard reset controller, V3D 7.x requires 
>> configuring
>> the V3D_SMS registers for proper power on/off and reset. Add the new
>> registers to `v3d_regs.h` and ensure they are properly configured during
>> device probing, removal, and reset.
>>
>> This change fixes GPU reset issues on the Raspberry Pi 5 (BCM2712).
>> Without exposing these registers, a GPU reset causes the GPU to hang,
>> stopping any further job execution and freezing the desktop GUI. The same
>> issue occurs when unloading and loading the v3d driver.
>>
>> Link: https://github.com/raspberrypi/linux/issues/6660
>> Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   drivers/gpu/drm/v3d/v3d_drv.c  | 40 ++++++++++++++++++++++++++++++++ 
>> ++++++++
>>   drivers/gpu/drm/v3d/v3d_drv.h  | 11 +++++++++++
>>   drivers/gpu/drm/v3d/v3d_gem.c  | 17 +++++++++++++++++
>>   drivers/gpu/drm/v3d/v3d_regs.h | 26 ++++++++++++++++++++++++++
>>   4 files changed, 94 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/ 
>> v3d_drv.c
>> index 
>> c63f0ed1bd8a3d5511085e76ed2fbd6ee7df6f80..122848cdccc4a02039d9ea2e77aa2f377886b5d6 100644
>> --- a/drivers/gpu/drm/v3d/v3d_drv.c
>> +++ b/drivers/gpu/drm/v3d/v3d_drv.c
>> @@ -263,6 +263,36 @@ static const struct of_device_id v3d_of_match[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, v3d_of_match);
>>
>> +static void
>> +v3d_idle_sms(struct v3d_dev *v3d)
>> +{
>> +    if (v3d->ver < V3D_GEN_71)
>> +        return;
>> +
>> +    V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
>> +
>> +    if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
>> +                    V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
>> +        DRM_ERROR("Failed to power up SMS\n");
>> +    }
>> +
>> +    v3d_reset_sms(v3d);
>> +}
>> +
>> +static void
>> +v3d_power_off_sms(struct v3d_dev *v3d)
>> +{
>> +    if (v3d->ver < V3D_GEN_71)
>> +        return;
>> +
>> +    V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
>> +
>> +    if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
>> +                    V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
>> +        DRM_ERROR("Failed to power off SMS\n");
>> +    }
>> +}
>> +
>>   static int
>>   map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
>>   {
>> @@ -300,6 +330,12 @@ static int v3d_platform_drm_probe(struct 
>> platform_device *pdev)
>>       if (ret)
>>           return ret;
>>
>> +    if (v3d->ver >= V3D_GEN_71) {
>> +        ret = map_regs(v3d, &v3d->sms_regs, "sms");
>> +        if (ret)
>> +            return ret;
> Is it correct, that BCM2712 now requires the SMS register and otherwise
> the driver doesn't probe?

Hum, yeah, it is correct, but I can send a patch to ensure backwards
compatibility. But keep in mind that you won't be able to reset the GPU
or unload/load (power on/off).

> 
> Just a note for the future: the devicetree is considered as an ABI [1],
> so new kernels should still work with old DTB (no regression). For
> Raspberry Pi OS, the kernel and DTB are always updated, but this doesn't
> apply for Linux Mainline. AFAIK V3D doesn't work with Linux Mainline on
> Raspberry Pi 5 yet, so this is just a theoretical problem.

Yeah, I had that in mind, but due to the reason pointed in the end of
your comment, it was okay to do so and ensure that reset and power on/
off will work when RPi 5 reach upstream support.

But, I'm open to send a patch removing the requirement.

Best Regards,
- Maíra

> 
> Best regards
> 
> [1] - https://docs.kernel.org/devicetree/bindings/ABI.html