Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 9/25/2023 10:00 PM, Mario Limonciello wrote:
> On 9/25/2023 11:27, Deucher, Alexander wrote:
>> [Public]
>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>> Shyam Sundar S K
>>> Sent: Friday, September 22, 2023 1:51 PM
>>> To: hdegoede@redhat.com; markgross@kernel.org; Natikar, Basavaraj
>>> <Basavaraj.Natikar@amd.com>; jikos@kernel.org;
>>> benjamin.tissoires@redhat.com; Deucher, Alexander
>>> <Alexander.Deucher@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
>>> airlied@gmail.com; daniel@ffwll.ch
>>> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>; amd-
>>> gfx@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; Patil Rajesh <Patil.Reddy@amd.com>;
>>> linux-
>>> input@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
>>> Subject: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set
>>> interface
>>>
>>> For the Smart PC Solution to fully work, it has to enact to the
>>> actions coming
>>> from TA. Add the initial code path for set interface to AMDGPU.
>>
>> This seems to be limited to backlight at this point.  What does
>> setting or not setting the backlight level mean for the system when
>> this framework is in place?  What if a user manually changes the
>> backlight level?  Additional comments below.
>>
> 
> It's also for the display count.

display count is on the PMF-GPU GET interface. On the SET interface
its only backlight for today.

Thanks,
Shyam

> 
>>>
>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21
>>> +++++++++++++++++++++
>>>   drivers/platform/x86/amd/pmf/pmf.h      |  2 ++
>>>   drivers/platform/x86/amd/pmf/tee-if.c   | 19 +++++++++++++++++--
>>>   include/linux/amd-pmf-io.h              |  1 +
>>>   4 files changed, 41 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> index 232d11833ddc..5c567bff0548 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data
>>> *pmf)
>>>        return 0;
>>>   }
>>>   EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>> +
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf) {
>>> +     struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>> +     struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>> +     struct backlight_device *bd;
>>> +
>>> +     if (!(adev->flags & AMD_IS_APU)) {
>>> +             DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>> +             return -ENODEV;
>>> +     }
>>> +
>>> +     bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> +     if (!bd)
>>> +             return -ENODEV;
>>> +
>>> +     backlight_device_set_brightness(bd, pmf->brightness);
>>> +
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -73,6 +73,7 @@
>>>   #define PMF_POLICY_STT_SKINTEMP_APU                          7
>>>   #define PMF_POLICY_STT_SKINTEMP_HS2                          8
>>>   #define
>>> PMF_POLICY_SYSTEM_STATE                                      9
>>> +#define
>>> PMF_POLICY_DISPLAY_BRIGHTNESS                                12
>>>   #define
>>> PMF_POLICY_P3T                                               38
>>>
>>>   /* TA macros */
>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {  };
>>>
>>>   struct pmf_action_table {
>>> +     unsigned long display_brightness;
>>>        enum system_state system_state;
>>>        unsigned long spl; /* in mW */
>>>        unsigned long sppt; /* in mW */
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>> index 1608996654e8..eefffff83a4c 100644
>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
>>> amd_pmf_dev *dev, u16 event)
>>>        return 0;
>>>   }
>>>
>>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>>> ta_pmf_enact_result *out)
>>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>>> +ta_pmf_enact_result *out)
>>>   {
>>>        u32 val, event = 0;
>>> -     int idx;
>>> +     int idx, ret;
>>>
>>>        for (idx = 0; idx < out->actions_count; idx++) {
>>>                val = out->actions_list[idx].value;
>>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>                                dev->prev_data->system_state = 0;
>>>                        }
>>>                        break;
>>> +
>>> +             case PMF_POLICY_DISPLAY_BRIGHTNESS:
>>> +                     ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>>> +                     if (ret)
>>> +                             return ret;
>>> +
>>> +                     dev->prev_data->display_brightness = dev-
>>>> gfx_data.brightness;
>>
>> Are we using standardized units for the brightness?  On the GPU
>> side, we align with the standard blacklight interface, but
>> internally, the driver has to convert units depending on the type of
>> backlight controller used on the platform.  Presumably PMF does
>> something similar?
>>
>> Alex
>>
>>> +                     if (dev->prev_data->display_brightness != val) {
>>> +                             dev->gfx_data.brightness = val;
>>> +                             amd_pmf_set_gfx_data(&dev->gfx_data);
>>> +                             dev_dbg(dev->dev, "update
>>> DISPLAY_BRIGHTNESS : %d\n", val);
>>> +                     }
>>> +                     break;
>>>                }
>>>        }
>>> +
>>> +     return 0;
>>>   }
>>>
>>>   static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) diff
>>> --git
>>> a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h index
>>> a2d4af231362..ecae387ddaa6 100644
>>> --- a/include/linux/amd-pmf-io.h
>>> +++ b/include/linux/amd-pmf-io.h
>>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {  };
>>>
>>>   int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>   #endif
>>> -- 
>>> 2.25.1
>>
>