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@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
>>> Shyam Sundar S K
>>> Sent: Friday, September 22, 2023 1:51 PM
>>> To: hdegoede@xxxxxxxxxx; markgross@xxxxxxxxxx; Natikar, Basavaraj
>>> <Basavaraj.Natikar@xxxxxxx>; jikos@xxxxxxxxxx;
>>> benjamin.tissoires@xxxxxxxxxx; Deucher, Alexander
>>> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
>>> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>;
>>> airlied@xxxxxxxxx; daniel@xxxxxxxx
>>> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@xxxxxxx>; amd-
>>> gfx@xxxxxxxxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx; dri-
>>> devel@xxxxxxxxxxxxxxxxxxxxx; Patil Rajesh <Patil.Reddy@xxxxxxx>;
>>> linux-
>>> input@xxxxxxxxxxxxxxx; Limonciello, Mario <Mario.Limonciello@xxxxxxx>
>>> 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@xxxxxxx>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
>>> ---
>>> 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
>>
>
