Hi,
I was not following this at first, so my apologies for
jumping in in the middle of the thread:
<snip>
>>>>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
>>>>> + unsigned long *state)
>>>>> +{
>>>>> + struct backlight_device *bd;
>>>>> +
>>>>> + if (!acpi_video_backlight_use_native())
>>>>> + return -ENODEV;
>>>>> +
>>>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> + if (!bd)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + *state = backlight_get_brightness(bd);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
>>>>> + unsigned long *state)
>>>>> +{
>>>>> + struct backlight_device *bd;
>>>>> +
>>>>> + if (!acpi_video_backlight_use_native())
>>>>> + return -ENODEV;
>>>>> +
>>>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> + if (!bd)
>>>>> + return -ENODEV;
>>>>> +
>>>>> + if (backlight_is_blank(bd))
>>>>> + *state = 0;
>>>>> + else
>>>>> + *state = bd->props.max_brightness;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>>>> + .get_max_state = amd_pmf_gpu_get_max_state,
>>>>> + .get_cur_state = amd_pmf_gpu_get_cur_state,
>>>>> +};
So first of all, good to see that this is using the
thermal_cooling_device APIs now, that is great thank you.
But the whole idea behind using the thermal_cooling_device APIs
is that amdgpu exports the cooling_device itself, rather then have
the AMD PMF code export it. Now the AMD PMF code is still poking
at the backlight_device itself, while the idea was to delegate
this to the GPU driver.
Actually seeing all the acpi_video_backlight_use_native()
checks here, I wonder why only have this work with native backlight
control. One step better would be to add thermal_cooling_device
support to the backlight core in:
drivers/video/backlight/backlight.c
Then it will work with any backlight control provider!
Last but not least this code MUST not call
acpi_video_backlight_use_native()
No code other then native GPU drivers must ever call
acpi_video_backlight_use_native(). This special function
not only checks if the native backlight control is the
one which the detection code in drivers/acpi/video_detect.c
has selected, it also signals to video_detect.c that
native GPU backlight control is available.
So by calling this in the AMD PMF code you are now
telling video_detect.c that native GPU backlight control
is available on all systems where AMD PMF runs.
As I already said I really believe the whole cooling
device should be registered somewhere else. But if you
do end up sticking with this then you MUST replace
the acpi_video_backlight_use_native() calls with:
if (acpi_video_get_backlight_type() == acpi_backlight_native) {...}
Regards,
Hans
