| Message ID | 20231014105426.26389-2-sumitg@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for _TFP and change throttle pctg | expand |
On Sat, Oct 14, 2023 at 12:54 PM Sumit Gupta <sumitg@nvidia.com> wrote: > > From: Jeff Brasen <jbrasen@nvidia.com> > > Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling. > As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both > are present in a Thermal zone. > > [1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling > Period)" > > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> > Signed-off-by: Sumit Gupta <sumitg@nvidia.com> > --- > drivers/acpi/thermal.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index d98ff69303b3..a91e3d566858 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -90,7 +90,7 @@ struct acpi_thermal_passive { > struct acpi_thermal_trip trip; > unsigned long tc1; > unsigned long tc2; > - unsigned long tsp; > + unsigned long passive_delay; This is a passive trip structure anyway, so the "passive_" prefix is redundant here. "delay" alone would be fine. > }; > > struct acpi_thermal_active { > @@ -404,11 +404,16 @@ static bool passive_trip_params_init(struct acpi_thermal *tz) > > tz->trips.passive.tc2 = tmp; > > - status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); > - if (ACPI_FAILURE(status)) > - return false; > + status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp); > + if (ACPI_FAILURE(status)) { > + status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); > + if (ACPI_FAILURE(status)) > + return false; > > - tz->trips.passive.tsp = tmp; > + tz->trips.passive.passive_delay = tmp * 100; > + } else { > + tz->trips.passive.passive_delay = tmp; > + } I would prefer the if () statement above to be structured the other way around, that is status = ... if (ACPI_SUCCESS(status)) { tz->trips.passive.delay = tmp; return true; } status = ... if (ACPI_FAILURE(status)) return false; etc. > > return true; > } > @@ -904,7 +909,7 @@ static int acpi_thermal_add(struct acpi_device *device) > > acpi_trip = &tz->trips.passive.trip; > if (acpi_thermal_trip_valid(acpi_trip)) { > - passive_delay = tz->trips.passive.tsp * 100; > + passive_delay = tz->trips.passive.passive_delay; > > trip->type = THERMAL_TRIP_PASSIVE; > trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk); > -- > 2.17.1 >
On 18/10/23 17:18, Rafael J. Wysocki wrote: > External email: Use caution opening links or attachments > > > On Sat, Oct 14, 2023 at 12:54 PM Sumit Gupta <sumitg@nvidia.com> wrote: >> >> From: Jeff Brasen <jbrasen@nvidia.com> >> >> Add support of "Thermal fast Sampling Period (_TFP)" for Passive cooling. >> As per [1], _TFP overrides the "Thermal Sampling Period (_TSP)" if both >> are present in a Thermal zone. >> >> [1] ACPI Specification 6.4 - section 11.4.17. _TFP (Thermal fast Sampling >> Period)" >> >> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com> >> Signed-off-by: Sumit Gupta <sumitg@nvidia.com> >> --- >> drivers/acpi/thermal.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c >> index d98ff69303b3..a91e3d566858 100644 >> --- a/drivers/acpi/thermal.c >> +++ b/drivers/acpi/thermal.c >> @@ -90,7 +90,7 @@ struct acpi_thermal_passive { >> struct acpi_thermal_trip trip; >> unsigned long tc1; >> unsigned long tc2; >> - unsigned long tsp; >> + unsigned long passive_delay; > > This is a passive trip structure anyway, so the "passive_" prefix is > redundant here. "delay" alone would be fine. > will change in v6. >> }; >> >> struct acpi_thermal_active { >> @@ -404,11 +404,16 @@ static bool passive_trip_params_init(struct acpi_thermal *tz) >> >> tz->trips.passive.tc2 = tmp; >> >> - status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); >> - if (ACPI_FAILURE(status)) >> - return false; >> + status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp); >> + if (ACPI_FAILURE(status)) { >> + status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); >> + if (ACPI_FAILURE(status)) >> + return false; >> >> - tz->trips.passive.tsp = tmp; >> + tz->trips.passive.passive_delay = tmp * 100; >> + } else { >> + tz->trips.passive.passive_delay = tmp; >> + } > > I would prefer the if () statement above to be structured the other > way around, that is > > status = ... > if (ACPI_SUCCESS(status)) { > tz->trips.passive.delay = tmp; > return true; > } > > status = ... > if (ACPI_FAILURE(status)) > return false; > > etc. > Ok. will change in v6. >> >> return true; >> } >> @@ -904,7 +909,7 @@ static int acpi_thermal_add(struct acpi_device *device) >> >> acpi_trip = &tz->trips.passive.trip; >> if (acpi_thermal_trip_valid(acpi_trip)) { >> - passive_delay = tz->trips.passive.tsp * 100; >> + passive_delay = tz->trips.passive.passive_delay; >> >> trip->type = THERMAL_TRIP_PASSIVE; >> trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk); >> -- >> 2.17.1 >>
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index d98ff69303b3..a91e3d566858 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -90,7 +90,7 @@ struct acpi_thermal_passive { struct acpi_thermal_trip trip; unsigned long tc1; unsigned long tc2; - unsigned long tsp; + unsigned long passive_delay; }; struct acpi_thermal_active { @@ -404,11 +404,16 @@ static bool passive_trip_params_init(struct acpi_thermal *tz) tz->trips.passive.tc2 = tmp; - status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); - if (ACPI_FAILURE(status)) - return false; + status = acpi_evaluate_integer(tz->device->handle, "_TFP", NULL, &tmp); + if (ACPI_FAILURE(status)) { + status = acpi_evaluate_integer(tz->device->handle, "_TSP", NULL, &tmp); + if (ACPI_FAILURE(status)) + return false; - tz->trips.passive.tsp = tmp; + tz->trips.passive.passive_delay = tmp * 100; + } else { + tz->trips.passive.passive_delay = tmp; + } return true; } @@ -904,7 +909,7 @@ static int acpi_thermal_add(struct acpi_device *device) acpi_trip = &tz->trips.passive.trip; if (acpi_thermal_trip_valid(acpi_trip)) { - passive_delay = tz->trips.passive.tsp * 100; + passive_delay = tz->trips.passive.passive_delay; trip->type = THERMAL_TRIP_PASSIVE; trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);