Use devm_thermal_of_cooling_device_register() to register the cooling
device. As a side effect, this fixes a driver bug:
thermal_cooling_device_unregister() was not called on device removal.
Fixes: f1fd4a4db7 ("hwmon: Add NPCM7xx PWM and Fan driver")
Cc: Tomer Maimon <tmaimon77@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Call devm_thermal_of_cooling_device_register() to register the cooling
device. Also introduce struct device *dev = &pdev->dev; to make the code
easier to read.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Call devm_thermal_of_cooling_device_register() to register the cooling
device. Also use devm_add_action_or_reset() to stop the fan on device
removal. This fixes a race condition since the fan was stopped before
the hwmon device was removed.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Use devm_thermal_of_cooling_device_register() to register the cooling
device. As a side effect, this fixes a driver bug:
thermal_cooling_device_unregister() was not called on removal.
Fixes: f198907d2f ("hwmon: (aspeed-pwm-tacho) cooling device support.")
Cc: Mykola Kostenok <c_mykolak@mellanox.com>
Cc: Joel Stanley <joel@jms.id.au>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Patrick Venture <venture@google.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Currently after store trip points number in 'ret', it is overwritten
afterwards, this cause incorrect trip point number always be shown in
the debug information after register of each thermal zone.
This patch fix this issue by moving get of trip number to
end of thermal zone registration.
Fixes: 6269e9f790 ("thermal: rcar_gen3_thermal: Register hwmon sysfs interface")
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Update calculation for the R-Car Gen3 and RZ/G2 SoCs which have a
thermal IP block controlled by this driver. That is the:
* R-Car D3 (r8a77995)
* R-Car E2 (r8a77990)
* R-Car V3M (r8a77970)
* RZ/G2E (r8a774c0)
The calculation update is as documented in the R-Car Gen3 User's Manual,
v1.50 Nov 2018:
- When CTEMP is less than 24
T = CTEMP[5:0] * 5.5 - 72
- When CTEMP is equal to/greater than 24
T = CTEMP[5:0] * 5 - 60
This was inspired by a patch in the BSP by Van Do <van.do.xw@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
Tested-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The CPU load values passed to the thermal_power_cpu_get_power
tracepoint are zero for all CPUs, unless, unless the
thermal_power_cpu_limit tracepoint is enabled too:
irq/41-rockchip-98 [000] .... 290.972410: thermal_power_cpu_get_power:
cpus=0000000f freq=1800000 load={{0x0,0x0,0x0,0x0}} dynamic_power=4815
vs
irq/41-rockchip-96 [000] .... 95.773585: thermal_power_cpu_get_power:
cpus=0000000f freq=1800000 load={{0x56,0x64,0x64,0x5e}} dynamic_power=4959
irq/41-rockchip-96 [000] .... 95.773596: thermal_power_cpu_limit:
cpus=0000000f freq=408000 cdev_state=10 power=416
There seems to be no good reason for omitting the CPU load information
depending on another tracepoint. My guess is that the intention was to
check whether thermal_power_cpu_get_power is (still) enabled, however
'load_cpu != NULL' already indicates that it was at least enabled when
cpufreq_get_requested_power() was entered, there seems little gain
from omitting the assignment if the tracepoint was just disabled, so
just remove the check.
Fixes: 6828a4711f ("thermal: add trace events to the power allocator governor")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Javi Merino <javi.merino@kernel.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
PX30 SOC has two Temperature Sensors for CPU and GPU.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Add a new compatible for thermal founding on PX30 SoCs.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Explicitly use the pinctrl to set/unset the right mode
instead of relying on the pinctrl init mode.
And it requires setting the tshut polarity before select pinctrl.
When the temperature sensor mode is set to 0, it will automatically
reset the board via the Clock-Reset-Unit (CRU) if the over temperature
threshold is reached. However, when the pinctrl initializes, it does a
transition to "otp_out" which may lead the SoC restart all the time.
"otp_out" IO may be connected to the RESET circuit on the hardware.
If the IO is in the wrong state, it will trigger RESET.
(similar to the effect of pressing the RESET button)
which will cause the soc to restart all the time.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Unlike DT framework, thermal-zones and its parameters can't be parsed
using ACPI framework. So that ACPI support is removed in this driver.
Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Reported-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The devres.o gets linked if HAS_IOMEM is present so on ARCH=um
allyesconfig (COMPILE_TEST) failed on many files with:
drivers/thermal/thermal_mmio.o:
In function 'thermal_mmio_probe':thermal_mmio.c:(.text+0xe1):
undefined reference to `devm_ioremap_resource'
The users of devm_ioremap_resource() which are compile-testable
should depend on HAS_IOMEM.
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The structure cpufreq_cooling_device provides a backpointer to the thermal
device but this one is used for a trace and to unregister. For the trace,
we don't really need this field and the unregister function as the same
pointer passed as parameter. Remove it.
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The copyright format does not conform to the format requested by
Linaro: https://wiki.linaro.org/Copyright
Fix it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
When the static power computation was removed, the test with the power
being negative was not removed. However, the substraction which was
responsible of the negative value was removed and the variable is now
an u32. A double reason to remove the test which does not make sense.
Fixes: 84fe2cab48 ("cpu_cooling: Drop static-power related stuff")
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Currently IRQ remains enabled after .remove, later if device is probed,
IRQ is requested before .thermal_init, this may cause IRQ function be
called before device is initialized.
this patch disables interrupt in .remove, to ensure irq function
only be called after device is fully initialized.
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Currently IRQF_SHARED type interrupt line is allocated, but it
is not appropriate, as the interrupt line isn't shared between
different devices, instead IRQF_ONESHOT is the proper type.
By changing interrupt type to IRQF_ONESHOT, now irq handler is
no longer needed, as clear of interrupt status can be done in
threaded interrupt context.
Because IRQF_ONESHOT type interrupt line is kept disabled until
the threaded handler has been run, so there is no need to protect
read/write of REG_GEN3_IRQSTR with lock.
Fixes: 7d4b269776 ("enable hardware interrupts for trip points")
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Tested-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
thermal_of_cooling_device_register() and thermal_cooling_device_register()
are typically called from driver probe functions, and
thermal_cooling_device_unregister() is called from remove functions. This
makes both a perfect candidate for device managed functions.
Introduce devm_thermal_of_cooling_device_register(). This function can
also be used to replace thermal_cooling_device_register() by passing a NULL
pointer as device node. The new function requires both struct device *
and struct device_node * as parameters since the struct device_node *
parameter is not always identical to dev->of_node.
Don't introduce a device managed remove function since it is not needed
at this point.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Certain ADC channels, such as the xilinx-ams temperature channels, give
milliCelcius already when read with iio_read_channel_processed.
Rather than having to provide a 1:1 dummy lookup table, simply allow to
bypass the mechanism.
Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
It's impossible to use this driver outside of Device Tree, so if the
probe function is called, the dev.of_node is guaranteed to not be NULL
and guarding against that is pointless. Drop it.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
This is a generic thermal driver for simple MMIO sensors, of which
amazon,al-thermal is one.
This device uses a single MMIO transaction to read the temperature and
report it to the thermal subsystem.
Signed-off-by: Talel Shenhar <talel@amazon.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Fix setting value for IRQCTL register. We are setting the last 6 bits
of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
to Hardware manual values 1 are "setting prohibited" for Gen3.
Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
This will allow calibration routines to correctly include the constants
from anywhere and allow more code sharing.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
qcs404 has a single TSENS IP block with 10 sensors. It uses version 1.4
of the TSENS IP, functionality for which is encapsulated inside the
qcom,tsens-v1 compatible.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
qcs404 uses v1 of the TSENS IP block. Create a fallback DT property
"qcom,tsens-v1" to gather common code
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
get_temp() learns to return temperature regardless of whether it is
returned as ADC code or direct temperature.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Just rename the function and move it to allow code sharing with future
versions of TSENS IP
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The current implementation is based on an algorithm published in the
docs. Instead of reading the temperature thrice w/o any explanation,
improve the algorithm.
This will become the basis for a common get_temp routine in the future.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The IP can support 'm' sensors while the platform can enable 'n' sensors
of the 'm' where n <= m.
Track maximum sensors supported by the IP so that we can correctly track
what subset of the sensors are supported on the platform.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The IDs cannot be negative, fix the data type.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
is_sensor_enabled() checks if the sensors are enabled on this platform.
It is possible that the SoC might choose not to enable all the sensors
that the IP block is capable of supporting.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
We print a calibration failure message on -EPROBE_DEFER from
nvmem/qfprom as follows:
[ 3.003090] qcom-tsens 4a9000.thermal-sensor: version: 1.4
[ 3.005376] qcom-tsens 4a9000.thermal-sensor: tsens calibration failed
[ 3.113248] qcom-tsens 4a9000.thermal-sensor: version: 1.4
This confuses people when, in fact, calibration succeeds later when
nvmem/qfprom device is available. Don't print this message on a
-EPROBE_DEFER.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
As we add support for newer versions of the TSENS IP, the current
approach isn't scaling because registers and bitfields get moved around,
requiring platform-specific hacks in the code. By moving to regmap, we
can hide the register level differences away from the code.
Define a common set of registers and bit-fields that we care about
across the various tsens IP versions.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Mark completion EQs as shared resources so that they can be used by CQs
with uid != 0.
Fixes: 7efce3691d ("IB/mlx5: Add obj create and destroy functionality")
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
As the obj_id in the firmware is not globally unique in general_object,
the object type must be considered upon checking for a valid object id.
Fixes: 2351776e87 ("IB/mlx5: Verify DEVX object type")
Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
flush_hash_pages() runs with data translation off, so current
task_struct has to be accesssed using physical address.
Fixes: f7354ccac8 ("powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU")
Cc: stable@vger.kernel.org # v5.1+
Reported-by: Erhard F. <erhard_f@mailbox.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
The strncpy() function may leave the destination string buffer
unterminated, better use strscpy() instead.
This fixes the following warning with gcc 8.2:
kernel/debug/kdb/kdb_io.c: In function 'kdb_getstr':
kernel/debug/kdb/kdb_io.c:449:3: warning: 'strncpy' specified bound 256 equals destination size [-Wstringop-truncation]
strncpy(kdb_prompt_str, prompt, CMD_BUFLEN);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
When dumping the page table in response to an unexpected kernel page
fault, we print the virtual (hashed) address of the page table base, but
display physical addresses for everything else.
Make the page table dumping code in show_pte() consistent, by printing
the page table base pointer as a physical address.
Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Add an assert that we don't use TypeC ports for eDP. That may in theory
be possible on TypeC legacy ports, but I'm not sure if that's a
practical scenario, so let's deal with that only if there's a use case.
Adding support for that wouldn't be too difficult, since TypeC mode
switching is not possible on TypeC legacy ports.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-12-imre.deak@intel.com
On ICL we have to make sure that we enable the AUX power domain in a
controlled way (corresponding to the port's actual TypeC mode). Since
the PPS lock - which takes an AUX power ref - is only needed on
eDP on all platforms and eDP/DP on VLV/CHV avoid taking it in all
other cases.
v2:
- Clarify commit log about the condition for taking the PPS lock.
(Ville)
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-11-imre.deak@intel.com
There isn't a separate power domain specific to PLLs. When programming
them we require the same power domain to be enabled which is needed when
accessing other display core parts (not specific to any
pipe/port/transcoder). This corresponds to the DISPLAY_CORE domain added
previously in this patchset, so use that instead to save bits in the
power domain mask.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-10-imre.deak@intel.com
The power get/put was added in
commit 1c767b339b ("drm/i915: take display port power domain in DP HPD handler")
Author: Imre Deak <imre.deak@intel.com>
Date: Mon Aug 18 14:42:42 2014 +0300
to account for the HW access in ibx_digital_port_connected(). This
latter call was in turn removed in
commit 7d23e3c37b ("drm/i915: Cleaning up intel_dp_hpd_pulse")
Author: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
Date: Wed Mar 30 18:05:23 2016 +0530
after which we didn't actually need the power reference.
One way we are accessing the HW during HPD pulse handling is via DP AUX
transfers, but the transfer function takes its own reference, so doesn't
need the reference in intel_dp_hpd_pulse().
The other spot is in
intel_psr_short_pulse()->intel_psr_disable_locked()
but that can only happen when the panel is enabled with the
corresponding modeset already holding the required power reference.
v2:
- Remove the unneeded power get/put from intel_psr_disable_locked().
(Ville)
- Checkpatch commit quoting format fix in the commit log.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-9-imre.deak@intel.com
We don't need the AUX power for the whole duration of the detect, only
when we're doing AUX transfers. The AUX transfer function takes its own
reference on the AUX power domain already. The two places during detect
which access display core registers (not specific to a
pipe/port/transcoder) only need the power domain that is required for
that access. That power domain is equivalent to the device global power
domain on most platforms (enabled whenever we hold a runtime PM
reference) except on CHV/VLV where it's equivalent to the display power
well.
Add a new power domain that reflects the above, and use this at the two
spots accessing registers. With that we can avoid taking the AUX
reference for the whole duration of the detect function.
Put the domains asynchronously to avoid the unneeded on-off-on toggling.
Also adapt the idea from with_intel_runtime_pm et al. for making it easy
to write short sequences where a display power ref is needed.
v2: (Ville)
- Add with_intel_display_power() helper to simplify things.
- s/bool res/bool is_connected/
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-8-imre.deak@intel.com
In a follow-up patch we will restrict holding the reference on the AUX
power domain to the AUX transfer function. To avoid the unnecessary
on-off-on power togglings drop the reference asynchronously.
There is no reason we couldn't do this in general and also put the
reference asynchronously in pps_unlock(); but that's a separate change
that can be done as a follow-up.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-6-imre.deak@intel.com
By disabling a power domain asynchronously we can restrict holding a
reference on that power domain to the actual code sequence that
requires the power to be on for the HW access it's doing, by also
avoiding unneeded on-off-on togglings of the power domain (since the
disabling happens with a delay).
One benefit is potential power saving due to the following two reasons:
1. The fact that we will now be holding the reference only for the
necessary duration by the end of the patchset. While simply not
delaying the disabling has the same benefit, it has the problem that
frequent on-off-on power switching has its own power cost (see the 2.
point below) and the debug trace for power well on/off events will
cause a lot of dmesg spam (see details about this further below).
2. Avoiding the power cost of freuqent on-off-on power switching. This
requires us to find the optimal disabling delay based on the measured
power cost of on->off and off->on switching of each power well vs.
the power of keeping the given power well on.
In this patchset I'm not providing this optimal delay for two
reasons:
a) I don't have the means yet to perform the measurement (with high
enough signal-to-noise ratio, or with the help of an energy
counter that takes switching into account). I'm currently looking
for a way to measure this.
b) Before reducing the disabling delay we need an alternative way for
debug tracing powerwell on/off events. Simply avoiding/throttling
the debug messages is not a solution, see further below.
Note that even in the case where we can't measure any considerable
power cost of frequent on-off switching of powerwells, it still would
make sense to do the disabling asynchronously (with 0 delay) to avoid
blocking on the disabling. On VLV I measured this disabling time
overhead to be 1ms on average with a worst case of 4ms.
In the case of the AUX power domains on ICL we would also need to keep
the sequence where we hold the power reference short, the way it would
be by the end of this patchset where we hold it only for the actual AUX
transfer. Anything else would make the locking we need for ICL TypeC
ports (whenever we hold a reference on any AUX power domain) rather
problematic, adding for instance unnecessary lockdep dependencies to
the required TypeC port lock.
I chose the disabling delay to be 100msec for now to avoid the unneeded
toggling (and so not to introduce dmesg spamming) in the DP MST sideband
signaling code. We could optimize this delay later, once we have the
means to measure the switching power cost (see above).
Note that simply removing/throttling the debug tracing for power well
on/off events is not a solution. We need to know the exact spots of
these events and cannot rely only on incorrect register accesses caught
(due to not holding a wakeref at the time of access). Incorrect
powerwell enabling/disabling could lead to other problems, for instance
we need to keep certain powerwells enabled for the duration of modesets
and AUX transfers.
v2:
- Clarify the commit log parts about power cost measurement and the
problem of simply removing/throttling debug tracing. (Chris)
- Optimize out local wakeref vars at intel_runtime_pm_put_raw() and
intel_display_power_put_async() call sites if
CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n. (Chris)
- Rebased on v2 of the wakeref w/o power-on guarantee patch.
- Add missing docbook headers.
v3:
- Checkpatch spelling/missing-empty-line fix.
v4:
- Fix unintended local wakeref var optimization when using
call-arguments with side-effects, by using inline funcs instead of
macros. In this patch in particular this will fix the
intel_display_power_grab_async_put_ref()->intel_runtime_pm_put_raw()
call).
No size change in practice (would be the same disregarding the
corresponding change in intel_display_power_grab_async_put_ref()):
$ size i915-macro.ko
text data bss dec hex filename
2455190 105890 10272 2571352 273c58 i915-macro.ko
$ size i915-inline.ko
text data bss dec hex filename
2455195 105890 10272 2571357 273c5d i915-inline.ko
Kudos to Stan for reporting the raw-wakeref WARNs this issue caused. His
config has CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n, which I didn't retest
after v1, and we are also not testing this config in CI.
Now tested both with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y/n on ICL,
connecting both Chamelium and regular DP, HDMI sinks.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190513192533.12586-1-imre.deak@intel.com
