* [PATCH 01/38] dt-bindings: interrupt-controller: arm,gic: Add compatible for Tegra186 AGIC
From: Thierry Reding @ 2020-06-12 14:18 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200612141903.2391044-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
The AGIC variant of GIC found in the audio subsystem of NVIDIA Tegra186
SoCs is mostly compatible with the version found on Tegra210. Provide a
Tegra210 compatible as a fallback compatible, but also require a
Tegra186 specific compatible for forwards compatibility in case there's
ever a need to implement instantiation-specific quirks.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/interrupt-controller/arm,gic.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
index 96f8803ff4e6..becc4364e419 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
@@ -53,6 +53,13 @@ properties:
- const: brcm,brahma-b15-gic
- const: arm,cortex-a15-gic
+ - items:
+ - enum:
+ - nvidia,tegra186-agic
+ - nvidia,tegra194-agic
+ - enum:
+ - nvidia,tegra210-agic
+
interrupt-controller: true
"#address-cells":
--
2.24.1
^ permalink raw reply related
* [PATCH 00/38] dt-bindings: json-schema conversions and cleanups
From: Thierry Reding @ 2020-06-12 14:18 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Hi Rob,
This set of patches is the result of trying to get all the Tegra DTS
files validated against the YAML bindings. I initially set out to do
this conversion because I wanted to add interconnect properties and
a way of validating them, so this started off as just a conversion
of various bindings and the addition of the interconnects properties
but then spun out of control a bit as I was running into various
issues with the validation.
I do have another set of patches of about 60 patches that fix various
preexisting issues in the device tree files, but after applying these
conversions and fixes as well as the DT updates I can validate all 32
and 64-bit device tree files for Tegra with a "make dtbs_check".
Not all of the device tree bindings have been converted yet, but the
main ones should be covered. I plan on doing a couple more conversions
once these have landed and as I find the time.
I wasn't sure whether I should be sending this as multiple smaller
series or a large set, but ended up deciding on the latter because a lot
of these are very similar, so should be easier to review as a lot. If
you prefer some other format, let me know.
Thierry
Thierry Reding (38):
dt-bindings: interrupt-controller: arm,gic: Add compatible for
Tegra186 AGIC
dt-bindings: memory: nvidia: Mark memory controller as interconnect
provider
dt-bindings: memory: Increase number of reg entries on Tegra194
dt-bindings: firmware: Convert Tegra186 BPMP bindings to json-schema
dt-bindings: firmware: tegra186-bpmp: Document interconnect paths
dt-bindings: display: tegra: Document display-hub
dt-bindings: display: tegra: Convert to json-schema
dt-bindings: display: tegra: Document interconnect paths
dt-bindings: gpu: tegra: Convert to json-schema
dt-bindings: gpu: tegra: Document interconnect paths
dt-bindings: mmc: tegra: Convert to json-schema
dt-bindings: mmc: tegra: Document interconnect paths
dt-bindings: pci: tegra: Convert to json-schema
dt-bindings: pci: tegra: Document interconnect paths
dt-bindings: sound: tegra: hda: Convert to json-schema
dt-bindings: sound: tegra: hda: Document interconnect paths
dt-bindings: usb: tegra-xusb: Convert to json-schema
dt-bindings: usb: tegra-xusb: Document interconnect paths
dt-bindings: net: dwc-qos-ethernet: Convert to json-schema
dt-bindings: net: dwc-qos-ethernet: Document interconnect paths
dt-bindings: sound: sgtl5000: Convert to json-schema
dt-bindings: gpio: tegra186: Use unique include guard
dt-bindings: gpio: tegra186: Convert to json-schema
dt-bindings: mfd: max77620: Convert to json-schema
dt-bindings: gpio: tegra: Convert to json-schema
dt-bindings: pci: iommu: Convert to json-schema
dt-bindings: tegra: Add missing compatible strings
dt-bindings: phy: tegra-xusb: Convert to json-schema
dt-bindings: tegra: pmc: Increase clock limit for power domains
dt-bindings: panel: Allow reg property for DSI panels
dt-bindings: panel: simple: Use unevaluatedProperties
dt-bindings: leds: Document rfkill* trigger
dt-bindings: memory-controller: Document Tegra132 EMC
dt-bindings: memory-controller: Fix "reg" entries on Tegra194
dt-bindings: memory: Update Tegra210 EMC bindings
dt-bindings: power: supply: sbs-battery: Document TI BQ20Z45
compatible
dt-bindings: pwm: Explicitly include pwm.yaml
dt-bindings: serial: Document Tegra-specific properties
.../devicetree/bindings/arm/tegra.yaml | 46 +-
.../arm/tegra/nvidia,tegra20-pmc.yaml | 2 +-
.../bindings/display/panel/panel-simple.yaml | 5 +-
.../display/tegra/nvidia,tegra20-host1x.txt | 466 ---
.../display/tegra/nvidia,tegra20-host1x.yaml | 1458 +++++++++
.../firmware/nvidia,tegra186-bpmp.txt | 107 -
.../firmware/nvidia,tegra186-bpmp.yaml | 201 ++
.../bindings/gpio/gpio-max77620.txt | 25 -
.../bindings/gpio/nvidia,tegra186-gpio.txt | 165 -
.../bindings/gpio/nvidia,tegra186-gpio.yaml | 215 ++
.../bindings/gpio/nvidia,tegra20-gpio.txt | 40 -
.../bindings/gpio/nvidia,tegra20-gpio.yaml | 111 +
.../devicetree/bindings/gpu/nvidia,gk20a.txt | 90 -
.../devicetree/bindings/gpu/nvidia,gk20a.yaml | 204 ++
.../interrupt-controller/arm,gic.yaml | 7 +
.../devicetree/bindings/leds/common.yaml | 17 +-
.../nvidia,tegra124-emc.yaml | 4 +-
.../nvidia,tegra186-mc.yaml | 70 +-
.../nvidia,tegra210-emc.yaml | 34 +-
.../devicetree/bindings/mfd/max77620.txt | 162 -
.../devicetree/bindings/mfd/max77620.yaml | 662 ++++
.../bindings/mmc/nvidia,tegra20-sdhci.txt | 117 -
.../bindings/mmc/nvidia,tegra20-sdhci.yaml | 283 ++
.../bindings/net/snps,dwc-qos-ethernet.txt | 168 -
.../bindings/net/snps,dwc-qos-ethernet.yaml | 322 ++
.../bindings/pci/nvidia,tegra20-pcie.txt | 682 ----
.../bindings/pci/nvidia,tegra20-pcie.yaml | 835 +++++
.../devicetree/bindings/pci/pci-iommu.txt | 171 -
.../devicetree/bindings/pci/pci-iommu.yaml | 168 +
.../phy/nvidia,tegra124-xusb-padctl.txt | 778 -----
.../phy/nvidia,tegra124-xusb-padctl.yaml | 2800 +++++++++++++++++
.../bindings/pinctrl/pinctrl-max77620.txt | 127 -
.../power/supply/sbs,sbs-battery.yaml | 6 +-
.../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 51 +-
.../bindings/pwm/google,cros-ec-pwm.yaml | 4 +-
.../devicetree/bindings/pwm/iqs620a-pwm.yaml | 4 +-
.../devicetree/bindings/pwm/pwm-samsung.yaml | 1 -
.../devicetree/bindings/pwm/pwm.yaml | 2 +
.../bindings/pwm/renesas,pwm-rcar.yaml | 4 +-
.../bindings/pwm/renesas,tpu-pwm.yaml | 4 +-
.../bindings/regulator/regulator-max77620.txt | 222 --
.../devicetree/bindings/serial/8250.yaml | 26 +
.../bindings/sound/nvidia,tegra30-hda.txt | 35 -
.../bindings/sound/nvidia,tegra30-hda.yaml | 141 +
.../devicetree/bindings/sound/sgtl5000.txt | 60 -
.../devicetree/bindings/sound/sgtl5000.yaml | 103 +
.../bindings/thermal/max77620_thermal.txt | 70 -
.../devicetree/bindings/trivial-devices.yaml | 2 -
.../bindings/usb/nvidia,tegra124-xusb.txt | 132 -
.../bindings/usb/nvidia,tegra124-xusb.yaml | 371 +++
include/dt-bindings/gpio/tegra186-gpio.h | 4 +-
51 files changed, 8103 insertions(+), 3681 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
delete mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt
create mode 100644 Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-max77620.txt
delete mode 100644 Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.txt
create mode 100644 Documentation/devicetree/bindings/gpio/nvidia,tegra186-gpio.yaml
delete mode 100644 Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
create mode 100644 Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.yaml
delete mode 100644 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
create mode 100644 Documentation/devicetree/bindings/gpu/nvidia,gk20a.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/max77620.txt
create mode 100644 Documentation/devicetree/bindings/mfd/max77620.yaml
delete mode 100644 Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.txt
create mode 100644 Documentation/devicetree/bindings/mmc/nvidia,tegra20-sdhci.yaml
delete mode 100644 Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
create mode 100644 Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.yaml
delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.yaml
delete mode 100644 Documentation/devicetree/bindings/pci/pci-iommu.txt
create mode 100644 Documentation/devicetree/bindings/pci/pci-iommu.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.yaml
delete mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-max77620.txt
delete mode 100644 Documentation/devicetree/bindings/regulator/regulator-max77620.txt
delete mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt
create mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml
delete mode 100644 Documentation/devicetree/bindings/sound/sgtl5000.txt
create mode 100644 Documentation/devicetree/bindings/sound/sgtl5000.yaml
delete mode 100644 Documentation/devicetree/bindings/thermal/max77620_thermal.txt
delete mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.yaml
--
2.24.1
^ permalink raw reply
* Re: [PATCH] memory: tegra: Avoid double error messaging when IRQ absent
From: Dmitry Osipenko @ 2020-06-12 14:15 UTC (permalink / raw)
To: Keyur Patel, Thierry Reding, Jonathan Hunter, Peter De Schrijver,
Kate Stewart, Greg Kroah-Hartman, Thomas Gleixner,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200611190758.172605-1-iamkeyur96-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
11.06.2020 22:07, Keyur Patel пишет:
> Since the commit 7723f4c ("driver core: platform: Add an error message
> to platform_get_irq*()") platform_get_irq() started issuing an error message.
> Thus, there is no need to have the same in the driver.
>
> Signed-off-by: Keyur Patel <iamkeyur96-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/memory/tegra/mc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index ec8403557ed4..f519c0987485 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -659,10 +659,8 @@ static int tegra_mc_probe(struct platform_device *pdev)
> }
>
> mc->irq = platform_get_irq(pdev, 0);
> - if (mc->irq < 0) {
> - dev_err(&pdev->dev, "interrupt not specified\n");
> + if (mc->irq < 0)
> return mc->irq;
> - }
>
> WARN(!mc->soc->client_id_mask, "missing client ID mask for this SoC\n");
>
>
Reviewed-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
^ permalink raw reply
* [PATCH] PCI: tegra: Remove PLL power supplies
From: Thierry Reding @ 2020-06-12 14:03 UTC (permalink / raw)
To: Lorenzo Pieralisi, Bjorn Helgaas
Cc: Rob Herring, Jonathan Hunter, linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
The Tegra PCI controller driver doesn't need to control the PLL power
supplies directly, but rather uses the pads provided by the XUSB pad
controller, which in turn is responsible for supplying power to the
PLLs.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
drivers/pci/controller/pci-tegra.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 235b456698fc..f87a09d21eb0 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2025,7 +2025,7 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
pcie->supplies[i++].supply = "hvdd-pex";
pcie->supplies[i++].supply = "vddio-pexctl-aud";
} else if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) {
- pcie->num_supplies = 6;
+ pcie->num_supplies = 3;
pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies,
sizeof(*pcie->supplies),
@@ -2033,14 +2033,11 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
if (!pcie->supplies)
return -ENOMEM;
- pcie->supplies[i++].supply = "avdd-pll-uerefe";
pcie->supplies[i++].supply = "hvddio-pex";
pcie->supplies[i++].supply = "dvddio-pex";
- pcie->supplies[i++].supply = "dvdd-pex-pll";
- pcie->supplies[i++].supply = "hvdd-pex-pll-e";
pcie->supplies[i++].supply = "vddio-pex-ctl";
} else if (of_device_is_compatible(np, "nvidia,tegra124-pcie")) {
- pcie->num_supplies = 7;
+ pcie->num_supplies = 4;
pcie->supplies = devm_kcalloc(dev, pcie->num_supplies,
sizeof(*pcie->supplies),
@@ -2050,11 +2047,8 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
pcie->supplies[i++].supply = "avddio-pex";
pcie->supplies[i++].supply = "dvddio-pex";
- pcie->supplies[i++].supply = "avdd-pex-pll";
pcie->supplies[i++].supply = "hvdd-pex";
- pcie->supplies[i++].supply = "hvdd-pex-pll-e";
pcie->supplies[i++].supply = "vddio-pex-ctl";
- pcie->supplies[i++].supply = "avdd-pll-erefe";
} else if (of_device_is_compatible(np, "nvidia,tegra30-pcie")) {
bool need_pexa = false, need_pexb = false;
--
2.24.1
^ permalink raw reply related
* [PATCH] usb: xhci: tegra: Remove PLL power supplies
From: Thierry Reding @ 2020-06-12 14:02 UTC (permalink / raw)
To: Mathias Nyman, Greg Kroah-Hartman
Cc: Jon Hunter, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
The Tegra XUSB controller driver doesn't need to control the PLL power
supplies directly, but rather uses the pads provided by the XUSB pad
controller, which in turn is responsible for supplying power to the
PLLs.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
drivers/usb/host/xhci-tegra.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 2eaf5c0af80c..9ce28ab47f4b 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1853,11 +1853,7 @@ static const char * const tegra124_supply_names[] = {
"avddio-pex",
"dvddio-pex",
"avdd-usb",
- "avdd-pll-utmip",
- "avdd-pll-erefe",
- "avdd-usb-ss-pll",
"hvdd-usb-ss",
- "hvdd-usb-ss-pll-e",
};
static const struct tegra_xusb_phy_type tegra124_phy_types[] = {
@@ -1931,10 +1927,6 @@ static const char * const tegra210_supply_names[] = {
"dvddio-pex",
"hvddio-pex",
"avdd-usb",
- "avdd-pll-utmip",
- "avdd-pll-uerefe",
- "dvdd-pex-pll",
- "hvdd-pex-pll-e",
};
static const struct tegra_xusb_phy_type tegra210_phy_types[] = {
--
2.24.1
^ permalink raw reply related
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Andrzej Pietrasiewicz @ 2020-06-12 8:47 UTC (permalink / raw)
To: Hans de Goede, Rafael J. Wysocki
Cc: Linux PM, ACPI Devel Maling List, Linux Kernel Mailing List,
linux-iio, Linux ARM, Linux Samsung SoC, linux-input, linux-tegra,
patches, ibm-acpi-devel, Platform Driver, Rafael J . Wysocki,
Len Brown, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Kukjin Kim
In-Reply-To: <40988408-8f36-3a52-6439-34084de6b129@redhat.com>
Hi Hans,
W dniu 12.06.2020 o 10:30, Hans de Goede pisze:
> Hi,
>
> On 6/10/20 3:41 PM, Andrzej Pietrasiewicz wrote:
>> Hi Hans,
>>
>> W dniu 10.06.2020 o 15:21, Hans de Goede pisze:
>>> Hi,
>>>
>>> On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote:
>>>> Hi All,
>>>>
>>>> W dniu 10.06.2020 o 12:38, Rafael J. Wysocki pisze:
>>>>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
>>>>>>> This is a quick respin of v3, with just two small changes, please see
>>>>>>> the changelog below.
>>>>>>>
>>>>>>> Userspace might want to implement a policy to temporarily disregard input
>>>>>>> from certain devices.
>>>>>>>
>>>>>>> An example use case is a convertible laptop, whose keyboard can be folded
>>>>>>> under the screen to create tablet-like experience. The user then must hold
>>>>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>>>>>> keys. It is therefore desirable to temporarily disregard input from the
>>>>>>> keyboard, until it is folded back. This obviously is a policy which should
>>>>>>> be kept out of the kernel, but the kernel must provide suitable means to
>>>>>>> implement such a policy.
>>>>>>
>>>>>> First of all sorry to start a somewhat new discussion about this
>>>>>> while this patch set is also somewhat far along in the review process,
>>>>>> but I believe what I discuss below needs to be taken into account.
>>>>>>
>>>>>> Yesterday I have been looking into why an Asus T101HA would not stay
>>>>>> suspended when the LID is closed. The cause is that the USB HID multi-touch
>>>>>> touchpad in the base of the device starts sending events when the screen
>>>>>> gets close to the touchpad (so when the LID is fully closed) and these
>>>>>> events are causing a wakeup from suspend. HID multi-touch devices
>>>>>> do have a way to tell them to fully stop sending events, also disabling
>>>>>> the USB remote wakeup the device is doing. The question is when to tell
>>>>>> it to not send events though ...
>>>>>>
>>>>>> So now I've been thinking about how to fix this and I believe that there
>>>>>> is some interaction between this problem and this patch-set.
>>>>>>
>>>>>> The problem I'm seeing on the T101HA is about wakeups, so the question
>>>>>> which I want to discuss is:
>>>>>>
>>>>>> 1. How does inhibiting interact with enabling /
>>>>>> disabling the device as a wakeup source ?
>>>>>>
>>>>>> 2. Since we have now made inhibiting equal open/close how does open/close
>>>>>> interact with a device being a wakeup source ?
>>>>>>
>>>>>> And my own initial (to be discussed) answers to these questions:
>>>>>>
>>>>>> 1. It seems to me that when a device is inhibited it should not be a
>>>>>> wakeup source, so where possible a input-device-driver should disable
>>>>>> a device's wakeup capabilities on suspend if inhibited
>>>>>
>>>>> If "inhibit" means "do not generate any events going forward", then
>>>>> this must also cover wakeup events, so I agree.
>>>>
>>>> I agree, too.
>>>>
>>>>>
>>>>>> 2. This one is trickier I don't think we have really clearly specified
>>>>>> any behavior here. The default behavior of most drivers seems to be
>>>>>> using something like this in their suspend callback:
>>>>>>
>>>>>> if (device_may_wakeup(dev))
>>>>>> enable_irq_wake(data->irq);
>>>>>> else if (input->users)
>>>>>> foo_stop_receiving_events(data);
>>>>>>
>>>>>> Since this is what most drivers seem to do I believe we should keep
>>>>>> this as is and that we should just clearly document that if the
>>>>>> input_device has users (has been opened) or not does not matter
>>>>>> for its wakeup behavior.
>>>>>>
>>>>>> Combining these 2 answers leads to this new pseudo code template
>>>>>> for an input-device's suspend method:
>>>>>>
>>>>>> /*
>>>>>> * If inhibited we have already disabled events and
>>>>>> * we do NOT want to setup the device as wake source.
>>>>>> */
>>>>>> if (input->inhibited)
>>>>>> return 0;
>>>>
>>>> Right, if a device is inhibited it shouldn't become a wakeup source,
>>>> because that would contradict the purpose of being inhibited.
>>>
>>> Ack. Note I do think that we need to document this (and more
>>> in general the answer to both questions from above) clearly so
>>> that going forward if there are any questions about how this is
>>> supposed to work we can just point to the docs.
>>>
>>> Can you do a follow-up patch, or include a patch in your next
>>> version which documents this (once we agree on what "this"
>>> exactly is) ?
>>
>> Sure I can. Just need to know when "this" becomes stable enough ;)
>> If this series otherwise looks mature enough I would opt for a
>> follow-up patch.
>
> FWIW after my flip-flop to agreeing with Dmitry that the 2
> (inhibit vs wakeup) should be completely orthogonal this new
> policy is stable/mature from my pov (and consistent with how
> we handle wakeup vs input_dev->users).
>
> I still think it would be good to do a follow-up documentation
> patch documenting that these (and esp. inhibit) are orthogonal.
>
> This will mean for example that if a device is inhibit but
> still wakeup enabled and the device's close method silences
> the devices, that it needs to be unsilenced in suspend.
> This might be worth mentioning in the docs even though
> drivers which silence the device on close should already
> unsilence the device on suspend when it is wakeup-enabled.
>
> Note maybe we should give it a couple of days for others to
> give their opinion before you submit the follow-up documentation
> patch.
>
True. I will send something after the weekend.
Andrzej
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Hans de Goede @ 2020-06-12 8:30 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, Rafael J. Wysocki
Cc: Linux PM, ACPI Devel Maling List, Linux Kernel Mailing List,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Linux ARM, Linux Samsung SoC,
linux-input-u79uwXL29TY76Z2rM5mHXA, linux-tegra,
patches-yzvPICuk2AA4QjBA90+/kJqQE7yCjDx5,
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Platform Driver,
Rafael J . Wysocki, Len Brown, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Kukjin Kim
In-Reply-To: <2d5fd063-66bc-c707-4041-84a17c0a7d04-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Hi,
On 6/10/20 3:41 PM, Andrzej Pietrasiewicz wrote:
> Hi Hans,
>
> W dniu 10.06.2020 o 15:21, Hans de Goede pisze:
>> Hi,
>>
>> On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote:
>>> Hi All,
>>>
>>> W dniu 10.06.2020 o 12:38, Rafael J. Wysocki pisze:
>>>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>
>>>>> Hi All,
>>>>>
>>>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
>>>>>> This is a quick respin of v3, with just two small changes, please see
>>>>>> the changelog below.
>>>>>>
>>>>>> Userspace might want to implement a policy to temporarily disregard input
>>>>>> from certain devices.
>>>>>>
>>>>>> An example use case is a convertible laptop, whose keyboard can be folded
>>>>>> under the screen to create tablet-like experience. The user then must hold
>>>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>>>>> keys. It is therefore desirable to temporarily disregard input from the
>>>>>> keyboard, until it is folded back. This obviously is a policy which should
>>>>>> be kept out of the kernel, but the kernel must provide suitable means to
>>>>>> implement such a policy.
>>>>>
>>>>> First of all sorry to start a somewhat new discussion about this
>>>>> while this patch set is also somewhat far along in the review process,
>>>>> but I believe what I discuss below needs to be taken into account.
>>>>>
>>>>> Yesterday I have been looking into why an Asus T101HA would not stay
>>>>> suspended when the LID is closed. The cause is that the USB HID multi-touch
>>>>> touchpad in the base of the device starts sending events when the screen
>>>>> gets close to the touchpad (so when the LID is fully closed) and these
>>>>> events are causing a wakeup from suspend. HID multi-touch devices
>>>>> do have a way to tell them to fully stop sending events, also disabling
>>>>> the USB remote wakeup the device is doing. The question is when to tell
>>>>> it to not send events though ...
>>>>>
>>>>> So now I've been thinking about how to fix this and I believe that there
>>>>> is some interaction between this problem and this patch-set.
>>>>>
>>>>> The problem I'm seeing on the T101HA is about wakeups, so the question
>>>>> which I want to discuss is:
>>>>>
>>>>> 1. How does inhibiting interact with enabling /
>>>>> disabling the device as a wakeup source ?
>>>>>
>>>>> 2. Since we have now made inhibiting equal open/close how does open/close
>>>>> interact with a device being a wakeup source ?
>>>>>
>>>>> And my own initial (to be discussed) answers to these questions:
>>>>>
>>>>> 1. It seems to me that when a device is inhibited it should not be a
>>>>> wakeup source, so where possible a input-device-driver should disable
>>>>> a device's wakeup capabilities on suspend if inhibited
>>>>
>>>> If "inhibit" means "do not generate any events going forward", then
>>>> this must also cover wakeup events, so I agree.
>>>
>>> I agree, too.
>>>
>>>>
>>>>> 2. This one is trickier I don't think we have really clearly specified
>>>>> any behavior here. The default behavior of most drivers seems to be
>>>>> using something like this in their suspend callback:
>>>>>
>>>>> if (device_may_wakeup(dev))
>>>>> enable_irq_wake(data->irq);
>>>>> else if (input->users)
>>>>> foo_stop_receiving_events(data);
>>>>>
>>>>> Since this is what most drivers seem to do I believe we should keep
>>>>> this as is and that we should just clearly document that if the
>>>>> input_device has users (has been opened) or not does not matter
>>>>> for its wakeup behavior.
>>>>>
>>>>> Combining these 2 answers leads to this new pseudo code template
>>>>> for an input-device's suspend method:
>>>>>
>>>>> /*
>>>>> * If inhibited we have already disabled events and
>>>>> * we do NOT want to setup the device as wake source.
>>>>> */
>>>>> if (input->inhibited)
>>>>> return 0;
>>>
>>> Right, if a device is inhibited it shouldn't become a wakeup source,
>>> because that would contradict the purpose of being inhibited.
>>
>> Ack. Note I do think that we need to document this (and more
>> in general the answer to both questions from above) clearly so
>> that going forward if there are any questions about how this is
>> supposed to work we can just point to the docs.
>>
>> Can you do a follow-up patch, or include a patch in your next
>> version which documents this (once we agree on what "this"
>> exactly is) ?
>
> Sure I can. Just need to know when "this" becomes stable enough ;)
> If this series otherwise looks mature enough I would opt for a
> follow-up patch.
FWIW after my flip-flop to agreeing with Dmitry that the 2
(inhibit vs wakeup) should be completely orthogonal this new
policy is stable/mature from my pov (and consistent with how
we handle wakeup vs input_dev->users).
I still think it would be good to do a follow-up documentation
patch documenting that these (and esp. inhibit) are orthogonal.
This will mean for example that if a device is inhibit but
still wakeup enabled and the device's close method silences
the devices, that it needs to be unsilenced in suspend.
This might be worth mentioning in the docs even though
drivers which silence the device on close should already
unsilence the device on suspend when it is wakeup-enabled.
Note maybe we should give it a couple of days for others to
give their opinion before you submit the follow-up documentation
patch.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Hans de Goede @ 2020-06-12 8:17 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, linux-kernel,
linux-iio, linux-arm-kernel, linux-samsung-soc, linux-input,
linux-tegra, patches, ibm-acpi-devel, platform-driver-x86
Cc: Rafael J . Wysocki, Len Brown, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Kukjin Kim,
Krzysztof Kozlowski, Dmitry Torokhov, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Vladimir Zapolskiy, Sylvain Lemieux, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Barry
In-Reply-To: <20200608112211.12125-1-andrzej.p@collabora.com>
Hi,
On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
> This is a quick respin of v3, with just two small changes, please see
> the changelog below.
>
> Userspace might want to implement a policy to temporarily disregard input
> from certain devices.
>
> An example use case is a convertible laptop, whose keyboard can be folded
> under the screen to create tablet-like experience. The user then must hold
> the laptop in such a way that it is difficult to avoid pressing the keyboard
> keys. It is therefore desirable to temporarily disregard input from the
> keyboard, until it is folded back. This obviously is a policy which should
> be kept out of the kernel, but the kernel must provide suitable means to
> implement such a policy.
>
> Due to interactions with suspend/resume, a helper has been added for drivers
> to decide if the device is being used or not (PATCH 1/7) and it has been
> applied to relevant drivers (PATCH 2,4,5,6/7).
>
> PATCH 7/7 adds support for inhibiting input devices.
>
> This work is inspired by:
>
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/45c2d7bb398f74adfae0017e20b224152fde3822
>
> and
>
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4ce0e8a3697edb8fd071110b3af65014512061c7
>
> In this respin the elan_i2c patch is dropped and converting it will be
> addressed later.
>
> v3..v4:
> - updated the comment in input_open_device() (Hans)
> - used more straightforward locking pattern in adc/exynos (Michał)
>
> v2..v3:
> - ignored autorepeat events in input_get_disposition() if a key is not
> pressed (Hans)
> - dropped inhibit()/uninhibit() driver callbacks (Hans)
> - split ACPI button patch into taking the lock and using the helper (Rafael)
> - dropped the elan_i2c conversion
> - fixed typos in exynos adc
>
> v1..v2:
> - added input_device_enabled() helper and used it in drivers (Dmitry)
> - the fact of open() and close() being called in inhibit/uninhibit paths has
> been emphasized in the commit message of PATCH 6/7 (Dmitry)
>
> Andrzej Pietrasiewicz (6):
> Input: add input_device_enabled()
> Input: use input_device_enabled()
> ACPI: button: Access input device's users under appropriate mutex
> ACPI: button: Use input_device_enabled() helper
> iio: adc: exynos: Use input_device_enabled()
> platform/x86: thinkpad_acpi: Use input_device_enabled()
>
> Patrik Fimml (1):
> Input: Add "inhibited" property
The entire series looks good to me:
Acked-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Hans de Goede @ 2020-06-12 8:14 UTC (permalink / raw)
To: Dmitry Torokhov, Rafael J. Wysocki
Cc: Andrzej Pietrasiewicz, Linux PM, ACPI Devel Maling List,
Linux Kernel Mailing List, linux-iio, Linux ARM,
Linux Samsung SoC, linux-input, linux-tegra, patches,
ibm-acpi-devel, Platform Driver, Rafael J . Wysocki, Len Brown,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Kuk
In-Reply-To: <20200610182836.GA248110@dtor-ws>
Hi,
On 6/10/20 8:28 PM, Dmitry Torokhov wrote:
> On Wed, Jun 10, 2020 at 12:38:30PM +0200, Rafael J. Wysocki wrote:
>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi All,
>>>
>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
>>>> This is a quick respin of v3, with just two small changes, please see
>>>> the changelog below.
>>>>
>>>> Userspace might want to implement a policy to temporarily disregard input
>>>> from certain devices.
>>>>
>>>> An example use case is a convertible laptop, whose keyboard can be folded
>>>> under the screen to create tablet-like experience. The user then must hold
>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>>> keys. It is therefore desirable to temporarily disregard input from the
>>>> keyboard, until it is folded back. This obviously is a policy which should
>>>> be kept out of the kernel, but the kernel must provide suitable means to
>>>> implement such a policy.
>>>
>>> First of all sorry to start a somewhat new discussion about this
>>> while this patch set is also somewhat far along in the review process,
>>> but I believe what I discuss below needs to be taken into account.
>>>
>>> Yesterday I have been looking into why an Asus T101HA would not stay
>>> suspended when the LID is closed. The cause is that the USB HID multi-touch
>>> touchpad in the base of the device starts sending events when the screen
>>> gets close to the touchpad (so when the LID is fully closed) and these
>>> events are causing a wakeup from suspend. HID multi-touch devices
>>> do have a way to tell them to fully stop sending events, also disabling
>>> the USB remote wakeup the device is doing. The question is when to tell
>>> it to not send events though ...
>>>
>>> So now I've been thinking about how to fix this and I believe that there
>>> is some interaction between this problem and this patch-set.
>>>
>>> The problem I'm seeing on the T101HA is about wakeups, so the question
>>> which I want to discuss is:
>>>
>>> 1. How does inhibiting interact with enabling /
>>> disabling the device as a wakeup source ?
>
> One should not affect the other.
>
>>>
>>> 2. Since we have now made inhibiting equal open/close how does open/close
>>> interact with a device being a wakeup source ?
>
> One did not affect another, and it should not.
>
>>>
>>> And my own initial (to be discussed) answers to these questions:
>>>
>>> 1. It seems to me that when a device is inhibited it should not be a
>>> wakeup source, so where possible a input-device-driver should disable
>>> a device's wakeup capabilities on suspend if inhibited
>>
>> If "inhibit" means "do not generate any events going forward", then
>> this must also cover wakeup events, so I agree.
>
> Why? These are separate concepts. Do we disable wake on lan when
> bringing network interface down? Do we update power/wakeup when device
> is inhibited? Do we restore it afterwards? Do we un-inhibit if we
> reenable wakeup after device is inhibited? Do we return error? How?
>
> Inhibit works on logical level, i.e. if I have several input interfaces
> on the same hardware device, I cam inhibit one leaving others intact.
> This does not mean that the device should stop generating wakeup events.
> We can't even guarantee this for composite devices.
After thinking more about this I believe you are right and we should
keep these as 2 separate, completely independent settings.
Especially since the wakeup setting typically is a setting of the
parent device, where as the inhibit is done on the actual input-dev.
###
Some quick background info on my original thoughts here, as mentioned
I started thinking about this because of spurious wakeups from suspend
when the lid of an asus t101ha is "touching" its touchpad. The HID
multi-touch protocol has a setting where we can ask the device to
stop sending events. So even though the kbd + touchpad are a
single composite USB device, we can disable wakeup (in a way)
for just the touchpad at the hid-multitouch level.
So I was thinking maybe adding a separate wakeup setting to the
input device itself for this. But thinking more about it, when
the lid is closed we can just disable wakeup on the entire USB
device, since the keyboard is covered by the lid too.
And then on suspend the hid-multitouch driver can detect that its
parent (or parents parent in the case of USB) has wakeup disabled
and also tell the device to stop scanning for fingers to save some
power.
We probably also need a close and open callbacks add the HID-driver
level, so that if there are no touchpad users we can also use
the same option to put the HID multi-touch device in a low power mode
where it does not scan for fingers.
<snip>
>>> A different, but related issue is how to make devices actually use the
>>> new inhibit support on the builtin keyboard + touchpad when say the lid
>>> is closed. Arguably this is an userspace problem, but it is a tricky
>>> one. Currently on most modern Linux distributions suspend-on-lid-close
>>> is handled by systemd-logind and most modern desktop-environments are
>>> happy to have logind handle this for them.
>>>
>>> But most knowledge about input devices and e.g. heurisitics to decide
>>> if a touchpad is internal or external are part of libinput. Now we could
>>> have libinput use the new inhibit support (1), but then when the lid
>>> closes we get race between whatever process is using libinput trying
>>> to inhibit the touchpad (which must be done before to suspend to disable
>>> it as wakeup source) and logind trying to suspend the system.
>>>
>>> One solution here would be to move the setting of the inhibit sysfs
>>> attr into logind, but that requires adding a whole bunch of extra
>>> knowledge to logind which does not really belong there IMHO.
>
> You do not need to push the knowledge into logind, you just need to
> communicate to logind what devices can be wakeup sources and which ones
> should not. Chrome OS uses udev tags/properties for that.
True, I did not think of doing the tag thingie + letting logind do
the inhibit on LID close based on that. logind could also disable
wakeup (to save power while suspended) on devices which are tagged
for it to do that (should probably be a separate tag from the
inhibit tag).
>>> I've been thinking a bit about this and to me it seems that the kernel
>>> is in the ideal position to automatically inhibit some devices when
>>> some EV_SW transitions from 0->1 (and uninhibit again on 1->0). The
>>> issue here is to chose on which devices to enable this. I believe
>>> that the auto inhibit on some switches mechanism is best done inside
>>> the kernel (disabled by default) and then we can have a sysfs
>>> attr called auto_inhibit_ev_sw_mask which can be set to e.g.
>>> (1 << SW_LID) to make the kernel auto-inhibit the input-device whenever
>>> the lid is closed, or to ((1 << SW_LID) | (1 << SW_TABLET_MODE)) to
>>> inhibit both when the lid is closed or when switched to tablet mode.
>
> This is a policy and should be kept out of the kernel. Yes, we had it
> implemented with rfkill input handler, but it caused quite a few issues.
> As far as I know it is not being used anymore and we should not try with
> SW_LID->inhibit either.
>
> I know it is faster to patch the kernel than to roll out proper
> userspace because everyone updates kernel regularly, but it does not
> mean it is the right solution.
Agreed, I just could not come up with a clean userspace solution, but
using udev+hwdb to set a tag for logind instead of having the write
to a new auto_inhibit_ev_sw_mask will work nicely.
So I think this is all resolved now (or at least we have a plan for it).
Regards,
Hans
^ permalink raw reply
* AMD IOMMU + SME + amdgpu regression
From: Alex Xu (Hello71) @ 2020-06-11 23:05 UTC (permalink / raw)
To: Joerg Roedel, linux-kernel, David Rientjes, Christoph Hellwig
Cc: Will Deacon, Robin Murphy, Marek Szyprowski, Kukjin Kim,
Krzysztof Kozlowski, David Woodhouse, Lu Baolu, Andy Gross,
Bjorn Andersson, Matthias Brugger, Rob Clark, Heiko Stuebner,
Gerald Schaefer, Thierry Reding, Jonathan Hunter,
Jean-Philippe Brucker, Daniel Drake, jonathan.derrick,
linux-samsung-soc, linux-arm-msm
In-Reply-To: <1591915710.rakbpzst8h.none.ref@localhost>
Hi,
amdgpu + IOMMU + SME is now working for me on 5.7, yay! But, it is
broken on torvalds master, boo. On boot, depending on which exact commit
I test, it either hangs immediately (with built-in driver, before
starting initramfs), displays some errors then hangs, or spams the
screen with many amdgpu errors.
I bisected the black screen hang to:
commit dce8d6964ebdb333383bacf5e7ab8c27df151218
Author: Joerg Roedel <jroedel@suse.de>
Date: Wed Apr 29 15:36:53 2020 +0200
iommu/amd: Convert to probe/release_device() call-backs
Convert the AMD IOMMU Driver to use the probe_device() and
release_device() call-backs of iommu_ops, so that the iommu core code
does the group and sysfs setup.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Link: https://lore.kernel.org/r/20200429133712.31431-16-joro@8bytes.org
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Testing torvalds master (623f6dc593) with the containing merge
(98bdc74b36) plus the DMA mapping merge (4e94d08734) reverted allows
amdgpu + IOMMU + SME to once again work.
I think that nobody is really working on amdgpu + SME, but it would be a
shame if it was supported and then incidentally broken by a small
change.
I am using an ASRock B450 Pro4 with Ryzen 1600 and ASUS RX 480. I don't
understand this code at all, but let me know what I can do to
troubleshoot.
Thanks,
Alex.
^ permalink raw reply
* [PATCH] memory: tegra: Avoid double error messaging when IRQ absent
From: Keyur Patel @ 2020-06-11 19:07 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Dmitry Osipenko,
Peter De Schrijver, Kate Stewart, Greg Kroah-Hartman, Keyur Patel,
Thomas Gleixner, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Since the commit 7723f4c ("driver core: platform: Add an error message
to platform_get_irq*()") platform_get_irq() started issuing an error message.
Thus, there is no need to have the same in the driver.
Signed-off-by: Keyur Patel <iamkeyur96-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/memory/tegra/mc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index ec8403557ed4..f519c0987485 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -659,10 +659,8 @@ static int tegra_mc_probe(struct platform_device *pdev)
}
mc->irq = platform_get_irq(pdev, 0);
- if (mc->irq < 0) {
- dev_err(&pdev->dev, "interrupt not specified\n");
+ if (mc->irq < 0)
return mc->irq;
- }
WARN(!mc->soc->client_id_mask, "missing client ID mask for this SoC\n");
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] drm/tegra: Add zpos property for cursor planes
From: Dmitry Osipenko @ 2020-06-11 11:27 UTC (permalink / raw)
To: Daniel Stone
Cc: Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA, dri-devel,
Jon Hunter
In-Reply-To: <CAPj87rMcXQozYX90wGYbv_vNnQ0-fLLnEdH3Lzio+B1L7xZuYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
11.06.2020 11:17, Daniel Stone пишет:
> Hi Dmitry,
>
> On Thu, 11 Jun 2020 at 08:54, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 10.06.2020 14:30, Thierry Reding пишет:
>>> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
>>> have a zpos property") a warning is emitted if there's a mix of planes
>>> with and without a zpos property.
>>
>> What problem does it solve?
>
> Well, it fixes the WARN_ON, which asserts that either no planes have
> the zpos property attached, or all planes have the zpos property
> attached. When only _some_ planes have the property, the property is
> pretty much useless: zpos exists to guarantee to userspace the
> relative ordering between planes, defined by the value of the
> property. If not all planes have the property, then userspace cannot
> reason about the ordering, since some of the planes have undefined
> ordering.
Hello Daniel,
So it's a kernel warning, thank you for the clarification! I'm running
only older Tegras that have a black-n-white cursor plane, but this is
not supported by the upstream kernel. No wonder that I haven't seen the
warning, this made me curious what this patch is about :)
^ permalink raw reply
* Re: [PATCH] drm/tegra: Add zpos property for cursor planes
From: Daniel Stone @ 2020-06-11 8:17 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA, dri-devel,
Jon Hunter
In-Reply-To: <ca53d82f-1fd0-b2da-cfbf-7183a977e1d1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Dmitry,
On Thu, 11 Jun 2020 at 08:54, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 10.06.2020 14:30, Thierry Reding пишет:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
> > have a zpos property") a warning is emitted if there's a mix of planes
> > with and without a zpos property.
>
> What problem does it solve?
Well, it fixes the WARN_ON, which asserts that either no planes have
the zpos property attached, or all planes have the zpos property
attached. When only _some_ planes have the property, the property is
pretty much useless: zpos exists to guarantee to userspace the
relative ordering between planes, defined by the value of the
property. If not all planes have the property, then userspace cannot
reason about the ordering, since some of the planes have undefined
ordering.
Cheers,
Daniel
^ permalink raw reply
* Re: [RFC PATCH v1 10/18] dt-bindings: tegra: Document VI and CSI port nodes
From: Sowjanya Komatineni @ 2020-06-10 21:11 UTC (permalink / raw)
To: Sakari Ailus
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
jonathanh-DDmLM1+adcrQT0dZR+AlfA, frankc-DDmLM1+adcrQT0dZR+AlfA,
hverkuil-qWit8jRvyhVmR6Xm/wNWPw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
helen.koike-ZGY8ohtN/8qB+jHODAdFcQ, digetx-Re5JQEeQqe8AvxtiuMwx3w,
sboyd-DgEjT+Ai2ygdnm+yROfE0A,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <075d9d2e-eaa8-1895-8759-64c682fee1c4-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 6/10/20 8:15 AM, Sowjanya Komatineni wrote:
>
> On 6/10/20 4:23 AM, Sakari Ailus wrote:
>> Hi Sowjanya,
>>
>> Thanks for the patchset.
>>
>> On Tue, Jun 09, 2020 at 11:02:32PM -0700, Sowjanya Komatineni wrote:
>>> This patch documents Tegra VI and CSI port and endpoint nodes along
>>> with the other required properties.
>>>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> .../display/tegra/nvidia,tegra20-host1x.txt | 87
>>> ++++++++++++++++++++++
>>> 1 file changed, 87 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>>
>>> index 4731921..f70a838 100644
>>> ---
>>> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> +++
>>> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> @@ -65,6 +65,48 @@ of the following host1x client modules:
>>> - power-domains: Must include sor powergate node as csicil
>>> is in
>>> SOR partition.
>>> + Optional properties for csi node:
>> What hardware does the csi node represent? A CSI-2 receiver? Something
>> else?
>>
>> If you have two connections, you need two ports. The example isn't quite
>> clear on this; it would appear to represent a single physical interface.
>
> CS-2 receiver with 2 connections. one for sink with sensor to csi and
> other as source with csi to Tegra vi.
>
> Was using separate port for sink and source and then I misunderstood
> device graph document and changed to have multiple endpoints in same
> port.
>
> Will update this in v2 to have separate port for each sink and source
> endpoint in csi in dt and also in driver implementation.
>
>>> +
>>> + - channel nodes: Max upto 6 channels/streams are supported
>>> with each CSI
>>> + brick can as either x4 or x2 based on hw connectivity to sensor.
>>> +
>>> + Required properties:
>>> + - reg: channel/stream index
>>> + - nvidia,mipi-calibrate: Should contain a phandle and a specifier
>>> + specifying which pads are used by this CSI port and need to be
>>> + calibrated. See also ../display/tegra/nvidia,tegra114-mipi.txt.
>>> +
>>> + - port: CSI port node and its endpoint nodes as per device graph
>>> + bindings defined in
>>> Documentation/devicetree/bindings/graph.txt.
>>> + Required properties:
>> You have both properties and nodes here. Same for the above (port is a
>> node).
> Will update document to separate out port node from properties
>>
>>> + - reg: csi port index based on hw csi lanes connectivity to the
>>> + sensor.
>>> + - bus-width: number of lanes used by this port. Supported lanes
>>> + are 1/2/4.
>> bus-width belongs to the endpoint. Note that this is for parallel busses
>> only. If you need the number of lanes, the property is called
>> data-lanes.
> Will update in v2 for having separate ports for sink and source
> endpoints will move bus-width to endpoint.
Thanks Sakari. Will switch to use data-lanes property in csi ports end
points in v2.
>>
>>> + - endpoint@0: sink node
>>> + Required properties:
>>> + - reg: endpoint id. This is used to retrieve pad for creating
>>> + media link
>>> + - remote-endpoint: phandle to sensor endpoint
>>> + - endpoint@1: source node
>>> + - reg: endpoint id. This is used to retrieve pad for creating
>>> + media link
>>> + - remote-endpoint: phandle to vi port endpoint
>>> +
>>> + Optional properties for vi node:
>>> + - ports: Video port nodes and endpoint nodes as per device graph
>>> bindings
>>> + defined in Documentation/devicetree/bindings/graph.txt
>>> + Max 6 ports are supported and each port should have one
>>> endpoint node.
>>> +
>>> + Required properties:
>>> + - port: VI port node and its sink endpoint node
>>> + Required properties:
>>> + - reg: should match port index
>>> + - endpoint@0: sink node
>>> + Required properties:
>>> + - reg: endpoint id must be 0
>>> + - remote-endpoint: phandle to CSI endpoint node.
>>> +
>>> - epp: encoder pre-processor
>>> Required properties:
>>> @@ -340,6 +382,22 @@ Example:
>>> ranges = <0x0 0x0 0x54080000 0x2000>;
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + imx219_vi_in0: endpoint@0 {
>>> + reg = <0>;
>>> + remote-endpoint = <&imx219_csi_out0>;
>>> + };
>>> + };
>>> + };
>>> +
>>> csi@838 {
>>> compatible = "nvidia,tegra210-csi";
>>> reg = <0x838 0x1300>;
>>> @@ -362,6 +420,35 @@ Example:
>>> <&tegra_car TEGRA210_CLK_CSI_TPG>;
>>> clock-names = "csi", "cilab", "cilcd", "cile",
>>> "csi_tpg";
>>> power-domains = <&pd_sor>;
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + channel@0 {
>>> + reg = <0>;
>>> + nvidia,mipi-calibrate = <&mipi 0x001>;
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + port@0 {
>>> + reg = <0>;
>>> + bus-width = <2>;
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + imx219_csi_in0: endpoint@0 {
>>> + reg = <0>;
>>> + remote-endpoint = <&imx219_out0>;
>>> + };
>>> +
>>> + imx219_csi_out0: endpoint@1 {
>>> + reg = <1>;
>>> + remote-endpoint = <&imx219_vi_in0>;
>>> + };
>>> + };
>>> + };
>>> };
>>> };
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Dmitry Torokhov @ 2020-06-10 18:28 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Hans de Goede, Andrzej Pietrasiewicz, Linux PM,
ACPI Devel Maling List, Linux Kernel Mailing List, linux-iio,
Linux ARM, Linux Samsung SoC, linux-input, linux-tegra, patches,
ibm-acpi-devel, Platform Driver, Rafael J . Wysocki, Len Brown,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen
In-Reply-To: <CAJZ5v0hB2ra2K=dd9ZjVyy1V2b1PmFHm79uDO2HtHU1D_4YUbw@mail.gmail.com>
On Wed, Jun 10, 2020 at 12:38:30PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi All,
> >
> > On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
> > > This is a quick respin of v3, with just two small changes, please see
> > > the changelog below.
> > >
> > > Userspace might want to implement a policy to temporarily disregard input
> > > from certain devices.
> > >
> > > An example use case is a convertible laptop, whose keyboard can be folded
> > > under the screen to create tablet-like experience. The user then must hold
> > > the laptop in such a way that it is difficult to avoid pressing the keyboard
> > > keys. It is therefore desirable to temporarily disregard input from the
> > > keyboard, until it is folded back. This obviously is a policy which should
> > > be kept out of the kernel, but the kernel must provide suitable means to
> > > implement such a policy.
> >
> > First of all sorry to start a somewhat new discussion about this
> > while this patch set is also somewhat far along in the review process,
> > but I believe what I discuss below needs to be taken into account.
> >
> > Yesterday I have been looking into why an Asus T101HA would not stay
> > suspended when the LID is closed. The cause is that the USB HID multi-touch
> > touchpad in the base of the device starts sending events when the screen
> > gets close to the touchpad (so when the LID is fully closed) and these
> > events are causing a wakeup from suspend. HID multi-touch devices
> > do have a way to tell them to fully stop sending events, also disabling
> > the USB remote wakeup the device is doing. The question is when to tell
> > it to not send events though ...
> >
> > So now I've been thinking about how to fix this and I believe that there
> > is some interaction between this problem and this patch-set.
> >
> > The problem I'm seeing on the T101HA is about wakeups, so the question
> > which I want to discuss is:
> >
> > 1. How does inhibiting interact with enabling /
> > disabling the device as a wakeup source ?
One should not affect the other.
> >
> > 2. Since we have now made inhibiting equal open/close how does open/close
> > interact with a device being a wakeup source ?
One did not affect another, and it should not.
> >
> > And my own initial (to be discussed) answers to these questions:
> >
> > 1. It seems to me that when a device is inhibited it should not be a
> > wakeup source, so where possible a input-device-driver should disable
> > a device's wakeup capabilities on suspend if inhibited
>
> If "inhibit" means "do not generate any events going forward", then
> this must also cover wakeup events, so I agree.
Why? These are separate concepts. Do we disable wake on lan when
bringing network interface down? Do we update power/wakeup when device
is inhibited? Do we restore it afterwards? Do we un-inhibit if we
reenable wakeup after device is inhibited? Do we return error? How?
Inhibit works on logical level, i.e. if I have several input interfaces
on the same hardware device, I cam inhibit one leaving others intact.
This does not mean that the device should stop generating wakeup events.
We can't even guarantee this for composite devices.
>
> > 2. This one is trickier I don't think we have really clearly specified
> > any behavior here. The default behavior of most drivers seems to be
> > using something like this in their suspend callback:
> >
> > if (device_may_wakeup(dev))
> > enable_irq_wake(data->irq);
> > else if (input->users)
> > foo_stop_receiving_events(data);
> >
> > Since this is what most drivers seem to do I believe we should keep
> > this as is and that we should just clearly document that if the
> > input_device has users (has been opened) or not does not matter
> > for its wakeup behavior.
> >
> > Combining these 2 answers leads to this new pseudo code template
> > for an input-device's suspend method:
> >
> > /*
> > * If inhibited we have already disabled events and
> > * we do NOT want to setup the device as wake source.
> > */
> > if (input->inhibited)
> > return 0;
> >
> > if (device_may_wakeup(dev))
> > enable_irq_wake(data->irq);
> > else if (input->users)
> > foo_stop_receiving_events(data);
> >
> > ###
>
> Sounds reasonable to me.
However it will not work. For many input devices connected to i2c we
declare interrupt as wakeup interrupt, and the driver does not need to
issue enable_irq_wake() and disable_irq_wake(). The wakeup handling is
happening in driver core, which is not aware of input-specific inhibit
(nor should it be).
I need to ping Mark about the patch adding the similar handling to SPI.
>
> > A different, but related issue is how to make devices actually use the
> > new inhibit support on the builtin keyboard + touchpad when say the lid
> > is closed. Arguably this is an userspace problem, but it is a tricky
> > one. Currently on most modern Linux distributions suspend-on-lid-close
> > is handled by systemd-logind and most modern desktop-environments are
> > happy to have logind handle this for them.
> >
> > But most knowledge about input devices and e.g. heurisitics to decide
> > if a touchpad is internal or external are part of libinput. Now we could
> > have libinput use the new inhibit support (1), but then when the lid
> > closes we get race between whatever process is using libinput trying
> > to inhibit the touchpad (which must be done before to suspend to disable
> > it as wakeup source) and logind trying to suspend the system.
> >
> > One solution here would be to move the setting of the inhibit sysfs
> > attr into logind, but that requires adding a whole bunch of extra
> > knowledge to logind which does not really belong there IMHO.
You do not need to push the knowledge into logind, you just need to
communicate to logind what devices can be wakeup sources and which ones
should not. Chrome OS uses udev tags/properties for that.
> >
> > I've been thinking a bit about this and to me it seems that the kernel
> > is in the ideal position to automatically inhibit some devices when
> > some EV_SW transitions from 0->1 (and uninhibit again on 1->0). The
> > issue here is to chose on which devices to enable this. I believe
> > that the auto inhibit on some switches mechanism is best done inside
> > the kernel (disabled by default) and then we can have a sysfs
> > attr called auto_inhibit_ev_sw_mask which can be set to e.g.
> > (1 << SW_LID) to make the kernel auto-inhibit the input-device whenever
> > the lid is closed, or to ((1 << SW_LID) | (1 << SW_TABLET_MODE)) to
> > inhibit both when the lid is closed or when switched to tablet mode.
This is a policy and should be kept out of the kernel. Yes, we had it
implemented with rfkill input handler, but it caused quite a few issues.
As far as I know it is not being used anymore and we should not try with
SW_LID->inhibit either.
I know it is faster to patch the kernel than to roll out proper
userspace because everyone updates kernel regularly, but it does not
mean it is the right solution.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [RFC PATCH v1 13/18] gpu: host1x: mipi: Add of_tegra_mipi_request() API
From: Sowjanya Komatineni @ 2020-06-10 17:00 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
jonathanh-DDmLM1+adcrQT0dZR+AlfA, frankc-DDmLM1+adcrQT0dZR+AlfA,
hverkuil-qWit8jRvyhVmR6Xm/wNWPw, sakari.ailus-X3B1VOXEql0,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
helen.koike-ZGY8ohtN/8qB+jHODAdFcQ
Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e84c6723-e94a-3fcf-8b38-eb680c88de25-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 6/10/20 6:33 AM, Dmitry Osipenko wrote:
> 10.06.2020 09:02, Sowjanya Komatineni пишет:
>> This patch adds an API of_tegra_mipi_request() to allow creating
>> mipi device for specific device node rather than a device so Tegra
>> CSI driver can use it for calibrating MIPI pads for each stream
>> independently.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/gpu/host1x/mipi.c | 10 ++++++++--
>> include/linux/host1x.h | 2 ++
>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
>> index e00809d..f51fe69 100644
>> --- a/drivers/gpu/host1x/mipi.c
>> +++ b/drivers/gpu/host1x/mipi.c
>> @@ -206,9 +206,9 @@ static int tegra_mipi_power_down(struct tegra_mipi *mipi)
>> return 0;
>> }
>>
>> -struct tegra_mipi_device *tegra_mipi_request(struct device *device)
>> +struct tegra_mipi_device *of_tegra_mipi_request(struct device *device,
>> + struct device_node *np)
>> {
>> - struct device_node *np = device->of_node;
>> struct tegra_mipi_device *dev;
>> struct of_phandle_args args;
>> int err;
>> @@ -252,6 +252,12 @@ struct tegra_mipi_device *tegra_mipi_request(struct device *device)
>> of_node_put(args.np);
>> return ERR_PTR(err);
>> }
>> +EXPORT_SYMBOL(of_tegra_mipi_request);
>> +
>> +struct tegra_mipi_device *tegra_mipi_request(struct device *device)
>> +{
>> + return of_tegra_mipi_request(device, device->of_node);
>> +}
>> EXPORT_SYMBOL(tegra_mipi_request);
>>
>> void tegra_mipi_free(struct tegra_mipi_device *device)
>> diff --git a/include/linux/host1x.h b/include/linux/host1x.h
>> index c230b4e..a61ca52 100644
>> --- a/include/linux/host1x.h
>> +++ b/include/linux/host1x.h
>> @@ -325,6 +325,8 @@ int host1x_client_resume(struct host1x_client *client);
>>
>> struct tegra_mipi_device;
>>
>> +struct tegra_mipi_device *of_tegra_mipi_request(struct device *device,
>> + struct device_node *np);
> Looks like adding a new function here is a bit of overkill. What about
> to change tegra_mipi_request() to take the node argument?
thought to avoid changes to existing usage of that function in other
drivers.
Will update in v2.
^ permalink raw reply
* Re: [RFC PATCH v1 05/18] i2c: tegra: Fix runtime resume to re-init VI I2C
From: Sowjanya Komatineni @ 2020-06-10 16:58 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, robh+dt, helen.koike
Cc: sboyd, gregkh, linux-media, devicetree, linux-tegra, linux-kernel,
linux-i2c
In-Reply-To: <fcced8cd-d80d-b09c-b657-cb413ec418f9@gmail.com>
On 6/10/20 6:14 AM, Dmitry Osipenko wrote:
> 10.06.2020 09:02, Sowjanya Komatineni пишет:
>> VI I2C is on host1x bus and is part of VE power domain.
>>
>> During suspend/resume VE power domain goes through power off/on.
>>
>> So, controller reset followed by i2c re-initialization is required
>> after the domain power up.
>>
>> This patch fixes it.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/i2c/busses/i2c-tegra.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index dba38a5..650240d 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -293,6 +293,8 @@ struct tegra_i2c_dev {
>> bool is_curr_atomic_xfer;
>> };
>>
>> +static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit);
>> +
>> static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
>> unsigned long reg)
>> {
>> @@ -679,8 +681,22 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>> goto disable_slow_clk;
>> }
>>
>> + /*
>> + * VI I2C device is attached to VE power domain which goes through
>> + * power ON/OFF during PM runtime resume/suspend. So, controller
>> + * should go through reset and need to re-initialize after power
>> + * domain ON.
>> + */
>> + if (i2c_dev->is_vi) {
>> + ret = tegra_i2c_init(i2c_dev, true);
>> + if (ret)
>> + goto disable_div_clk;
>> + }
>> +
>> return 0;
>>
>> +disable_div_clk:
>> + clk_disable(i2c_dev->div_clk);
>> disable_slow_clk:
>> if (i2c_dev->slow_clk)
>> clk_disable(i2c_dev->slow_clk);
>>
> The clk_disable() can cope with a NULL argument. Won't it be cleaner to
> remove the conditions?
will remove in v2
^ permalink raw reply
* [PATCH v1] clk: tegra: pll: Improve PLLM enable-state detection
From: Dmitry Osipenko @ 2020-06-10 16:37 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
Prashant Gaikwad, Michael Turquette, Stephen Boyd
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Power Management Controller (PMC) can override the PLLM clock settings,
including the enable-state. Although PMC could only act as a second level
gate, meaning that PLLM needs to be enabled by the Clock and Reset
Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
order to be functional. Please note that this patch doesn't fix any known
problem, and thus, it's merely a minor improvement.
Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/clk/tegra/clk-pll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 0b212cf2e794..4fd1edba8caf 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -330,17 +330,18 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
static int clk_pll_is_enabled(struct clk_hw *hw)
{
struct tegra_clk_pll *pll = to_clk_pll(hw);
+ bool pllm_enabled = true;
u32 val;
if (pll->params->flags & TEGRA_PLLM) {
val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
- return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
+ pllm_enabled = !!(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
}
val = pll_readl_base(pll);
- return val & PLL_BASE_ENABLE ? 1 : 0;
+ return pllm_enabled && (val & PLL_BASE_ENABLE);
}
static void _clk_pll_enable(struct clk_hw *hw)
--
2.26.0
^ permalink raw reply related
* Re: [RFC PATCH v1 10/18] dt-bindings: tegra: Document VI and CSI port nodes
From: Sowjanya Komatineni @ 2020-06-10 15:15 UTC (permalink / raw)
To: Sakari Ailus
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
jonathanh-DDmLM1+adcrQT0dZR+AlfA, frankc-DDmLM1+adcrQT0dZR+AlfA,
hverkuil-qWit8jRvyhVmR6Xm/wNWPw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
helen.koike-ZGY8ohtN/8qB+jHODAdFcQ, digetx-Re5JQEeQqe8AvxtiuMwx3w,
sboyd-DgEjT+Ai2ygdnm+yROfE0A,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20200610112303.GB805-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
On 6/10/20 4:23 AM, Sakari Ailus wrote:
> Hi Sowjanya,
>
> Thanks for the patchset.
>
> On Tue, Jun 09, 2020 at 11:02:32PM -0700, Sowjanya Komatineni wrote:
>> This patch documents Tegra VI and CSI port and endpoint nodes along
>> with the other required properties.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>> .../display/tegra/nvidia,tegra20-host1x.txt | 87 ++++++++++++++++++++++
>> 1 file changed, 87 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> index 4731921..f70a838 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> @@ -65,6 +65,48 @@ of the following host1x client modules:
>> - power-domains: Must include sor powergate node as csicil is in
>> SOR partition.
>>
>> + Optional properties for csi node:
> What hardware does the csi node represent? A CSI-2 receiver? Something
> else?
>
> If you have two connections, you need two ports. The example isn't quite
> clear on this; it would appear to represent a single physical interface.
CS-2 receiver with 2 connections. one for sink with sensor to csi and
other as source with csi to Tegra vi.
Was using separate port for sink and source and then I misunderstood
device graph document and changed to have multiple endpoints in same port.
Will update this in v2 to have separate port for each sink and source
endpoint in csi in dt and also in driver implementation.
>> +
>> + - channel nodes: Max upto 6 channels/streams are supported with each CSI
>> + brick can as either x4 or x2 based on hw connectivity to sensor.
>> +
>> + Required properties:
>> + - reg: channel/stream index
>> + - nvidia,mipi-calibrate: Should contain a phandle and a specifier
>> + specifying which pads are used by this CSI port and need to be
>> + calibrated. See also ../display/tegra/nvidia,tegra114-mipi.txt.
>> +
>> + - port: CSI port node and its endpoint nodes as per device graph
>> + bindings defined in Documentation/devicetree/bindings/graph.txt.
>> + Required properties:
> You have both properties and nodes here. Same for the above (port is a
> node).
Will update document to separate out port node from properties
>
>> + - reg: csi port index based on hw csi lanes connectivity to the
>> + sensor.
>> + - bus-width: number of lanes used by this port. Supported lanes
>> + are 1/2/4.
> bus-width belongs to the endpoint. Note that this is for parallel busses
> only. If you need the number of lanes, the property is called data-lanes.
Will update in v2 for having separate ports for sink and source
endpoints will move bus-width to endpoint.
>
>> + - endpoint@0: sink node
>> + Required properties:
>> + - reg: endpoint id. This is used to retrieve pad for creating
>> + media link
>> + - remote-endpoint: phandle to sensor endpoint
>> + - endpoint@1: source node
>> + - reg: endpoint id. This is used to retrieve pad for creating
>> + media link
>> + - remote-endpoint: phandle to vi port endpoint
>> +
>> + Optional properties for vi node:
>> + - ports: Video port nodes and endpoint nodes as per device graph bindings
>> + defined in Documentation/devicetree/bindings/graph.txt
>> + Max 6 ports are supported and each port should have one endpoint node.
>> +
>> + Required properties:
>> + - port: VI port node and its sink endpoint node
>> + Required properties:
>> + - reg: should match port index
>> + - endpoint@0: sink node
>> + Required properties:
>> + - reg: endpoint id must be 0
>> + - remote-endpoint: phandle to CSI endpoint node.
>> +
>> - epp: encoder pre-processor
>>
>> Required properties:
>> @@ -340,6 +382,22 @@ Example:
>>
>> ranges = <0x0 0x0 0x54080000 0x2000>;
>>
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + imx219_vi_in0: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&imx219_csi_out0>;
>> + };
>> + };
>> + };
>> +
>> csi@838 {
>> compatible = "nvidia,tegra210-csi";
>> reg = <0x838 0x1300>;
>> @@ -362,6 +420,35 @@ Example:
>> <&tegra_car TEGRA210_CLK_CSI_TPG>;
>> clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
>> power-domains = <&pd_sor>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> + nvidia,mipi-calibrate = <&mipi 0x001>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + bus-width = <2>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + imx219_csi_in0: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&imx219_out0>;
>> + };
>> +
>> + imx219_csi_out0: endpoint@1 {
>> + reg = <1>;
>> + remote-endpoint = <&imx219_vi_in0>;
>> + };
>> + };
>> + };
>> };
>> };
>>
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Rafael J. Wysocki @ 2020-06-10 14:01 UTC (permalink / raw)
To: Hans de Goede
Cc: Andrzej Pietrasiewicz, Rafael J. Wysocki, Linux PM,
ACPI Devel Maling List, Linux Kernel Mailing List,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Linux ARM, Linux Samsung SoC,
linux-input-u79uwXL29TY76Z2rM5mHXA, linux-tegra,
patches-yzvPICuk2AA4QjBA90+/kJqQE7yCjDx5,
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Platform Driver,
Rafael J . Wysocki, Len Brown, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen
In-Reply-To: <3e61c9c1-b211-da9f-c55b-b44eb6522f2a-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Wed, Jun 10, 2020 at 3:21 PM Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> Hi,
>
> On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote:
> > Hi All,
> >
[cut]
> > What would it mean to become a wakeup source if there are no users,
> > or nobody has ever opened the device? There are no interested
> > input handlers (users) so what's the point of becoming a wakeup
> > source? Why would the system need to wake up?
>
> Well this is what we have been doing so far, so arguably we
> need to keep doing it to avoid regressions / breaking our ABI.
>
> Lets for example take a laptop, where when suspended the
> power-button is the only valid wakeup-source and this is
> running good old slackware with fvwm2 or windowmaker as
> "desktop environment", then likely no process will have
> the power-button input evdev node open. Still we should
> wakeup the laptop on the power-button press, otherwise
> it will never wakeup.
>
> Note I agree with you that the way this works is not
> ideal, I just do not think that we can change it.
Please note that "no users" merely means that user space is not
interested in receiving and processing the events from that device.
If it is configured for system wakeup, it doesn't matter whether or
not user space will consume the related events.
Thanks!
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Hans de Goede @ 2020-06-10 13:52 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrzej Pietrasiewicz, Linux PM, ACPI Devel Maling List,
Linux Kernel Mailing List, linux-iio-u79uwXL29TY76Z2rM5mHXA,
Linux ARM, Linux Samsung SoC, linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-tegra, patches-yzvPICuk2AA4QjBA90+/kJqQE7yCjDx5,
ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Platform Driver,
Rafael J . Wysocki, Len Brown, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Kuk
In-Reply-To: <CAJZ5v0hB2ra2K=dd9ZjVyy1V2b1PmFHm79uDO2HtHU1D_4YUbw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 6/10/20 12:38 PM, Rafael J. Wysocki wrote:
> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
<snip>
>> A different, but related issue is how to make devices actually use the
>> new inhibit support on the builtin keyboard + touchpad when say the lid
>> is closed. Arguably this is an userspace problem, but it is a tricky
>> one. Currently on most modern Linux distributions suspend-on-lid-close
>> is handled by systemd-logind and most modern desktop-environments are
>> happy to have logind handle this for them.
>>
>> But most knowledge about input devices and e.g. heurisitics to decide
>> if a touchpad is internal or external are part of libinput. Now we could
>> have libinput use the new inhibit support (1), but then when the lid
>> closes we get race between whatever process is using libinput trying
>> to inhibit the touchpad (which must be done before to suspend to disable
>> it as wakeup source) and logind trying to suspend the system.
>>
>> One solution here would be to move the setting of the inhibit sysfs
>> attr into logind, but that requires adding a whole bunch of extra
>> knowledge to logind which does not really belong there IMHO.
>>
>> I've been thinking a bit about this and to me it seems that the kernel
>> is in the ideal position to automatically inhibit some devices when
>> some EV_SW transitions from 0->1 (and uninhibit again on 1->0). The
>> issue here is to chose on which devices to enable this. I believe
>> that the auto inhibit on some switches mechanism is best done inside
>> the kernel (disabled by default) and then we can have a sysfs
>> attr called auto_inhibit_ev_sw_mask which can be set to e.g.
>> (1 << SW_LID) to make the kernel auto-inhibit the input-device whenever
>> the lid is closed, or to ((1 << SW_LID) | (1 << SW_TABLET_MODE)) to
>> inhibit both when the lid is closed or when switched to tablet mode.
>
> I agree that the kernel is the right place to handle this, but it
> requires some extra knowledge about dependencies between devices.
>
> It'd be kind of like power resources in ACPI, so for each state of a
> "master" device (in principle, there may be more states of it than
> just two) there would be a list of "dependent" intput devices that
> need to be inhibited when the "master" device goes into that state.
So a big part of the reason to punt the decision on which input
devices to enable this auto-inhibit is that we don't really have
information about those relationsships / device-links you are
suggesting here. libinput is already doing inhibiting inside
userspace for e.g. the tablet-mode switch but it relies on heuristics
+ quirk tables to decide which keyboards should be inhibited and which
not.
E.g. for a 360 degree hinges 2-in-1 we want to disable the builtin
keyboard, when folded into in tablet mode, but not any external ones.
Mostly the builtin kbd will be PS2 but I have one such 2-in-1 here
in my home office with a USB kbd ...
In general of the master devices there will be only 1, there will be
only 1 lid switch and only 1 tablet-mode switch. So my idea with the
auto_inhibit_ev_sw_mask, is for it to be a per input-device setting.
So using your terms, all input devices with the (1 << SW_LID) bit
set in their auto_inhibit_ev_sw_mask will be dependents of the
(master) device which actually is reporting the SW_LID bit.
The idea here is for this to work the same as how the rfkill code
from net/rfkill/input.c works, except instead of binding e.g.
KEY_WLAN to toggling the sw-state of rfkill devices with a type
of RFKILL_TYPE_WLAN. This will bind SW_LID to inhibiting input
devices with the SW_LID bit set in their auto_inhibit_ev_sw_mask.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Andrzej Pietrasiewicz @ 2020-06-10 13:41 UTC (permalink / raw)
To: Hans de Goede, Rafael J. Wysocki
Cc: Linux PM, ACPI Devel Maling List, Linux Kernel Mailing List,
linux-iio, Linux ARM, Linux Samsung SoC, linux-input, linux-tegra,
patches, ibm-acpi-devel, Platform Driver, Rafael J . Wysocki,
Len Brown, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Kukjin Kim
In-Reply-To: <3e61c9c1-b211-da9f-c55b-b44eb6522f2a@redhat.com>
Hi Hans,
W dniu 10.06.2020 o 15:21, Hans de Goede pisze:
> Hi,
>
> On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote:
>> Hi All,
>>
>> W dniu 10.06.2020 o 12:38, Rafael J. Wysocki pisze:
>>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
>>>>> This is a quick respin of v3, with just two small changes, please see
>>>>> the changelog below.
>>>>>
>>>>> Userspace might want to implement a policy to temporarily disregard input
>>>>> from certain devices.
>>>>>
>>>>> An example use case is a convertible laptop, whose keyboard can be folded
>>>>> under the screen to create tablet-like experience. The user then must hold
>>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>>>> keys. It is therefore desirable to temporarily disregard input from the
>>>>> keyboard, until it is folded back. This obviously is a policy which should
>>>>> be kept out of the kernel, but the kernel must provide suitable means to
>>>>> implement such a policy.
>>>>
>>>> First of all sorry to start a somewhat new discussion about this
>>>> while this patch set is also somewhat far along in the review process,
>>>> but I believe what I discuss below needs to be taken into account.
>>>>
>>>> Yesterday I have been looking into why an Asus T101HA would not stay
>>>> suspended when the LID is closed. The cause is that the USB HID multi-touch
>>>> touchpad in the base of the device starts sending events when the screen
>>>> gets close to the touchpad (so when the LID is fully closed) and these
>>>> events are causing a wakeup from suspend. HID multi-touch devices
>>>> do have a way to tell them to fully stop sending events, also disabling
>>>> the USB remote wakeup the device is doing. The question is when to tell
>>>> it to not send events though ...
>>>>
>>>> So now I've been thinking about how to fix this and I believe that there
>>>> is some interaction between this problem and this patch-set.
>>>>
>>>> The problem I'm seeing on the T101HA is about wakeups, so the question
>>>> which I want to discuss is:
>>>>
>>>> 1. How does inhibiting interact with enabling /
>>>> disabling the device as a wakeup source ?
>>>>
>>>> 2. Since we have now made inhibiting equal open/close how does open/close
>>>> interact with a device being a wakeup source ?
>>>>
>>>> And my own initial (to be discussed) answers to these questions:
>>>>
>>>> 1. It seems to me that when a device is inhibited it should not be a
>>>> wakeup source, so where possible a input-device-driver should disable
>>>> a device's wakeup capabilities on suspend if inhibited
>>>
>>> If "inhibit" means "do not generate any events going forward", then
>>> this must also cover wakeup events, so I agree.
>>
>> I agree, too.
>>
>>>
>>>> 2. This one is trickier I don't think we have really clearly specified
>>>> any behavior here. The default behavior of most drivers seems to be
>>>> using something like this in their suspend callback:
>>>>
>>>> if (device_may_wakeup(dev))
>>>> enable_irq_wake(data->irq);
>>>> else if (input->users)
>>>> foo_stop_receiving_events(data);
>>>>
>>>> Since this is what most drivers seem to do I believe we should keep
>>>> this as is and that we should just clearly document that if the
>>>> input_device has users (has been opened) or not does not matter
>>>> for its wakeup behavior.
>>>>
>>>> Combining these 2 answers leads to this new pseudo code template
>>>> for an input-device's suspend method:
>>>>
>>>> /*
>>>> * If inhibited we have already disabled events and
>>>> * we do NOT want to setup the device as wake source.
>>>> */
>>>> if (input->inhibited)
>>>> return 0;
>>
>> Right, if a device is inhibited it shouldn't become a wakeup source,
>> because that would contradict the purpose of being inhibited.
>
> Ack. Note I do think that we need to document this (and more
> in general the answer to both questions from above) clearly so
> that going forward if there are any questions about how this is
> supposed to work we can just point to the docs.
>
> Can you do a follow-up patch, or include a patch in your next
> version which documents this (once we agree on what "this"
> exactly is) ?
Sure I can. Just need to know when "this" becomes stable enough ;)
If this series otherwise looks mature enough I would opt for a
follow-up patch.
>
>>>>
>>>> if (device_may_wakeup(dev))
>>>> enable_irq_wake(data->irq);
>>
>> What would it mean to become a wakeup source if there are no users,
>> or nobody has ever opened the device? There are no interested
>> input handlers (users) so what's the point of becoming a wakeup
>> source? Why would the system need to wake up?
>
> Well this is what we have been doing so far, so arguably we
> need to keep doing it to avoid regressions / breaking our ABI.
>
> Lets for example take a laptop, where when suspended the
> power-button is the only valid wakeup-source and this is
> running good old slackware with fvwm2 or windowmaker as
> "desktop environment", then likely no process will have
> the power-button input evdev node open. Still we should
> wakeup the laptop on the power-button press, otherwise
> it will never wakeup.
>
True, thanks for explaining.
> Note I agree with you that the way this works is not
> ideal, I just do not think that we can change it.
>
Regards,
Andrzej
^ permalink raw reply
* Re: [RFC PATCH v1 13/18] gpu: host1x: mipi: Add of_tegra_mipi_request() API
From: Dmitry Osipenko @ 2020-06-10 13:33 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
jonathanh-DDmLM1+adcrQT0dZR+AlfA, frankc-DDmLM1+adcrQT0dZR+AlfA,
hverkuil-qWit8jRvyhVmR6Xm/wNWPw, sakari.ailus-X3B1VOXEql0,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
helen.koike-ZGY8ohtN/8qB+jHODAdFcQ
Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1591768960-31648-14-git-send-email-skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
10.06.2020 09:02, Sowjanya Komatineni пишет:
> This patch adds an API of_tegra_mipi_request() to allow creating
> mipi device for specific device node rather than a device so Tegra
> CSI driver can use it for calibrating MIPI pads for each stream
> independently.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/gpu/host1x/mipi.c | 10 ++++++++--
> include/linux/host1x.h | 2 ++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
> index e00809d..f51fe69 100644
> --- a/drivers/gpu/host1x/mipi.c
> +++ b/drivers/gpu/host1x/mipi.c
> @@ -206,9 +206,9 @@ static int tegra_mipi_power_down(struct tegra_mipi *mipi)
> return 0;
> }
>
> -struct tegra_mipi_device *tegra_mipi_request(struct device *device)
> +struct tegra_mipi_device *of_tegra_mipi_request(struct device *device,
> + struct device_node *np)
> {
> - struct device_node *np = device->of_node;
> struct tegra_mipi_device *dev;
> struct of_phandle_args args;
> int err;
> @@ -252,6 +252,12 @@ struct tegra_mipi_device *tegra_mipi_request(struct device *device)
> of_node_put(args.np);
> return ERR_PTR(err);
> }
> +EXPORT_SYMBOL(of_tegra_mipi_request);
> +
> +struct tegra_mipi_device *tegra_mipi_request(struct device *device)
> +{
> + return of_tegra_mipi_request(device, device->of_node);
> +}
> EXPORT_SYMBOL(tegra_mipi_request);
>
> void tegra_mipi_free(struct tegra_mipi_device *device)
> diff --git a/include/linux/host1x.h b/include/linux/host1x.h
> index c230b4e..a61ca52 100644
> --- a/include/linux/host1x.h
> +++ b/include/linux/host1x.h
> @@ -325,6 +325,8 @@ int host1x_client_resume(struct host1x_client *client);
>
> struct tegra_mipi_device;
>
> +struct tegra_mipi_device *of_tegra_mipi_request(struct device *device,
> + struct device_node *np);
Looks like adding a new function here is a bit of overkill. What about
to change tegra_mipi_request() to take the node argument?
^ permalink raw reply
* Re: [PATCH v4 0/7] Support inhibiting input devices
From: Hans de Goede @ 2020-06-10 13:21 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, Rafael J. Wysocki
Cc: Collabora Kernel ML, Nick Dyer, linux-iio, Platform Driver,
ibm-acpi-devel, Laxman Dewangan, Peter Meerwald-Stadler,
Peter Hutterer, Fabio Estevam, Linux Samsung SoC,
Krzysztof Kozlowski, Jonathan Hunter, ACPI Devel Maling List,
Kukjin Kim, NXP Linux Team, linux-input, Len Brown,
Michael Hennerich, Linux PM, Sascha Hauer, Sylvain Lemieux,
Henrique de Moraes Holschuh
In-Reply-To: <6136f26c-e090-e025-af55-4c5f3a6aec92@collabora.com>
Hi,
On 6/10/20 3:12 PM, Andrzej Pietrasiewicz wrote:
> Hi All,
>
> W dniu 10.06.2020 o 12:38, Rafael J. Wysocki pisze:
>> On Wed, Jun 10, 2020 at 11:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi All,
>>>
>>> On 6/8/20 1:22 PM, Andrzej Pietrasiewicz wrote:
>>>> This is a quick respin of v3, with just two small changes, please see
>>>> the changelog below.
>>>>
>>>> Userspace might want to implement a policy to temporarily disregard input
>>>> from certain devices.
>>>>
>>>> An example use case is a convertible laptop, whose keyboard can be folded
>>>> under the screen to create tablet-like experience. The user then must hold
>>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>>> keys. It is therefore desirable to temporarily disregard input from the
>>>> keyboard, until it is folded back. This obviously is a policy which should
>>>> be kept out of the kernel, but the kernel must provide suitable means to
>>>> implement such a policy.
>>>
>>> First of all sorry to start a somewhat new discussion about this
>>> while this patch set is also somewhat far along in the review process,
>>> but I believe what I discuss below needs to be taken into account.
>>>
>>> Yesterday I have been looking into why an Asus T101HA would not stay
>>> suspended when the LID is closed. The cause is that the USB HID multi-touch
>>> touchpad in the base of the device starts sending events when the screen
>>> gets close to the touchpad (so when the LID is fully closed) and these
>>> events are causing a wakeup from suspend. HID multi-touch devices
>>> do have a way to tell them to fully stop sending events, also disabling
>>> the USB remote wakeup the device is doing. The question is when to tell
>>> it to not send events though ...
>>>
>>> So now I've been thinking about how to fix this and I believe that there
>>> is some interaction between this problem and this patch-set.
>>>
>>> The problem I'm seeing on the T101HA is about wakeups, so the question
>>> which I want to discuss is:
>>>
>>> 1. How does inhibiting interact with enabling /
>>> disabling the device as a wakeup source ?
>>>
>>> 2. Since we have now made inhibiting equal open/close how does open/close
>>> interact with a device being a wakeup source ?
>>>
>>> And my own initial (to be discussed) answers to these questions:
>>>
>>> 1. It seems to me that when a device is inhibited it should not be a
>>> wakeup source, so where possible a input-device-driver should disable
>>> a device's wakeup capabilities on suspend if inhibited
>>
>> If "inhibit" means "do not generate any events going forward", then
>> this must also cover wakeup events, so I agree.
>
> I agree, too.
>
>>
>>> 2. This one is trickier I don't think we have really clearly specified
>>> any behavior here. The default behavior of most drivers seems to be
>>> using something like this in their suspend callback:
>>>
>>> if (device_may_wakeup(dev))
>>> enable_irq_wake(data->irq);
>>> else if (input->users)
>>> foo_stop_receiving_events(data);
>>>
>>> Since this is what most drivers seem to do I believe we should keep
>>> this as is and that we should just clearly document that if the
>>> input_device has users (has been opened) or not does not matter
>>> for its wakeup behavior.
>>>
>>> Combining these 2 answers leads to this new pseudo code template
>>> for an input-device's suspend method:
>>>
>>> /*
>>> * If inhibited we have already disabled events and
>>> * we do NOT want to setup the device as wake source.
>>> */
>>> if (input->inhibited)
>>> return 0;
>
> Right, if a device is inhibited it shouldn't become a wakeup source,
> because that would contradict the purpose of being inhibited.
Ack. Note I do think that we need to document this (and more
in general the answer to both questions from above) clearly so
that going forward if there are any questions about how this is
supposed to work we can just point to the docs.
Can you do a follow-up patch, or include a patch in your next
version which documents this (once we agree on what "this"
exactly is) ?
>>>
>>> if (device_may_wakeup(dev))
>>> enable_irq_wake(data->irq);
>
> What would it mean to become a wakeup source if there are no users,
> or nobody has ever opened the device? There are no interested
> input handlers (users) so what's the point of becoming a wakeup
> source? Why would the system need to wake up?
Well this is what we have been doing so far, so arguably we
need to keep doing it to avoid regressions / breaking our ABI.
Lets for example take a laptop, where when suspended the
power-button is the only valid wakeup-source and this is
running good old slackware with fvwm2 or windowmaker as
"desktop environment", then likely no process will have
the power-button input evdev node open. Still we should
wakeup the laptop on the power-button press, otherwise
it will never wakeup.
Note I agree with you that the way this works is not
ideal, I just do not think that we can change it.
>>> else if (input->users)
>>> foo_stop_receiving_events(data);
>>>
>>> ###
<snip>
Regards,
Hans
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] drm/tegra: Add zpos property for cursor planes
From: Dmitry Osipenko @ 2020-06-10 13:16 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jon Hunter
In-Reply-To: <20200610113059.2164132-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
10.06.2020 14:30, Thierry Reding пишет:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
> have a zpos property") a warning is emitted if there's a mix of planes
> with and without a zpos property.
>
> On Tegra, cursor planes are always composited on top of all other
> planes, which is why they never had a zpos property attached to them.
> However, since the composition order is fixed, this is trivial to
> remedy by simply attaching an immutable zpos property to them.
>
> Reported-by: Jonathan Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
What problem does it solve?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox