* Re: [PATCH 00/14] chrome_laptop: stop being a platform driver
From: Benson Leung @ 2018-03-12 19:17 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Benson Leung, Nick Dyer, Olof Johansson,
linux-kernel, bleung
In-Reply-To: <20180312190907.174301-1-dmitry.torokhov@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]
On Mon, Mar 12, 2018 at 12:08:53PM -0700, Dmitry Torokhov wrote:
> Hi,
>
> This series is a combination of Atmel touchscreen driver stopping using
> platform data and moving over to generic device properties, and
> chromeos-laptop switching from being platform driver, which is the wrong
> abstraction for it, and moving to using i2c bis notifier. Switching from
> platform driver to the notifiers allows us to get rid of the ugly code that
> manually tries to handle deferrals in case i2c bus is not ready at the time
> we start initializing the module.
>
> Benson, because chromeos-laptop is instantiating Atmel devices the 2
> changes are intertwined. While we could apply them in stages, with parts
> going through input and parts going through your tree I was wondering if we
> could speed up the process and push the series through one or another tree
> (if you are OK with the patches, of course).
>
I'm all for this change. I can push this through my tree.
Thanks,
Benson
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 0/4] new driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-03-12 20:51 UTC (permalink / raw)
To: Clément VUCHENER
Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, lkml, linux-input
In-Reply-To: <CAM4jgCq5UT0SBKA1DMfsAKd8R+yn0ViPr0U-iE8-tWjgA9hTTQ@mail.gmail.com>
On Mon, Mar 12, 2018 at 03:30:43PM +0100, Clément VUCHENER wrote:
> 2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>:
> > This patchset implements a driver for Valve Steam Controller, based on a
> > reverse analysis by myself.
> >
> > Sorry, I've been out of town for a few weeks and couldn't keep up with this...
> >
> > @Pierre-Loup and @Clément, could you please have another look at this and
> > check if it is worthy? Benjamin will not commit it without an express ACK from
> > Valve. Of course he is right to be cautious, but I checked this driver with
> > the Steam Client and all seems to go just fine. I think that there is a lot of
> > Linux out of the desktop that could use this driver and cannot use the Steam
> > Client. Worst case scenario, this driver can now be blacklisted, but I hope
> > that will not be needed.
>
> I tested the driver with my 4.15 fedora kernel (I only built the
> module not the whole kernel) and I got double inputs (your driver
> input device + steam uinput device) when testing Shovel Knight with
> Steam Big Picture. It seems to work fine when the inputs are the same,
> but after changing the controller configuration in Steam, the issue
> became apparent.
I assumed that when several joysticks are available, games would listen
to one one of them. It looks like I'm wrong, and some (many?) games will
listen to all available joysticks at the same time. Thus having two
logical joysticks that represent the same physical one is not good.
An easy solution would be that Steam Client grabs this driver
(ioctl(EVIOCGRAB)) when creating the uinput device. Another solution
would be that Steam Client blacklists this driver, of course.
> And without Steam and your external tool, you get double inputs too. I
> tried RetroArch and it was unusable because of the keyboard inputs
> from the lizard mode (e.g. pressing B also presses Esc and quits
> RetroArch). Having to download and compile an external tool to make
> the driver work properly may be too difficult for the user. Your goal
> was to provide an alternative to user space drivers but now you
> actually depend on (a very simple) one.
Yes, I noticed that. TBH, this driver without Steam Client or the
user-space tool is not very nice, precisely because you'll get constant
Escape and Enter presses, and most games react to those.
Frankly speaking, I'm not sure how to proceed. I can think of the
following options:
1.Steam Client installation could add a file to blacklist
hid-steam, just as it adds a few udev rules.
2.The default CONFIG_HID_STEAM can be changed to "n". Maybe only
on the architectures for which there is a Steam Client available.
This way DIY projects will still be able to use it.
3.This driver could be abandoned :-(. Just use Steam Client if possible or
any of the user-mode drivers available.
If we decide for 1 or 2, then the lizard mode could be disabled without
ill effects. We could even enable the gyro and all the other gadgets
without worring about current compatibility.
At the end of the day, I think that it is up to Valve what to do.
Best Regards.
Rodrigo.
> Also the button and axis codes do not match the gamepad API doc
> (https://www.kernel.org/doc/Documentation/input/gamepad.txt).
>
> >
> > For full reference, I'm adding a full changelog of this patchset.
> >
> > Changes in v5:
> > * Fix license SPDX to GPL-2.0+.
> > * Minor stylistic changes (BIT(3) instead 0x08 and so on).
> >
> > Changes in v4:
> > * Add command to check the wireless connection status on probe, without
> > waiting for a message (thanks to Clément Vuchener for the tip).
> > * Removed the error code on redundant connection/disconnection messages. That
> > was harmless but polluted dmesg.
> > * Added buttons for touching the left-pad and right-pad.
> > * Fixed a misplaced #include from 2/4 to 1/4.
> >
> > Changes in v3:
> > * Use RCU to do the dynamic connec/disconnect of wireless devices.
> > * Remove entries in hid-quirks.c as they are no longer needed. This allows
> > this module to be blacklisted without side effects.
> > * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> > existing use cases (lizard mode). A user-space tool to do that is
> > linked.
> > * Fully separated axes for joystick and left-pad. As it happens.
> > * Add fuzz values for left/right pad axes, they are a little wiggly.
> >
> > Changes in v2:
> > * Remove references to USB. Now the interesting interfaces are selected by
> > looking for the ones with feature reports.
> > * Feature reports buffers are allocated with hid_alloc_report_buf().
> > * Feature report length is checked, to avoid overflows in case of
> > corrupt/malicius USB devices.
> > * Resolution added to the ABS axes.
> > * A lot of minor cleanups.
> >
> > Rodrigo Rivas Costa (4):
> > HID: add driver for Valve Steam Controller
> > HID: steam: add serial number information.
> > HID: steam: command to check wireless connection
> > HID: steam: add battery device.
> >
> > drivers/hid/Kconfig | 8 +
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 4 +
> > drivers/hid/hid-steam.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 807 insertions(+)
> > create mode 100644 drivers/hid/hid-steam.c
> >
> > --
> > 2.16.2
> >
^ permalink raw reply
* [PATCH] Input: alps - Demystify trackstick initialization for v3 and v6 protocols
From: Pali Rohár @ 2018-03-12 23:09 UTC (permalink / raw)
To: Dmitry Torokhov, Masaki Ota; +Cc: linux-input, linux-kernel
Remove cite "Not sure what this does, but it is absolutely essential".
Extract initialization of trackstick part when touchpad is in passthrought
mode for v3 and v6 protocols into own function. Initialization for v3 is:
setscale11, setscale11, setscale11, nibble 0x9, nibble 0x4. Initialization
for v6 is: setscale11, setscale11, setscale11, setrate 0xC8, setrate 0x14.
Nibbles 0x9 and 0x4 for v3 protocol correspondent to setrate 0xC8 and 0x14,
therefore these sequences are same.
When touchpad is in passthrought mode, then OS communicates with trackstick
and this sequence is some magic vendor PS/2 command to put trackstick into
"extended" mode. After that sequence trackstick starts reporting packets in
some vendor 4 bytes format (first byte is always 0xE8).
Next step after configuring trackstick to be in "extended" mode, is to
configure touchpad for v3 protocol to expect that trackstick reports data
in "extended" mode. For v3 protocol this is done by setting bit 1 in
register 0xC2C8 (offset 0x08 from base address 0xC2C0).
When both touchpad and trackstick are not configured for "extended" mode
then touchpad reports trackstick packets in different format, which is not
supported by psmouse/alps driver (yet).
In Cirque documentation GP-AN- 130823 INTERFACING TO GEN4 OVER I2C (PDF)
available at http://www.cirque.com/gen4-dev-resources is Logical Address
0xC2C8 named as PS2AuxControl and Bit Number 1 as ProcessAuxExtendedData
with description: Auxiliary device data is assumed to be extended data when
set.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
drivers/input/mouse/alps.c | 80 ++++++++++++++++++++++++++++------------------
1 file changed, 49 insertions(+), 31 deletions(-)
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index dbe57da8c1a1..010c1bcdb06d 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2063,14 +2063,11 @@ static int alps_hw_init_v1_v2(struct psmouse *psmouse)
return 0;
}
-static int alps_hw_init_v6(struct psmouse *psmouse)
+/* Must be in passthrough mode when calling this function */
+static int alps_trackstick_enter_extended_mode_v3_v6(struct psmouse *psmouse)
{
unsigned char param[2] = {0xC8, 0x14};
- /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
- if (alps_passthrough_mode_v2(psmouse, true))
- return -1;
-
if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
@@ -2078,9 +2075,25 @@ static int alps_hw_init_v6(struct psmouse *psmouse)
ps2_command(&psmouse->ps2dev, ¶m[1], PSMOUSE_CMD_SETRATE))
return -1;
+ return 0;
+}
+
+static int alps_hw_init_v6(struct psmouse *psmouse)
+{
+ int ret;
+
+ /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
+ if (alps_passthrough_mode_v2(psmouse, true))
+ return -1;
+
+ ret = alps_trackstick_enter_extended_mode_v3_v6(psmouse);
+
if (alps_passthrough_mode_v2(psmouse, false))
return -1;
+ if (ret)
+ return ret;
+
if (alps_absolute_mode_v6(psmouse)) {
psmouse_err(psmouse, "Failed to enable absolute mode\n");
return -1;
@@ -2154,10 +2167,18 @@ static int alps_probe_trackstick_v3_v7(struct psmouse *psmouse, int reg_base)
static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
{
- struct ps2dev *ps2dev = &psmouse->ps2dev;
int ret = 0;
+ int reg_val;
unsigned char param[4];
+ /*
+ * We need to configure trackstick to report data for touchpad in
+ * extended format. And also we need to tell touchpad to expect data
+ * from trackstick in extended format. Without this configuration
+ * trackstick packets sent from touchpad are in basic format which is
+ * different from what we expect.
+ */
+
if (alps_passthrough_mode_v3(psmouse, reg_base, true))
return -EIO;
@@ -2175,39 +2196,36 @@ static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
ret = -ENODEV;
} else {
psmouse_dbg(psmouse, "trackstick E7 report: %3ph\n", param);
-
- /*
- * Not sure what this does, but it is absolutely
- * essential. Without it, the touchpad does not
- * work at all and the trackstick just emits normal
- * PS/2 packets.
- */
- if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
- ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
- ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
- alps_command_mode_send_nibble(psmouse, 0x9) ||
- alps_command_mode_send_nibble(psmouse, 0x4)) {
- psmouse_err(psmouse,
- "Error sending magic E6 sequence\n");
+ if (alps_trackstick_enter_extended_mode_v3_v6(psmouse)) {
+ psmouse_err(psmouse, "Failed to enter into trackstick extended mode\n");
ret = -EIO;
- goto error;
}
+ }
+
+ if (alps_passthrough_mode_v3(psmouse, reg_base, false))
+ return -EIO;
+
+ if (ret)
+ return ret;
+ if (alps_enter_command_mode(psmouse))
+ return -EIO;
+
+ reg_val = alps_command_mode_read_reg(psmouse, reg_base + 0x08);
+ if (reg_val == -1) {
+ ret = -EIO;
+ } else {
/*
- * This ensures the trackstick packets are in the format
- * supported by this driver. If bit 1 isn't set the packet
- * format is different.
+ * Tell touchpad that trackstick is now in extended mode.
+ * If bit 1 isn't set the packet format is different.
*/
- if (alps_enter_command_mode(psmouse) ||
- alps_command_mode_write_reg(psmouse,
- reg_base + 0x08, 0x82) ||
- alps_exit_command_mode(psmouse))
+ reg_val |= BIT(1);
+ if (__alps_command_mode_write_reg(psmouse, reg_val))
ret = -EIO;
}
-error:
- if (alps_passthrough_mode_v3(psmouse, reg_base, false))
- ret = -EIO;
+ if (alps_exit_command_mode(psmouse))
+ return -EIO;
return ret;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] Input: alps - Demystify trackstick initialization for v3 and v6 protocols
From: Pali Rohár @ 2018-03-12 23:13 UTC (permalink / raw)
To: Masaki Ota, Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20180312230915.26050-1-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6552 bytes --]
Masaki, if you have access to the internal ALPS v3 / Rushmore
documentation, I would like to have a review of this patch or
confirmation of those information :-)
On Tuesday 13 March 2018 00:09:15 Pali Rohár wrote:
> Remove cite "Not sure what this does, but it is absolutely essential".
>
> Extract initialization of trackstick part when touchpad is in passthrought
> mode for v3 and v6 protocols into own function. Initialization for v3 is:
> setscale11, setscale11, setscale11, nibble 0x9, nibble 0x4. Initialization
> for v6 is: setscale11, setscale11, setscale11, setrate 0xC8, setrate 0x14.
> Nibbles 0x9 and 0x4 for v3 protocol correspondent to setrate 0xC8 and 0x14,
> therefore these sequences are same.
>
> When touchpad is in passthrought mode, then OS communicates with trackstick
> and this sequence is some magic vendor PS/2 command to put trackstick into
> "extended" mode. After that sequence trackstick starts reporting packets in
> some vendor 4 bytes format (first byte is always 0xE8).
>
> Next step after configuring trackstick to be in "extended" mode, is to
> configure touchpad for v3 protocol to expect that trackstick reports data
> in "extended" mode. For v3 protocol this is done by setting bit 1 in
> register 0xC2C8 (offset 0x08 from base address 0xC2C0).
>
> When both touchpad and trackstick are not configured for "extended" mode
> then touchpad reports trackstick packets in different format, which is not
> supported by psmouse/alps driver (yet).
>
> In Cirque documentation GP-AN- 130823 INTERFACING TO GEN4 OVER I2C (PDF)
> available at http://www.cirque.com/gen4-dev-resources is Logical Address
> 0xC2C8 named as PS2AuxControl and Bit Number 1 as ProcessAuxExtendedData
> with description: Auxiliary device data is assumed to be extended data when
> set.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> drivers/input/mouse/alps.c | 80 ++++++++++++++++++++++++++++------------------
> 1 file changed, 49 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index dbe57da8c1a1..010c1bcdb06d 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2063,14 +2063,11 @@ static int alps_hw_init_v1_v2(struct psmouse *psmouse)
> return 0;
> }
>
> -static int alps_hw_init_v6(struct psmouse *psmouse)
> +/* Must be in passthrough mode when calling this function */
> +static int alps_trackstick_enter_extended_mode_v3_v6(struct psmouse *psmouse)
> {
> unsigned char param[2] = {0xC8, 0x14};
>
> - /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
> - if (alps_passthrough_mode_v2(psmouse, true))
> - return -1;
> -
> if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> @@ -2078,9 +2075,25 @@ static int alps_hw_init_v6(struct psmouse *psmouse)
> ps2_command(&psmouse->ps2dev, ¶m[1], PSMOUSE_CMD_SETRATE))
> return -1;
>
> + return 0;
> +}
> +
> +static int alps_hw_init_v6(struct psmouse *psmouse)
> +{
> + int ret;
> +
> + /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
> + if (alps_passthrough_mode_v2(psmouse, true))
> + return -1;
> +
> + ret = alps_trackstick_enter_extended_mode_v3_v6(psmouse);
> +
> if (alps_passthrough_mode_v2(psmouse, false))
> return -1;
>
> + if (ret)
> + return ret;
> +
> if (alps_absolute_mode_v6(psmouse)) {
> psmouse_err(psmouse, "Failed to enable absolute mode\n");
> return -1;
> @@ -2154,10 +2167,18 @@ static int alps_probe_trackstick_v3_v7(struct psmouse *psmouse, int reg_base)
>
> static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
> {
> - struct ps2dev *ps2dev = &psmouse->ps2dev;
> int ret = 0;
> + int reg_val;
> unsigned char param[4];
>
> + /*
> + * We need to configure trackstick to report data for touchpad in
> + * extended format. And also we need to tell touchpad to expect data
> + * from trackstick in extended format. Without this configuration
> + * trackstick packets sent from touchpad are in basic format which is
> + * different from what we expect.
> + */
> +
> if (alps_passthrough_mode_v3(psmouse, reg_base, true))
> return -EIO;
>
> @@ -2175,39 +2196,36 @@ static int alps_setup_trackstick_v3(struct psmouse *psmouse, int reg_base)
> ret = -ENODEV;
> } else {
> psmouse_dbg(psmouse, "trackstick E7 report: %3ph\n", param);
> -
> - /*
> - * Not sure what this does, but it is absolutely
> - * essential. Without it, the touchpad does not
> - * work at all and the trackstick just emits normal
> - * PS/2 packets.
> - */
> - if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> - ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> - ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> - alps_command_mode_send_nibble(psmouse, 0x9) ||
> - alps_command_mode_send_nibble(psmouse, 0x4)) {
> - psmouse_err(psmouse,
> - "Error sending magic E6 sequence\n");
> + if (alps_trackstick_enter_extended_mode_v3_v6(psmouse)) {
> + psmouse_err(psmouse, "Failed to enter into trackstick extended mode\n");
> ret = -EIO;
> - goto error;
> }
> + }
> +
> + if (alps_passthrough_mode_v3(psmouse, reg_base, false))
> + return -EIO;
> +
> + if (ret)
> + return ret;
>
> + if (alps_enter_command_mode(psmouse))
> + return -EIO;
> +
> + reg_val = alps_command_mode_read_reg(psmouse, reg_base + 0x08);
> + if (reg_val == -1) {
> + ret = -EIO;
> + } else {
> /*
> - * This ensures the trackstick packets are in the format
> - * supported by this driver. If bit 1 isn't set the packet
> - * format is different.
> + * Tell touchpad that trackstick is now in extended mode.
> + * If bit 1 isn't set the packet format is different.
> */
> - if (alps_enter_command_mode(psmouse) ||
> - alps_command_mode_write_reg(psmouse,
> - reg_base + 0x08, 0x82) ||
> - alps_exit_command_mode(psmouse))
> + reg_val |= BIT(1);
> + if (__alps_command_mode_write_reg(psmouse, reg_val))
> ret = -EIO;
> }
>
> -error:
> - if (alps_passthrough_mode_v3(psmouse, reg_base, false))
> - ret = -EIO;
> + if (alps_exit_command_mode(psmouse))
> + return -EIO;
>
> return ret;
> }
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH v5 06/36] drm/rockchip: Only wait for panel ACK on PSR entry
From: Enric Balletbo i Serra @ 2018-03-13 0:26 UTC (permalink / raw)
To: Heiko Stübner
Cc: architt, inki.dae, thierry.reding, hjc, seanpaul, airlied, tfiga,
dri-devel, dianders, a.hajda, ykk, kernel, m.szyprowski,
linux-samsung-soc, jy0922.shim, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, Laurent.pinchart
In-Reply-To: <1909495.WV5cT3kS9W@diego>
On 12/03/18 17:58, Heiko Stübner wrote:
> Hi,
>
> the subject is misleading I think, as this is touching only the generic
> bridge code and not anything Rockchip-related, so should probably
> be "drm/bridge"?
>
Right, I'll fix this when I need to send another version, I'll wait, for now, a
bit more for reviews on the other patches.
Thanks
Enric
> Am Freitag, 9. März 2018, 23:22:57 CET schrieb Enric Balletbo i Serra:
>> From: zain wang <wzz@rock-chips.com>
>>
>> We currently wait for the panel to mirror our intended PSR state
>> before continuing on both PSR enter and PSR exit. This is really
>> only important to do when we're entering PSR, since we want to
>> be sure the last frame we pushed is being served from the panel's
>> internal fb before shutting down the soc blocks (vop/analogix).
>>
>> This patch changes the behavior such that we only wait for the
>> panel to complete the PSR transition when we're entering PSR, and
>> to skip verification when we're exiting.
>>
>> Cc: Stéphane Marchesin <marcheu@chromium.org>
>> Cc: Sonny Rao <sonnyrao@chromium.org>
>> Signed-off-by: zain wang <wzz@rock-chips.com>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> change itself looks good, so
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
^ permalink raw reply
* Re: [PATCH v5 2/4] rtc: at91sam9: Set name of regmap_config
From: Alexandre Belloni @ 2018-03-13 0:37 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, broonie, Guenter Roeck, Arnd Bergmann, Joseph Lo,
Rob Herring, Catalin Marinas, lee.jones, Emil Renner Berthing,
Heiko Stuebner, Brian Norris, Thomas Gleixner,
Philippe Ombredanne, linux-rockchip, Kate Stewart, linux-input,
Will Deacon, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav <arvind.yadav.cs@
In-Reply-To: <20180312095142.30854-3-jeffy.chen@rock-chips.com>
On 12/03/2018 at 17:51:40 +0800, Jeffy Chen wrote:
> We are now allowing to register debugfs without a valid device, and not
> having a valid name will end up using "dummy*" to create debugfs dir.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v5: None
> Changes in v4:
> Not to use rtc dev to init regmap, the gpbr is not directly related to
> the rtc.
>
> Changes in v3: None
>
> drivers/rtc/rtc-at91sam9.c | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH V1] Input: pm8941-pwrkey: add resin key capabilities
From: Tirupathi Reddy T @ 2018-03-13 8:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: robh+dt, mark.rutland, linux-input, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20180310182714.GD260013@dtor-ws>
Hi Dmitry,
On 3/10/2018 11:57 PM, Dmitry Torokhov wrote:
> On Wed, Mar 07, 2018 at 12:21:47PM -0800, Dmitry Torokhov wrote:
>> Hi Tirupathi,
>>
>> On Wed, Mar 07, 2018 at 01:39:33PM +0530, Tirupathi Reddy wrote:
>>> Add resin key support to handle different types of key events
>>> defined in different platforms.
> Also, when you say "resin" key, I do not suppose it refers to teh
> material of the key, but rather have something to so with "reset in"
> signal or similar when is not actually used for resetting the device?
The resin is a "reset in" line to the "Power Management IC".
Usually in a regular usecase this line is used for "volume down key"
functionality and in other cases to reset the device on a long-press.
>>> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
>>> ---
>>> .../bindings/input/qcom,pm8941-pwrkey.txt | 20 ++++++-
>>> drivers/input/misc/pm8941-pwrkey.c | 63 +++++++++++++++++++++-
>>> 2 files changed, 81 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> index 07bf55f..1c437e9 100644
>>> --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> @@ -20,6 +20,14 @@ PROPERTIES
>>> defined by the binding document describing the node's
>>> interrupt parent.
>>>
>>> +- interrupt-names:
>>> + Usage: required
>>> + Value type: <stringlist>
>>> + Definition: Interrupt names. This list must match up 1-to-1 with the
>>> + interrupts specified in the 'interrupts' property. "kpdpwr"
>>> + must be specified. "resin" may be specified for some
>>> + platforms.
>>> +
>>> - debounce:
>>> Usage: optional
>>> Value type: <u32>
>>> @@ -32,12 +40,22 @@ PROPERTIES
>>> Definition: presence of this property indicates that the KPDPWR_N pin
>>> should be configured for pull up.
>>>
>>> +- linux,code:
>>> + Usage: required for "resin" key
>>> + Value type: <u32>
>>> + Definition: The input key-code associated with the resin key.
>>> + Use the linux event codes defined in
>>> + include/dt-bindings/input/linux-event-codes.h
>>> +
>>> EXAMPLE
>>>
>>> pwrkey@800 {
>>> compatible = "qcom,pm8941-pwrkey";
>>> reg = <0x800>;
>>> - interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
>>> + interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
>>> + <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
>>> + interrupt-names = "kpdpwr", "resin";
>>> debounce = <15625>;
>>> bias-pull-up;
>>> + linux,code = <KEY_VOLUMEDOWN>;
>>> };
>>> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
>>> index 18ad956..3f801cf 100644
>>> --- a/drivers/input/misc/pm8941-pwrkey.c
>>> +++ b/drivers/input/misc/pm8941-pwrkey.c
>>> @@ -28,6 +28,7 @@
>>>
>>> #define PON_RT_STS 0x10
>>> #define PON_KPDPWR_N_SET BIT(0)
>>> +#define PON_RESIN_N_SET BIT(1)
>>>
>>> #define PON_PS_HOLD_RST_CTL 0x5a
>>> #define PON_PS_HOLD_RST_CTL2 0x5b
>>> @@ -46,6 +47,8 @@
>>> struct pm8941_pwrkey {
>>> struct device *dev;
>>> int irq;
>>> + int resin_irq;
>>> + u32 resin_key_code;
>>> u32 baseaddr;
>>> struct regmap *regmap;
>>> struct input_dev *input;
>>> @@ -130,6 +133,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
>>> return IRQ_HANDLED;
>>> }
>>>
>>> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
>>> +{
>>> + struct pm8941_pwrkey *pwrkey = _data;
>>> + unsigned int sts;
>>> + int error;
>>> +
>>> + error = regmap_read(pwrkey->regmap,
>>> + pwrkey->baseaddr + PON_RT_STS, &sts);
>>> + if (error)
>>> + return IRQ_HANDLED;
>> It looks like you are reading the same register as the power on key.
>> Won't you lose events if both are pressed at the same time?
>>
>> I think you need a unified interrupt handler...
We are reading the real-time status register where both these
bits would be set and the PMIC arb would dispatch 2 interrupts.
>>> +
>>> + input_report_key(pwrkey->input, pwrkey->resin_key_code,
>>> + !!(sts & PON_RESIN_N_SET));
>>> + input_sync(pwrkey->input);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
>>> {
>>> struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
>>> @@ -153,6 +174,35 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
>>> static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
>>> pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
>>>
>>> +static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey)
>>> +{
>>> + int error;
>>> +
>>> + /*
>>> + * Get the standard-key parameters. This might not be
>>> + * specified if there is no key mapping on the reset line.
>>> + */
>>> + error = of_property_read_u32(pwrkey->dev->of_node, "linux,code",
>>> + &pwrkey->resin_key_code);
>>> + if (error) {
>>> + dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
>>> + return error;
>>> + }
>>> +
>>> + /* Register key configuration */
>>> + input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
>>> +
>>> + error = devm_request_threaded_irq(pwrkey->dev, pwrkey->resin_irq,
>>> + NULL, pm8941_resinkey_irq,
>>> + IRQF_ONESHOT,
>>> + "pm8941_resinkey", pwrkey);
>>> + if (error)
>>> + dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
>>> + error);
>>> +
>>> + return error;
>>> +}
>>> +
>>> static int pm8941_pwrkey_probe(struct platform_device *pdev)
>>> {
>>> struct pm8941_pwrkey *pwrkey;
>>> @@ -182,7 +232,7 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
>>> return -ENODEV;
>>> }
>>>
>>> - pwrkey->irq = platform_get_irq(pdev, 0);
>>> + pwrkey->irq = platform_get_irq_byname(pdev, "kpdpwr");
>> You are breaking support for old DTS here I'm afraid.
Correct. The other way to address it is keep old code and enforce
kpdpwr interrupt should be the first one in interrupt list.
Addressed in Patch V2.
>>> if (pwrkey->irq < 0) {
>>> dev_err(&pdev->dev, "failed to get irq\n");
>>> return pwrkey->irq;
>>> @@ -241,6 +291,17 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
>>> return error;
>>> }
>>>
>>> + pwrkey->resin_irq = platform_get_irq_byname(pdev, "resin");
>>> + if (pwrkey->resin_irq >= 0) {
>> This is not entirely correct. I'd say if you have -ENXIO you can
>> conclude that the support for "resin" key is not enabled, but the rest
>> of the errors you should report up the stack. Deferrals will be retried
>> and other errors are fatal.
Addressed in Patch V2.
>>> + /* resin key capabilities are defined in device node */
>>> + error = pm8941_resin_key_init(pwrkey);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed resin key initialization: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> + }
>>> +
>>> error = input_register_device(pwrkey->input);
>>> if (error) {
>>> dev_err(&pdev->dev, "failed to register input device: %d\n",
>>> --
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>>
>> Thanks.
>>
>> --
>> Dmitry
^ permalink raw reply
* [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities
From: Tirupathi Reddy @ 2018-03-13 8:35 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland
Cc: tirupath, linux-input, linux-arm-msm, devicetree, linux-kernel
Add resin key support to handle different types of key events
defined in different platforms.
Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
---
.../bindings/input/qcom,pm8941-pwrkey.txt | 20 ++++++-
drivers/input/misc/pm8941-pwrkey.c | 65 +++++++++++++++++++++-
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
index 07bf55f..f499cf8 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
+++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
@@ -20,6 +20,14 @@ PROPERTIES
defined by the binding document describing the node's
interrupt parent.
+- interrupt-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Interrupt names. This list must match up 1-to-1 with the
+ interrupts specified in the 'interrupts' property. "kpdpwr"
+ must be specified and should be the first entry of the list.
+ "resin" may be specified for some platforms.
+
- debounce:
Usage: optional
Value type: <u32>
@@ -32,12 +40,22 @@ PROPERTIES
Definition: presence of this property indicates that the KPDPWR_N pin
should be configured for pull up.
+- linux,code:
+ Usage: required for "resin" key
+ Value type: <u32>
+ Definition: The input key-code associated with the resin key.
+ Use the linux event codes defined in
+ include/dt-bindings/input/linux-event-codes.h
+
EXAMPLE
pwrkey@800 {
compatible = "qcom,pm8941-pwrkey";
reg = <0x800>;
- interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
+ interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "kpdpwr", "resin";
debounce = <15625>;
bias-pull-up;
+ linux,code = <KEY_VOLUMEDOWN>;
};
diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
index 18ad956..2fffc7c 100644
--- a/drivers/input/misc/pm8941-pwrkey.c
+++ b/drivers/input/misc/pm8941-pwrkey.c
@@ -28,6 +28,7 @@
#define PON_RT_STS 0x10
#define PON_KPDPWR_N_SET BIT(0)
+#define PON_RESIN_N_SET BIT(1)
#define PON_PS_HOLD_RST_CTL 0x5a
#define PON_PS_HOLD_RST_CTL2 0x5b
@@ -46,6 +47,7 @@
struct pm8941_pwrkey {
struct device *dev;
int irq;
+ u32 resin_key_code;
u32 baseaddr;
struct regmap *regmap;
struct input_dev *input;
@@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
return IRQ_HANDLED;
}
+static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
+{
+ struct pm8941_pwrkey *pwrkey = _data;
+ unsigned int sts;
+ int error;
+
+ error = regmap_read(pwrkey->regmap,
+ pwrkey->baseaddr + PON_RT_STS, &sts);
+ if (error)
+ return IRQ_HANDLED;
+
+ input_report_key(pwrkey->input, pwrkey->resin_key_code,
+ !!(sts & PON_RESIN_N_SET));
+ input_sync(pwrkey->input);
+
+ return IRQ_HANDLED;
+}
+
static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
{
struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
@@ -153,12 +173,40 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
+static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey, int irq)
+{
+ int error;
+
+ /*
+ * Get the standard-key parameters. This might not be
+ * specified if there is no key mapping on the reset line.
+ */
+ error = of_property_read_u32(pwrkey->dev->of_node, "linux,code",
+ &pwrkey->resin_key_code);
+ if (error) {
+ dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
+ return error;
+ }
+
+ /* Register key configuration */
+ input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
+
+ error = devm_request_threaded_irq(pwrkey->dev, irq, NULL,
+ pm8941_resinkey_irq, IRQF_ONESHOT,
+ "pm8941_resinkey", pwrkey);
+ if (error)
+ dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
+ error);
+
+ return error;
+}
+
static int pm8941_pwrkey_probe(struct platform_device *pdev)
{
struct pm8941_pwrkey *pwrkey;
bool pull_up;
u32 req_delay;
- int error;
+ int error, resin_irq;
if (of_property_read_u32(pdev->dev.of_node, "debounce", &req_delay))
req_delay = 15625;
@@ -241,6 +289,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
return error;
}
+ resin_irq = platform_get_irq_byname(pdev, "resin");
+ if (resin_irq < 0 && resin_irq != -ENXIO) {
+ if (resin_irq != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get resin irq\n");
+ return resin_irq;
+ } else if (resin_irq >= 0) {
+ /* resin key capabilities are defined in device node */
+ error = pm8941_resin_key_init(pwrkey, resin_irq);
+ if (error) {
+ dev_err(&pdev->dev, "failed resin key initialization: %d\n",
+ error);
+ return error;
+ }
+ }
+
error = input_register_device(pwrkey->input);
if (error) {
dev_err(&pdev->dev, "failed to register input device: %d\n",
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities
From: Trilok Soni @ 2018-03-13 19:24 UTC (permalink / raw)
To: Tirupathi Reddy, dmitry.torokhov, robh+dt, mark.rutland
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <1520930158-5854-1-git-send-email-tirupath@codeaurora.org>
Hi Tirupathi,
On 3/13/2018 1:35 AM, Tirupathi Reddy wrote:
> Add resin key support to handle different types of key events
> defined in different platforms.
Describe "resin" in the commit text? or just call it same as power key
or different purpose?
>
> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> +{
> + struct pm8941_pwrkey *pwrkey = _data;
> + unsigned int sts;
> + int error;
> +
> + error = regmap_read(pwrkey->regmap,
> + pwrkey->baseaddr + PON_RT_STS, &sts);
> + if (error)
> + return IRQ_HANDLED;
You may want to document here why you still return as IRQ_HANDLED on the
error after reading the status register?
>
> + resin_irq = platform_get_irq_byname(pdev, "resin");
> + if (resin_irq < 0 && resin_irq != -ENXIO) {
> + if (resin_irq != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get resin irq\n");
> + return resin_irq;
> + } else if (resin_irq >= 0) {
> + /* resin key capabilities are defined in device node */
> + error = pm8941_resin_key_init(pwrkey, resin_irq);
> + if (error) {
> + dev_err(&pdev->dev, "failed resin key initialization: %d\n",
> + error);
> + return error;
> + }
> + }
> +
Any reason you don't want to create different driver for "resin" key? Is
this going to be physically different key on the platform? Please
describe why it needs to be part of the power key driver.
---Trilok Soni
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V1] Input: pm8941-pwrkey: add resin key capabilities
From: Trilok Soni @ 2018-03-13 19:25 UTC (permalink / raw)
To: Tirupathi Reddy T, Dmitry Torokhov
Cc: robh+dt, mark.rutland, linux-input, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <d0991385-d03b-a42e-9303-679f0c16de34@codeaurora.org>
Tirupathi,
On 3/13/2018 1:25 AM, Tirupathi Reddy T wrote:
> Hi Dmitry,
>
> On 3/10/2018 11:57 PM, Dmitry Torokhov wrote:
>> On Wed, Mar 07, 2018 at 12:21:47PM -0800, Dmitry Torokhov wrote:
>>> Hi Tirupathi,
>>>
>>> On Wed, Mar 07, 2018 at 01:39:33PM +0530, Tirupathi Reddy wrote:
>>>> Add resin key support to handle different types of key events
>>>> defined in different platforms.
>>
I replied to your patch without looking at response from Dmitry :) and
you will see some comments matching... you can check my comments too
though ...
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities
From: Bjorn Andersson @ 2018-03-14 2:40 UTC (permalink / raw)
To: Tirupathi Reddy
Cc: dmitry.torokhov, robh+dt, mark.rutland, linux-input,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <1520930158-5854-1-git-send-email-tirupath@codeaurora.org>
On Tue 13 Mar 01:35 PDT 2018, Tirupathi Reddy wrote:
> Add resin key support to handle different types of key events
> defined in different platforms.
>
> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> ---
> .../bindings/input/qcom,pm8941-pwrkey.txt | 20 ++++++-
> drivers/input/misc/pm8941-pwrkey.c | 65 +++++++++++++++++++++-
> 2 files changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> index 07bf55f..f499cf8 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> @@ -20,6 +20,14 @@ PROPERTIES
> defined by the binding document describing the node's
> interrupt parent.
>
> +- interrupt-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Interrupt names. This list must match up 1-to-1 with the
> + interrupts specified in the 'interrupts' property. "kpdpwr"
> + must be specified and should be the first entry of the list.
> + "resin" may be specified for some platforms.
> +
> - debounce:
> Usage: optional
> Value type: <u32>
> @@ -32,12 +40,22 @@ PROPERTIES
> Definition: presence of this property indicates that the KPDPWR_N pin
> should be configured for pull up.
>
> +- linux,code:
> + Usage: required for "resin" key
> + Value type: <u32>
> + Definition: The input key-code associated with the resin key.
> + Use the linux event codes defined in
> + include/dt-bindings/input/linux-event-codes.h
For completeness sake I think this should list both the key code for the
power key and for the RESIN key.
> +
> EXAMPLE
>
> pwrkey@800 {
> compatible = "qcom,pm8941-pwrkey";
> reg = <0x800>;
> - interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> + interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
> + <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> + interrupt-names = "kpdpwr", "resin";
> debounce = <15625>;
> bias-pull-up;
Do we need to support defining the bias for the resin as well?
Perhaps it makes more sense to describe the RESIN key with a separate
compatible and define that in a node of its own (we can still implement
this in the same driver - if there's any chance of reuse...).
> + linux,code = <KEY_VOLUMEDOWN>;
> };
> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
> index 18ad956..2fffc7c 100644
> --- a/drivers/input/misc/pm8941-pwrkey.c
> +++ b/drivers/input/misc/pm8941-pwrkey.c
> @@ -28,6 +28,7 @@
>
> #define PON_RT_STS 0x10
> #define PON_KPDPWR_N_SET BIT(0)
> +#define PON_RESIN_N_SET BIT(1)
>
> #define PON_PS_HOLD_RST_CTL 0x5a
> #define PON_PS_HOLD_RST_CTL2 0x5b
> @@ -46,6 +47,7 @@
> struct pm8941_pwrkey {
> struct device *dev;
> int irq;
> + u32 resin_key_code;
> u32 baseaddr;
> struct regmap *regmap;
> struct input_dev *input;
> @@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> +{
> + struct pm8941_pwrkey *pwrkey = _data;
> + unsigned int sts;
> + int error;
> +
> + error = regmap_read(pwrkey->regmap,
> + pwrkey->baseaddr + PON_RT_STS, &sts);
This needs to be broken because the line is 81 chars long, if you use a
shorter name for the return value (e.g. "ret") you don't need to do
this.
> + if (error)
> + return IRQ_HANDLED;
> +
> + input_report_key(pwrkey->input, pwrkey->resin_key_code,
> + !!(sts & PON_RESIN_N_SET));
Put pwrkey->resin_key_code in a local variable named "key" (or even
key_code) and you don't need to line wrap this one.
> + input_sync(pwrkey->input);
> +
> + return IRQ_HANDLED;
> +}
> +
Regards,
Bjorn
^ permalink raw reply
* [PATCH v4] HID: google: add google hammer HID driver
From: Nicolas Boichat @ 2018-03-14 5:22 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-kernel, linux-input, groeck, dtor
From: Wei-Ning Huang <wnhuang@chromium.org>
Add Google hammer HID driver. This driver allow us to control hammer
keyboard backlight and support future features.
We add a new HID quirk, that allows us to have the keyboard interface
to bind to google-hammer driver, while the touchpad interface can
bind to the multitouch driver.
Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
Changes since v3:
- Rebase on latest linux-next/master, which moved the quirk list from
hid-core.c to hid-quirks.c. Also, completely rework the logic to make
it possible to bind google-hammer driver to keyboard interface, and
generic multitouch driver to touchpad interface, as it is much simpler
to do now that quirks are read early in hid_add_device.
- Add dynamic backlight detection support (only such devices have an
output HID descriptor).
- Add support for wand (one more USB product ID).
- Add SPDX-License-Identifier, fix one minor formatting issue.
drivers/hid/Kconfig | 6 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 4 ++
drivers/hid/hid-google-hammer.c | 116 ++++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-quirks.c | 5 ++
include/linux/hid.h | 2 +
7 files changed, 137 insertions(+)
create mode 100644 drivers/hid/hid-google-hammer.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 1fce4c94e5ac..60252fd796f6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -339,6 +339,12 @@ config HOLTEK_FF
Say Y here if you have a Holtek On Line Grip based game controller
and want to have force feedback support for it.
+config HID_GOOGLE_HAMMER
+ tristate "Google Hammer Keyboard"
+ depends on USB_HID && LEDS_CLASS
+ ---help---
+ Say Y here if you have a Google Hammer device.
+
config HID_GT683R
tristate "MSI GT68xR LED support"
depends on LEDS_CLASS && USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 713601c7bfa6..17a8bd97da9d 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_HID_ELO) += hid-elo.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_GEMBIRD) += hid-gembird.o
obj-$(CONFIG_HID_GFRM) += hid-gfrm.o
+obj-$(CONFIG_HID_GOOGLE_HAMMER) += hid-google-hammer.o
obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
obj-$(CONFIG_HID_HOLTEK) += hid-holtek-kbd.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 36af26c2565b..61c7d135d680 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2116,6 +2116,10 @@ int hid_add_device(struct hid_device *hdev)
ret = hid_scan_report(hdev);
if (ret)
hid_warn(hdev, "bad device descriptor (%d)\n", ret);
+
+ if (hdev->quirks & HID_QUIRK_NO_GENERIC &&
+ hdev->group == HID_GROUP_GENERIC)
+ hdev->group = HID_GROUP_GENERIC_OVERRIDE;
}
/* XXX hack, any other cleaner solution after the driver core
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
new file mode 100644
index 000000000000..e7ad042a8464
--- /dev/null
+++ b/drivers/hid/hid-google-hammer.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Google Hammer device.
+ *
+ * Copyright (c) 2017 Google Inc.
+ * Author: Wei-Ning Huang <wnhuang@google.com>
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/hid.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+
+#define MAX_BRIGHTNESS 100
+
+struct hammer_kbd_leds {
+ struct led_classdev cdev;
+ struct hid_device *hdev;
+ u8 buf[2] ____cacheline_aligned;
+};
+
+static int hammer_kbd_brightness_set_blocking(struct led_classdev *cdev,
+ enum led_brightness br)
+{
+ struct hammer_kbd_leds *led = container_of(cdev,
+ struct hammer_kbd_leds,
+ cdev);
+ int ret;
+
+ led->buf[0] = 0;
+ led->buf[1] = br;
+
+ ret = hid_hw_output_report(led->hdev, led->buf, sizeof(led->buf));
+ if (ret == -ENOSYS)
+ ret = hid_hw_raw_request(led->hdev, 0, led->buf,
+ sizeof(led->buf),
+ HID_OUTPUT_REPORT,
+ HID_REQ_SET_REPORT);
+ if (ret < 0)
+ hid_err(led->hdev, "failed to set keyboard backlight: %d\n",
+ ret);
+ return ret;
+}
+
+static int hammer_register_leds(struct hid_device *hdev)
+{
+ struct hammer_kbd_leds *kbd_backlight;
+
+ kbd_backlight = devm_kzalloc(&hdev->dev,
+ sizeof(*kbd_backlight),
+ GFP_KERNEL);
+ if (!kbd_backlight)
+ return -ENOMEM;
+
+ kbd_backlight->hdev = hdev;
+ kbd_backlight->cdev.name = "hammer::kbd_backlight";
+ kbd_backlight->cdev.max_brightness = MAX_BRIGHTNESS;
+ kbd_backlight->cdev.brightness_set_blocking =
+ hammer_kbd_brightness_set_blocking;
+ kbd_backlight->cdev.flags = LED_HW_PLUGGABLE;
+
+ /* Set backlight to 0% initially. */
+ hammer_kbd_brightness_set_blocking(&kbd_backlight->cdev, 0);
+
+ return devm_led_classdev_register(&hdev->dev, &kbd_backlight->cdev);
+}
+
+static int hammer_input_configured(struct hid_device *hdev,
+ struct hid_input *hi)
+{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+
+ struct list_head *report_list =
+ &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
+
+ if (intf->cur_altsetting->desc.bInterfaceProtocol ==
+ USB_INTERFACE_PROTOCOL_KEYBOARD && !list_empty(report_list)) {
+ int err = hammer_register_leds(hdev);
+
+ if (err)
+ hid_warn(hdev,
+ "Failed to register keyboard backlight: %d\n",
+ err);
+ }
+
+ return 0;
+}
+
+static const struct hid_device_id hammer_devices[] = {
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC_OVERRIDE,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC_OVERRIDE,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STAFF) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC_OVERRIDE,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_WAND) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, hammer_devices);
+
+static struct hid_driver hammer_driver = {
+ .name = "hammer",
+ .id_table = hammer_devices,
+ .input_configured = hammer_input_configured,
+};
+module_hid_driver(hammer_driver);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 41f227a841fb..5a3a7ead3012 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -448,7 +448,10 @@
#define USB_DEVICE_ID_GOODTOUCH_000f 0x000f
#define USB_VENDOR_ID_GOOGLE 0x18d1
+#define USB_DEVICE_ID_GOOGLE_HAMMER 0x5022
#define USB_DEVICE_ID_GOOGLE_TOUCH_ROSE 0x5028
+#define USB_DEVICE_ID_GOOGLE_STAFF 0x502b
+#define USB_DEVICE_ID_GOOGLE_WAND 0x502d
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 587e2681a53f..d112a14da200 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -84,6 +84,11 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_FUTABA, USB_DEVICE_ID_LED_DISPLAY), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, USB_DEVICE_ID_GREENASIA_DUAL_USB_JOYPAD), HID_QUIRK_MULTI_INPUT },
+#if IS_ENABLED(CONFIG_HID_GOOGLE_HAMMER)
+ { HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER), HID_QUIRK_NO_GENERIC },
+ { HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STAFF), HID_QUIRK_NO_GENERIC },
+ { HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_WAND), HID_QUIRK_NO_GENERIC },
+#endif
{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_DRIVING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_FIGHTING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_HAPP, USB_DEVICE_ID_UGCI_FLYING), HID_QUIRK_BADPAD | HID_QUIRK_MULTI_INPUT },
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dfea5a656a1a..f2655265f8b5 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -340,6 +340,7 @@ struct hid_item {
#define HID_QUIRK_NO_EMPTY_INPUT 0x00000100
/* 0x00000200 reserved for backward compatibility, was NO_INIT_INPUT_REPORTS */
#define HID_QUIRK_ALWAYS_POLL 0x00000400
+#define HID_QUIRK_NO_GENERIC 0x00000800
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x00010000
#define HID_QUIRK_SKIP_OUTPUT_REPORT_ID 0x00020000
#define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP 0x00040000
@@ -359,6 +360,7 @@ struct hid_item {
#define HID_GROUP_MULTITOUCH 0x0002
#define HID_GROUP_SENSOR_HUB 0x0003
#define HID_GROUP_MULTITOUCH_WIN_8 0x0004
+#define HID_GROUP_GENERIC_OVERRIDE 0x0005
/*
* Vendor specific HID device groups
--
2.16.2.804.g6dcf76e118-goog
^ permalink raw reply related
* Re: [PATCH v5 01/36] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR
From: Archit Taneja @ 2018-03-14 5:29 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, Stéphane Marchesin,
linux-arm-kernel, mark.yao, wzz, hl, jingoohan1, sw0312.kim,
linux-kernel, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-2-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: Yakir Yang <ykk@rock-chips.com>
>
> Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
> function, or print the sink PSR error state if we failed to apply the
> requested PSR setting.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
> Cc: 征增 王 <wzz@rock-chips.com>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> [seanpaul changed timeout loop to a readx poll]
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 6 ++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 35 +++++++++++++++++++---
> 3 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index a693ab3078f0..e738aa6de1af 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -122,8 +122,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -149,8 +148,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> if (ret != 1)
> dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
>
> - analogix_dp_send_psr_spd(dp, &psr_vsc);
> - return 0;
> + return analogix_dp_send_psr_spd(dp, &psr_vsc);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 5c6a28806129..b039b28d8fcc 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,6 +20,8 @@
> #define MAX_CR_LOOP 5
> #define MAX_EQ_LOOP 5
>
> +#define DP_TIMEOUT_PSR_LOOP_MS 300
> +
> /* DP_MAX_LANE_COUNT */
> #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1)
> #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
> @@ -247,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
> void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc);
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> struct drm_dp_aux_msg *msg);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 303083ad28e3..005a3f7005d2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -10,10 +10,11 @@
> * option) any later version.
> */
>
> -#include <linux/device.h>
> -#include <linux/io.h>
> #include <linux/delay.h>
> +#include <linux/device.h>
> #include <linux/gpio.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
>
> #include <drm/bridge/analogix_dp.h>
>
> @@ -992,10 +993,25 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
> }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc)
> +static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp)
> +{
> + ssize_t val;
> + u8 status;
> +
> + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &status);
> + if (val < 0) {
> + dev_err(dp->dev, "PSR_STATUS read failed ret=%zd", val);
> + return val;
> + }
> + return status;
> +}
> +
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> + struct edp_vsc_psr *vsc)
> {
> unsigned int val;
> + int ret;
> + ssize_t psr_status;
>
> /* don't send info frame */
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1036,6 +1052,17 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> val |= IF_EN;
> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> + ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status,
> + psr_status >= 0 &&
> + ((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||
> + (!vsc->DB1 && psr_status == DP_PSR_SINK_INACTIVE)), 1500,
> + DP_TIMEOUT_PSR_LOOP_MS * 1000);
> + if (ret) {
> + dev_warn(dp->dev, "Failed to apply PSR %d\n", ret);
> + return ret;
> + }
> + return 0;
> }
>
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 03/36] drm/bridge: analogix_dp: Don't change psr while bridge is disabled
From: Archit Taneja @ 2018-03-14 5:30 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, Tomeu Vizoso, jingoohan1, sw0312.kim, dianders,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-4-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> There is a race between AUX CH bring-up and enabling bridge which will
> cause link training to fail. To avoid hitting it, don't change psr state
> while enabling the bridge.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> [seanpaul fixed up the commit message a bit and renamed *_supported to *_enabled]
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 15 ++++++++-------
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +-
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +-
> include/drm/bridge/analogix_dp.h | 2 +-
> 4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index e738aa6de1af..ee00d3d920e0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -98,18 +98,18 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> return 0;
> }
>
> -int analogix_dp_psr_supported(struct analogix_dp_device *dp)
> +int analogix_dp_psr_enabled(struct analogix_dp_device *dp)
> {
>
> - return dp->psr_support;
> + return dp->psr_enable;
> }
> -EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
> +EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled);
>
> int analogix_dp_enable_psr(struct analogix_dp_device *dp)
> {
> struct edp_vsc_psr psr_vsc;
>
> - if (!dp->psr_support)
> + if (!dp->psr_enable)
> return 0;
>
> /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
> @@ -131,7 +131,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> struct edp_vsc_psr psr_vsc;
> int ret;
>
> - if (!dp->psr_support)
> + if (!dp->psr_enable)
> return 0;
>
> /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
> @@ -871,8 +871,8 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> /* Enable video */
> analogix_dp_start_video(dp);
>
> - dp->psr_support = analogix_dp_detect_sink_psr(dp);
> - if (dp->psr_support)
> + dp->psr_enable = analogix_dp_detect_sink_psr(dp);
> + if (dp->psr_enable)
> analogix_dp_enable_sink_psr(dp);
> }
>
> @@ -1117,6 +1117,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> if (ret)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
>
> + dp->psr_enable = false;
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
> }
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b039b28d8fcc..e135a42cb19e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -170,7 +170,7 @@ struct analogix_dp_device {
> int dpms_mode;
> int hpd_gpio;
> bool force_hpd;
> - bool psr_support;
> + bool psr_enable;
>
> struct mutex panel_lock;
> bool panel_is_modeset;
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 36334839a3f8..3e8bf79bea58 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -82,7 +82,7 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
> struct rockchip_dp_device *dp = to_dp(encoder);
> int ret;
>
> - if (!analogix_dp_psr_supported(dp->adp))
> + if (!analogix_dp_psr_enabled(dp->adp))
> return;
>
> DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index 711fff9b6803..e9a1116d2f8e 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -41,7 +41,7 @@ struct analogix_dp_plat_data {
> struct drm_connector *);
> };
>
> -int analogix_dp_psr_supported(struct analogix_dp_device *dp);
> +int analogix_dp_psr_enabled(struct analogix_dp_device *dp);
> int analogix_dp_enable_psr(struct analogix_dp_device *dp);
> int analogix_dp_disable_psr(struct analogix_dp_device *dp);
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 05/36] drm/bridge: analogix_dp: add fast link train for eDP
From: Archit Taneja @ 2018-03-14 5:52 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, dianders,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-6-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> We would meet a short black screen when exit PSR with the full link
> training, In this case, we should use fast link train instead of full
> link training.
>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142 ++++++++++++++++-----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
> 2 files changed, 114 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index ee00d3d920e0..806c3878b3d6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -10,17 +10,18 @@
> * option) any later version.
> */
>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/err.h>
> #include <linux/clk.h>
> -#include <linux/io.h>
> +#include <linux/component.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> #include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> -#include <linux/gpio.h>
> -#include <linux/component.h>
> #include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
This re-ordering doesn't seem like it should be a part of this patch,
you can let it stay if it happens to cause conflicts with future
patches. Other than that:
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
>
> #include <drm/drmP.h>
> #include <drm/drm_atomic_helper.h>
> @@ -35,6 +36,8 @@
>
> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>
> +static const bool verify_fast_training;
> +
> struct bridge_init {
> struct i2c_client *client;
> struct device_node *node;
> @@ -528,7 +531,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> {
> int lane, lane_count, retval;
> u32 reg;
> - u8 link_align, link_status[2], adjust_request[2];
> + u8 link_align, link_status[2], adjust_request[2], spread;
>
> usleep_range(400, 401);
>
> @@ -571,6 +574,20 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> dev_dbg(dp->dev, "final lane count = %.2x\n",
> dp->link_train.lane_count);
>
> + retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD,
> + &spread);
> + if (retval != 1) {
> + dev_err(dp->dev, "failed to read downspread %d\n",
> + retval);
> + dp->fast_train_support = false;
> + } else {
> + dp->fast_train_support =
> + (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?
> + true : false;
> + }
> + dev_dbg(dp->dev, "fast link training %s\n",
> + dp->fast_train_support ? "supported" : "unsupported");
> +
> /* set enhanced mode if available */
> analogix_dp_set_enhanced_mode(dp);
> dp->link_train.lt_state = FINISHED;
> @@ -627,10 +644,12 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp,
> *lane_count = DPCD_MAX_LANE_COUNT(data);
> }
>
> -static void analogix_dp_init_training(struct analogix_dp_device *dp,
> - enum link_lane_count_type max_lane,
> - int max_rate)
> +static int analogix_dp_full_link_train(struct analogix_dp_device *dp,
> + u32 max_lanes, u32 max_rate)
> {
> + int retval = 0;
> + bool training_finished = false;
> +
> /*
> * MACRO_RST must be applied after the PLL_LOCK to avoid
> * the DP inter pair skew issue for at least 10 us
> @@ -656,18 +675,13 @@ static void analogix_dp_init_training(struct analogix_dp_device *dp,
> }
>
> /* Setup TX lane count & rate */
> - if (dp->link_train.lane_count > max_lane)
> - dp->link_train.lane_count = max_lane;
> + if (dp->link_train.lane_count > max_lanes)
> + dp->link_train.lane_count = max_lanes;
> if (dp->link_train.link_rate > max_rate)
> dp->link_train.link_rate = max_rate;
>
> /* All DP analog module power up */
> analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
> -}
> -
> -static int analogix_dp_sw_link_training(struct analogix_dp_device *dp)
> -{
> - int retval = 0, training_finished = 0;
>
> dp->link_train.lt_state = START;
>
> @@ -702,22 +716,88 @@ static int analogix_dp_sw_link_training(struct analogix_dp_device *dp)
> return retval;
> }
>
> -static int analogix_dp_set_link_train(struct analogix_dp_device *dp,
> - u32 count, u32 bwtype)
> +static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
> {
> - int i;
> - int retval;
> + int i, ret;
> + u8 link_align, link_status[2];
> + enum pll_status status;
>
> - for (i = 0; i < DP_TIMEOUT_LOOP_COUNT; i++) {
> - analogix_dp_init_training(dp, count, bwtype);
> - retval = analogix_dp_sw_link_training(dp);
> - if (retval == 0)
> - break;
> + analogix_dp_reset_macro(dp);
> +
> + analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> + analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
>
> - usleep_range(100, 110);
> + for (i = 0; i < dp->link_train.lane_count; i++) {
> + analogix_dp_set_lane_link_training(dp,
> + dp->link_train.training_lane[i], i);
> }
>
> - return retval;
> + ret = readx_poll_timeout(analogix_dp_get_pll_lock_status, dp, status,
> + status != PLL_UNLOCKED, 120,
> + 120 * DP_TIMEOUT_LOOP_COUNT);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);
> + return ret;
> + }
> +
> + /* source Set training pattern 1 */
> + analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
> + /* From DP spec, pattern must be on-screen for a minimum 500us */
> + usleep_range(500, 600);
> +
> + analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
> + /* From DP spec, pattern must be on-screen for a minimum 500us */
> + usleep_range(500, 600);
> +
> + /* TODO: enhanced_mode?*/
> + analogix_dp_set_training_pattern(dp, DP_NONE);
> +
> + /*
> + * Useful for debugging issues with fast link training, disable for more
> + * speed
> + */
> + if (verify_fast_training) {
> + ret = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,
> + &link_align);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dp->dev, "Read align status failed %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status,
> + 2);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dp->dev, "Read link status failed %d\n",
> + ret);
> + return ret;
> + }
> +
> + if (analogix_dp_clock_recovery_ok(link_status,
> + dp->link_train.lane_count)) {
> + DRM_DEV_ERROR(dp->dev, "Clock recovery failed\n");
> + analogix_dp_reduce_link_rate(dp);
> + return -EIO;
> + }
> +
> + if (analogix_dp_channel_eq_ok(link_status, link_align,
> + dp->link_train.lane_count)) {
> + DRM_DEV_ERROR(dp->dev, "Channel EQ failed\n");
> + analogix_dp_reduce_link_rate(dp);
> + return -EIO;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int analogix_dp_train_link(struct analogix_dp_device *dp)
> +{
> + if (dp->fast_train_support)
> + return analogix_dp_fast_link_train(dp);
> +
> + return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count,
> + dp->video_info.max_link_rate);
> }
>
> static int analogix_dp_config_video(struct analogix_dp_device *dp)
> @@ -846,10 +926,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> DRM_ERROR("failed to disable the panel\n");
> }
>
> - ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
> - dp->video_info.max_link_rate);
> + ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
> + DP_TIMEOUT_TRAINING_US * 5);
> if (ret) {
> - dev_err(dp->dev, "unable to do link train\n");
> + dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
> return;
> }
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index e135a42cb19e..920607d7eb3e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -20,6 +20,8 @@
> #define MAX_CR_LOOP 5
> #define MAX_EQ_LOOP 5
>
> +/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */
> +#define DP_TIMEOUT_TRAINING_US 22000
> #define DP_TIMEOUT_PSR_LOOP_MS 300
>
> /* DP_MAX_LANE_COUNT */
> @@ -171,6 +173,7 @@ struct analogix_dp_device {
> int hpd_gpio;
> bool force_hpd;
> bool psr_enable;
> + bool fast_train_support;
>
> struct mutex panel_lock;
> bool panel_is_modeset;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 06/36] drm/rockchip: Only wait for panel ACK on PSR entry
From: Archit Taneja @ 2018-03-14 5:54 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, Stéphane Marchesin,
Sonny Rao, linux-arm-kernel, mark.yao, wzz, hl, jingoohan1,
sw0312.kim, linux-kernel, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-7-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> We currently wait for the panel to mirror our intended PSR state
> before continuing on both PSR enter and PSR exit. This is really
> only important to do when we're entering PSR, since we want to
> be sure the last frame we pushed is being served from the panel's
> internal fb before shutting down the soc blocks (vop/analogix).
>
> This patch changes the behavior such that we only wait for the
> panel to complete the PSR transition when we're entering PSR, and
> to skip verification when we're exiting.
>
With the subject fix:
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +-
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 ++++-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 806c3878b3d6..5a2e35dc41e3 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -125,7 +125,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> - return analogix_dp_send_psr_spd(dp, &psr_vsc);
> + return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -151,7 +151,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> if (ret != 1)
> dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
>
> - return analogix_dp_send_psr_spd(dp, &psr_vsc);
> + return analogix_dp_send_psr_spd(dp, &psr_vsc, false);
> }
> EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 920607d7eb3e..6a96ef7e6934 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -253,7 +253,7 @@ void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc);
> + struct edp_vsc_psr *vsc, bool blocking);
> ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> struct drm_dp_aux_msg *msg);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 005a3f7005d2..9df2f3ef000c 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1007,7 +1007,7 @@ static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp)
> }
>
> int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc)
> + struct edp_vsc_psr *vsc, bool blocking)
> {
> unsigned int val;
> int ret;
> @@ -1053,6 +1053,9 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> val |= IF_EN;
> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>
> + if (!blocking)
> + return 0;
> +
> ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status,
> psr_status >= 0 &&
> ((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 07/36] drm/bridge: analogix_dp: Move enable video into config_video()
From: Archit Taneja @ 2018-03-14 5:59 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-8-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
>
> We need to enable video before analogix_dp_is_video_stream_on(), so
> we can get the right video stream status.
>
> Cc: 征增 王 <wzz@rock-chips.com>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5a2e35dc41e3..f9661b410cb9 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp)
> if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0)
> break;
> if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> - dev_err(dp->dev, "Timeout of video streamclk ok\n");
> + dev_err(dp->dev, "Timeout of slave video streamclk ok\n");
> return -ETIMEDOUT;
> }
> -
> - usleep_range(1, 2);
> + usleep_range(1000, 1001);
Could we briefly explain in the commit message why we need to increase
the delay in the timeout loop? Is it a consequence of calling
analogix_dp_start_video() earlier, or is this the preferred time
mentioned in the specs?
Thanks,
Archit
> }
>
> /* Set to use the register calculated M/N video */
> @@ -838,6 +837,9 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp)
> /* Configure video slave mode */
> analogix_dp_enable_video_master(dp, 0);
>
> + /* Enable video */
> + analogix_dp_start_video(dp);
> +
> timeout_loop = 0;
>
> for (;;) {
> @@ -948,9 +950,6 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> DRM_ERROR("failed to enable the panel\n");
> }
>
> - /* Enable video */
> - analogix_dp_start_video(dp);
> -
> dp->psr_enable = analogix_dp_detect_sink_psr(dp);
> if (dp->psr_enable)
> analogix_dp_enable_sink_psr(dp);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 08/36] drm/bridge: analogix_dp: Check AUX_EN status when doing AUX transfer
From: Archit Taneja @ 2018-03-14 6:01 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, dianders, a.hajda, ykk, kernel, m.szyprowski,
linux-samsung-soc, jy0922.shim, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
kyungmin.park, Laurent.pinchart, kuankuan.y, hshi,
Stéphane Marchesin
In-Reply-To: <20180309222327.18689-9-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
>
> We should check AUX_EN bit to confirm the AUX CH operation is completed.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 +++++++++++++----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 9df2f3ef000c..e78c861b9e06 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1073,9 +1073,9 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
> {
> u32 reg;
> u8 *buffer = msg->buffer;
> - int timeout_loop = 0;
> unsigned int i;
> int num_transferred = 0;
> + int ret;
>
> /* Buffer size of AUX CH is 16 bytes */
> if (WARN_ON(msg->size > 16))
> @@ -1139,17 +1139,20 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>
> writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
>
> - /* Is AUX CH command reply received? */
> + ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2,
> + reg, !(reg & AUX_EN), 25, 500 * 1000);
> + if (ret) {
> + dev_err(dp->dev, "AUX CH enable timeout!\n");
> + return -ETIMEDOUT;
> + }
> +
> /* TODO: Wait for an interrupt instead of looping? */
> - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> - while (!(reg & RPLY_RECEIV)) {
> - timeout_loop++;
> - if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
> - dev_err(dp->dev, "AUX CH command reply failed!\n");
> - return -ETIMEDOUT;
> - }
> - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
> - usleep_range(10, 11);
> + /* Is AUX CH command reply received? */
> + ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_INT_STA,
> + reg, reg & RPLY_RECEIV, 10, 20 * 1000);
> + if (ret) {
> + dev_err(dp->dev, "AUX CH cmd reply timeout!\n");
> + return -ETIMEDOUT;
> }
>
> /* Clear interrupt source for AUX CH command reply */
>
^ permalink raw reply
* Re: [PATCH v5 09/36] drm/bridge: analogix_dp: Don't use fast link training when panel just powered up
From: Archit Taneja @ 2018-03-14 6:03 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-10-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> Panel would reset its setting when it powers down. It would forget the last
> succeeded link training setting. So we can't use the last successful link
> training setting to do fast link training. Let's reset fast_train_enable in
> analogix_dp_bridge_disable();
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +++++----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +-
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index f9661b410cb9..ea7a80a989c6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -579,14 +579,14 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> if (retval != 1) {
> dev_err(dp->dev, "failed to read downspread %d\n",
> retval);
> - dp->fast_train_support = false;
> + dp->fast_train_enable = false;
> } else {
> - dp->fast_train_support =
> + dp->fast_train_enable =
> (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?
> true : false;
> }
> dev_dbg(dp->dev, "fast link training %s\n",
> - dp->fast_train_support ? "supported" : "unsupported");
> + dp->fast_train_enable ? "supported" : "unsupported");
>
> /* set enhanced mode if available */
> analogix_dp_set_enhanced_mode(dp);
> @@ -793,7 +793,7 @@ static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
>
> static int analogix_dp_train_link(struct analogix_dp_device *dp)
> {
> - if (dp->fast_train_support)
> + if (dp->fast_train_enable)
> return analogix_dp_fast_link_train(dp);
>
> return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count,
> @@ -1197,6 +1197,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
>
> dp->psr_enable = false;
> + dp->fast_train_enable = false;
> dp->dpms_mode = DRM_MODE_DPMS_OFF;
> }
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 6a96ef7e6934..403ff853464b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -173,7 +173,7 @@ struct analogix_dp_device {
> int hpd_gpio;
> bool force_hpd;
> bool psr_enable;
> - bool fast_train_support;
> + bool fast_train_enable;
>
> struct mutex panel_lock;
> bool panel_is_modeset;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 10/36] drm/bridge: analogix_dp: Retry bridge enable when it failed
From: Archit Taneja @ 2018-03-14 6:07 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-11-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> When we enable bridge failed, we have to retry it, otherwise we would get
> the abnormal display.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 65 +++++++++++++++++-----
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +-
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 +-
> 3 files changed, 56 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index ea7a80a989c6..c81733b8185e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -43,8 +43,10 @@ struct bridge_init {
> struct device_node *node;
> };
>
> -static void analogix_dp_init_dp(struct analogix_dp_device *dp)
> +static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> + int ret;
> +
> analogix_dp_reset(dp);
>
> analogix_dp_swreset(dp);
> @@ -56,10 +58,13 @@ static void analogix_dp_init_dp(struct analogix_dp_device *dp)
> analogix_dp_enable_sw_function(dp);
>
> analogix_dp_config_interrupt(dp);
> - analogix_dp_init_analog_func(dp);
> + ret = analogix_dp_init_analog_func(dp);
> + if (ret)
> + return ret;
>
> analogix_dp_init_hpd(dp);
> analogix_dp_init_aux(dp);
> + return 0;
> }
>
> static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> @@ -918,7 +923,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> -static void analogix_dp_commit(struct analogix_dp_device *dp)
> +static int analogix_dp_commit(struct analogix_dp_device *dp)
> {
> int ret;
>
> @@ -928,11 +933,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> DRM_ERROR("failed to disable the panel\n");
> }
>
> - ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
> - DP_TIMEOUT_TRAINING_US * 5);
> + ret = analogix_dp_train_link(dp);
> if (ret) {
> dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
> - return;
> + return ret;
> }
>
> analogix_dp_enable_scramble(dp, 1);
> @@ -953,6 +957,7 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
> dp->psr_enable = analogix_dp_detect_sink_psr(dp);
> if (dp->psr_enable)
> analogix_dp_enable_sink_psr(dp);
> + return 0;
> }
>
> /*
> @@ -1149,12 +1154,9 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge)
> DRM_ERROR("failed to setup the panel ret = %d\n", ret);
> }
>
> -static void analogix_dp_bridge_enable(struct drm_bridge *bridge)
> +static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> {
> - struct analogix_dp_device *dp = bridge->driver_private;
> -
> - if (dp->dpms_mode == DRM_MODE_DPMS_ON)
> - return;
> + int ret;
>
> pm_runtime_get_sync(dp->dev);
>
> @@ -1162,11 +1164,46 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge)
> dp->plat_data->power_on(dp->plat_data);
>
> phy_power_on(dp->phy);
> - analogix_dp_init_dp(dp);
> +
> + ret = analogix_dp_init_dp(dp);
> + if (ret)
> + goto out_dp_init;
> +
> + ret = analogix_dp_commit(dp);
> + if (ret)
> + goto out_dp_init;
> +
> enable_irq(dp->irq);
> - analogix_dp_commit(dp);
> + return 0;
>
> - dp->dpms_mode = DRM_MODE_DPMS_ON;
> +out_dp_init:
> + phy_power_off(dp->phy);
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> + pm_runtime_put_sync(dp->dev);
> +
> + return ret;
> +}
> +
> +static void analogix_dp_bridge_enable(struct drm_bridge *bridge)
> +{
> + struct analogix_dp_device *dp = bridge->driver_private;
> + int timeout_loop = 0;
> +
> + if (dp->dpms_mode == DRM_MODE_DPMS_ON)
> + return;
> +
> + while (timeout_loop < MAX_PLL_LOCK_LOOP) {
> + if (analogix_dp_set_bridge(dp) == 0) {
> + dp->dpms_mode = DRM_MODE_DPMS_ON;
> + return;
> + }
> + dev_err(dp->dev, "failed to set bridge, retry: %d\n",
> + timeout_loop);
> + timeout_loop++;
> + usleep_range(10, 11);
> + }
> + dev_err(dp->dev, "too many times retry set bridge, give it up\n");
> }
>
> static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 403ff853464b..769255dc6e99 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -19,6 +19,7 @@
> #define DP_TIMEOUT_LOOP_COUNT 100
> #define MAX_CR_LOOP 5
> #define MAX_EQ_LOOP 5
> +#define MAX_PLL_LOCK_LOOP 5
>
> /* Training takes 22ms if AUX channel comm fails. Use this as retry interval */
> #define DP_TIMEOUT_TRAINING_US 22000
> @@ -197,7 +198,7 @@ void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable);
> void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
> enum analog_power_block block,
> bool enable);
> -void analogix_dp_init_analog_func(struct analogix_dp_device *dp);
> +int analogix_dp_init_analog_func(struct analogix_dp_device *dp);
> void analogix_dp_init_hpd(struct analogix_dp_device *dp);
> void analogix_dp_force_hpd(struct analogix_dp_device *dp);
> enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp);
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index e78c861b9e06..b47c5af43560 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -333,7 +333,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
> }
> }
>
> -void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> +int analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> {
> u32 reg;
> int timeout_loop = 0;
> @@ -355,7 +355,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> timeout_loop++;
> if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> dev_err(dp->dev, "failed to get pll lock status\n");
> - return;
> + return -ETIMEDOUT;
> }
> usleep_range(10, 20);
> }
> @@ -366,6 +366,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp)
> reg &= ~(SERDES_FIFO_FUNC_EN_N | LS_CLK_DOMAIN_FUNC_EN_N
> | AUX_FUNC_EN_N);
> writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_2);
> + return 0;
> }
>
> void analogix_dp_clear_hotplug_interrupts(struct analogix_dp_device *dp)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 11/36] drm/bridge: analogix_dp: Wait for HPD signal before configuring link
From: Archit Taneja @ 2018-03-14 6:09 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
linux-input, orjan.eide, wxt, jeffy.chen, Stéphane Marchesin,
linux-arm-kernel, mark.yao, wzz, hl, jingoohan1, sw0312.kim,
linux-kernel, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-12-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> According to DP spec v1.3 chap 3.5.1.2 Link Training, Link Policy Maker
> must first detect that the HPD signal is asserted high by the Downstream
> Device before establishing a link with it.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index c81733b8185e..92fb9a072cb6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1169,6 +1169,17 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> if (ret)
> goto out_dp_init;
>
> + /*
> + * According to DP spec v1.3 chap 3.5.1.2 Link Training,
> + * We should first make sure the HPD signal is asserted high by device
> + * when we want to establish a link with it.
> + */
> + ret = analogix_dp_detect_hpd(dp);
> + if (ret) {
> + DRM_ERROR("failed to get hpd single ret = %d\n", ret);
> + goto out_dp_init;
> + }
> +
> ret = analogix_dp_commit(dp);
> if (ret)
> goto out_dp_init;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 12/36] drm/bridge: analogix_dp: Set PD_INC_BG first when powering up edp phy
From: Archit Taneja @ 2018-03-14 6:10 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-13-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> Following the correct power up sequence:
> dp_pd=ff => dp_pd=7f => wait 10us => dp_pd=00
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 10 ++++++++--
> drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 3 +++
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index b47c5af43560..bb72f8b0e603 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -321,10 +321,16 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
> break;
> case POWER_ALL:
> if (enable) {
> - reg = DP_PHY_PD | AUX_PD | CH3_PD | CH2_PD |
> - CH1_PD | CH0_PD;
> + reg = DP_ALL_PD;
> writel(reg, dp->reg_base + phy_pd_addr);
> } else {
> + reg = DP_ALL_PD;
> + writel(reg, dp->reg_base + phy_pd_addr);
> + usleep_range(10, 15);
> + reg &= ~DP_INC_BG;
> + writel(reg, dp->reg_base + phy_pd_addr);
> + usleep_range(10, 15);
> +
> writel(0x00, dp->reg_base + phy_pd_addr);
> }
> break;
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> index 40200c652533..9602668669f4 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> @@ -342,12 +342,15 @@
> #define DP_PLL_REF_BIT_1_2500V (0x7 << 0)
>
> /* ANALOGIX_DP_PHY_PD */
> +#define DP_INC_BG (0x1 << 7)
> +#define DP_EXP_BG (0x1 << 6)
> #define DP_PHY_PD (0x1 << 5)
> #define AUX_PD (0x1 << 4)
> #define CH3_PD (0x1 << 3)
> #define CH2_PD (0x1 << 2)
> #define CH1_PD (0x1 << 1)
> #define CH0_PD (0x1 << 0)
> +#define DP_ALL_PD (0xff)
>
> /* ANALOGIX_DP_PHY_TEST */
> #define MACRO_RST (0x1 << 5)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 13/36] drm/bridge: analogix_dp: Ensure edp is disabled when shutting down the panel
From: Archit Taneja @ 2018-03-14 6:14 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-14-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
>
> When panel is shut down, we should make sure edp can be disabled to avoid
> undefined behavior.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 92fb9a072cb6..3a222e7e46ee 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1160,6 +1160,12 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
>
> pm_runtime_get_sync(dp->dev);
>
> + ret = clk_prepare_enable(dp->clock);
> + if (ret < 0) {
> + DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
> + goto out_dp_clk_pre;
> + }
> +
> if (dp->plat_data->power_on)
> dp->plat_data->power_on(dp->plat_data);
>
> @@ -1191,6 +1197,8 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
> phy_power_off(dp->phy);
> if (dp->plat_data->power_off)
> dp->plat_data->power_off(dp->plat_data);
> + clk_disable_unprepare(dp->clock);
> +out_dp_clk_pre:
> pm_runtime_put_sync(dp->dev);
>
> return ret;
> @@ -1233,11 +1241,14 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
> }
>
> disable_irq(dp->irq);
> + analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
> phy_power_off(dp->phy);
>
> if (dp->plat_data->power_off)
> dp->plat_data->power_off(dp->plat_data);
>
> + clk_disable_unprepare(dp->clock);
> +
> pm_runtime_put_sync(dp->dev);
>
> ret = analogix_dp_prepare_panel(dp, false, true);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 14/36] drm/bridge: analogix_dp: Extend hpd check time to 100ms
From: Archit Taneja @ 2018-03-14 6:15 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen,
Stéphane Marchesin, linux-arm-kernel, mark.yao, wzz, hl,
jingoohan1, sw0312.kim, dianders, kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-15-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
>
> There was a 1ms delay to detect the hpd signal, which is too short to
> detect a short pulse. This patch extends this delay to 100ms.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: 征增 王 <wzz@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 3a222e7e46ee..6cbde8473f58 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -76,7 +76,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
> return 0;
>
> timeout_loop++;
> - usleep_range(10, 11);
> + usleep_range(1000, 1100);
> }
>
> /*
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v5 15/36] drm/bridge: analogix_dp: Fix incorrect usage of enhanced mode
From: Archit Taneja @ 2018-03-14 6:17 UTC (permalink / raw)
To: Enric Balletbo i Serra, inki.dae, thierry.reding, hjc, seanpaul,
airlied, tfiga, heiko
Cc: dri-devel, linux-kernel, Laurent.pinchart, ykk, kernel,
m.szyprowski, linux-samsung-soc, rydberg, krzk, linux-rockchip,
kgene, linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
mark.yao, wzz, hl, jingoohan1, sw0312.kim, dianders,
kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180309222327.18689-16-enric.balletbo@collabora.com>
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
> From: zain wang <wzz@rock-chips.com>
>
> Enhanced mode is required by the eDP 1.2 specification, and not doing it
> early could result in a period of time where we have a link transmitting
> idle packets without it. Since there is no reason to disable it, we just
> enable it at the beginning of link training and then keep it on all the
> time.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
Thanks,
Archit
>
> Cc: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: zain wang <wzz@rock-chips.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6cbde8473f58..1eed35f9eb8d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -281,6 +281,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
> retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2);
> if (retval < 0)
> return retval;
> + /* set enhanced mode if available */
> + analogix_dp_set_enhanced_mode(dp);
>
> /* Set TX pre-emphasis to minimum */
> for (lane = 0; lane < lane_count; lane++)
> @@ -593,8 +595,6 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
> dev_dbg(dp->dev, "fast link training %s\n",
> dp->fast_train_enable ? "supported" : "unsupported");
>
> - /* set enhanced mode if available */
> - analogix_dp_set_enhanced_mode(dp);
> dp->link_train.lt_state = FINISHED;
>
> return 0;
> @@ -940,8 +940,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp)
> }
>
> analogix_dp_enable_scramble(dp, 1);
> - analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
> - analogix_dp_enable_enhanced_mode(dp, 1);
>
> analogix_dp_init_video(dp);
> ret = analogix_dp_config_video(dp);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ 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