linux/drivers/gpu/drm/msm/dp/dp_aux.c

730 lines
18 KiB
C
Raw Permalink Normal View History

// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
*/
#include <linux/delay.h>
#include <linux/iopoll.h>
#include <linux/phy/phy.h>
#include <drm/drm_print.h>
#include "dp_reg.h"
#include "dp_aux.h"
enum msm_dp_aux_err {
DP_AUX_ERR_NONE,
DP_AUX_ERR_ADDR,
DP_AUX_ERR_TOUT,
DP_AUX_ERR_NACK,
DP_AUX_ERR_DEFER,
DP_AUX_ERR_NACK_DEFER,
DP_AUX_ERR_PHY,
};
struct msm_dp_aux_private {
struct device *dev;
void __iomem *aux_base;
struct phy *phy;
struct mutex mutex;
struct completion comp;
enum msm_dp_aux_err aux_error_num;
u32 retry_cnt;
bool cmd_busy;
bool native;
bool read;
bool no_send_addr;
bool no_send_stop;
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
bool initted;
bool is_edp;
drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected As documented in the description of the transfer() function of "struct drm_dp_aux", the transfer() function can be called at any time regardless of the state of the DP port. Specifically if the kernel has the DP AUX character device enabled and userspace accesses "/dev/drm_dp_auxN" directly then the AUX transfer function will be called regardless of whether a DP device is connected. For eDP panels we have a special rule where we wait (with a 5 second timeout) for HPD to go high. This rule was important before all panels drivers were converted to call wait_hpd_asserted() and actually can be removed in a future commit. For external DP devices we never checked for HPD. That means that trying to access the DP AUX character device (AKA `hexdump -C /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my system: $ time hexdump -C /dev/drm_dp_aux0 hexdump: /dev/drm_dp_aux0: Connection timed out real 0m8.200s We want access to the drm_dp_auxN character device to fail faster than 8 seconds when no DP cable is plugged in. Let's add a test to make transfers fail right away if a device isn't plugged in. Rather than testing the HPD line directly, we have the dp_display module tell us when AUX transfers should be enabled so we can handle cases where HPD is signaled out of band like with Type C. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/583127/ Link: https://lore.kernel.org/r/20240315143621.v2.1.I16aff881c9fe82b5e0fc06ca312da017aa7b5b3e@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2024-03-15 14:36:29 -07:00
bool enable_xfers;
u32 offset;
u32 segment;
struct drm_dp_aux msm_dp_aux;
};
static inline u32 msm_dp_read_aux(struct msm_dp_aux_private *aux, u32 offset)
{
return readl_relaxed(aux->aux_base + offset);
}
static inline void msm_dp_write_aux(struct msm_dp_aux_private *aux,
u32 offset, u32 data)
{
/*
* To make sure aux reg writes happens before any other operation,
* this function uses writel() instread of writel_relaxed()
*/
writel(data, aux->aux_base + offset);
}
static void msm_dp_aux_clear_hw_interrupts(struct msm_dp_aux_private *aux)
{
msm_dp_read_aux(aux, REG_DP_PHY_AUX_INTERRUPT_STATUS);
msm_dp_write_aux(aux, REG_DP_PHY_AUX_INTERRUPT_CLEAR, 0x1f);
msm_dp_write_aux(aux, REG_DP_PHY_AUX_INTERRUPT_CLEAR, 0x9f);
msm_dp_write_aux(aux, REG_DP_PHY_AUX_INTERRUPT_CLEAR, 0);
}
/*
* NOTE: resetting AUX controller will also clear any pending HPD related interrupts
*/
static void msm_dp_aux_reset(struct msm_dp_aux_private *aux)
{
u32 aux_ctrl;
aux_ctrl = msm_dp_read_aux(aux, REG_DP_AUX_CTRL);
aux_ctrl |= DP_AUX_CTRL_RESET;
msm_dp_write_aux(aux, REG_DP_AUX_CTRL, aux_ctrl);
usleep_range(1000, 1100); /* h/w recommended delay */
aux_ctrl &= ~DP_AUX_CTRL_RESET;
msm_dp_write_aux(aux, REG_DP_AUX_CTRL, aux_ctrl);
}
static void msm_dp_aux_enable(struct msm_dp_aux_private *aux)
{
u32 aux_ctrl;
aux_ctrl = msm_dp_read_aux(aux, REG_DP_AUX_CTRL);
msm_dp_write_aux(aux, REG_DP_TIMEOUT_COUNT, 0xffff);
msm_dp_write_aux(aux, REG_DP_AUX_LIMITS, 0xffff);
aux_ctrl |= DP_AUX_CTRL_ENABLE;
msm_dp_write_aux(aux, REG_DP_AUX_CTRL, aux_ctrl);
}
static void msm_dp_aux_disable(struct msm_dp_aux_private *aux)
{
u32 aux_ctrl;
aux_ctrl = msm_dp_read_aux(aux, REG_DP_AUX_CTRL);
aux_ctrl &= ~DP_AUX_CTRL_ENABLE;
msm_dp_write_aux(aux, REG_DP_AUX_CTRL, aux_ctrl);
}
static int msm_dp_aux_wait_for_hpd_connect_state(struct msm_dp_aux_private *aux,
unsigned long wait_us)
{
u32 state;
/* poll for hpd connected status every 2ms and timeout after wait_us */
return readl_poll_timeout(aux->aux_base +
REG_DP_DP_HPD_INT_STATUS,
state, state & DP_DP_HPD_STATE_STATUS_CONNECTED,
min(wait_us, 2000), wait_us);
}
#define MAX_AUX_RETRIES 5
static ssize_t msm_dp_aux_write(struct msm_dp_aux_private *aux,
struct drm_dp_aux_msg *msg)
{
u8 data[4];
u32 reg;
ssize_t len;
u8 *msgdata = msg->buffer;
int const AUX_CMD_FIFO_LEN = 128;
int i = 0;
if (aux->read)
len = 0;
else
len = msg->size;
/*
* cmd fifo only has depth of 144 bytes
* limit buf length to 128 bytes here
*/
if (len > AUX_CMD_FIFO_LEN - 4) {
DRM_ERROR("buf size greater than allowed size of 128 bytes\n");
return -EINVAL;
}
/* Pack cmd and write to HW */
data[0] = (msg->address >> 16) & 0xf; /* addr[19:16] */
if (aux->read)
data[0] |= BIT(4); /* R/W */
data[1] = msg->address >> 8; /* addr[15:8] */
data[2] = msg->address; /* addr[7:0] */
data[3] = msg->size - 1; /* len[7:0] */
for (i = 0; i < len + 4; i++) {
reg = (i < 4) ? data[i] : msgdata[i - 4];
reg <<= DP_AUX_DATA_OFFSET;
reg &= DP_AUX_DATA_MASK;
reg |= DP_AUX_DATA_WRITE;
/* index = 0, write */
if (i == 0)
reg |= DP_AUX_DATA_INDEX_WRITE;
msm_dp_write_aux(aux, REG_DP_AUX_DATA, reg);
}
msm_dp_write_aux(aux, REG_DP_AUX_TRANS_CTRL, 0);
msm_dp_aux_clear_hw_interrupts(aux);
reg = 0; /* Transaction number == 1 */
if (!aux->native) { /* i2c */
reg |= DP_AUX_TRANS_CTRL_I2C;
if (aux->no_send_addr)
reg |= DP_AUX_TRANS_CTRL_NO_SEND_ADDR;
if (aux->no_send_stop)
reg |= DP_AUX_TRANS_CTRL_NO_SEND_STOP;
}
reg |= DP_AUX_TRANS_CTRL_GO;
msm_dp_write_aux(aux, REG_DP_AUX_TRANS_CTRL, reg);
return len;
}
static ssize_t msm_dp_aux_cmd_fifo_tx(struct msm_dp_aux_private *aux,
struct drm_dp_aux_msg *msg)
{
ssize_t ret;
unsigned long time_left;
reinit_completion(&aux->comp);
ret = msm_dp_aux_write(aux, msg);
if (ret < 0)
return ret;
time_left = wait_for_completion_timeout(&aux->comp,
msecs_to_jiffies(250));
if (!time_left)
return -ETIMEDOUT;
return ret;
}
static ssize_t msm_dp_aux_cmd_fifo_rx(struct msm_dp_aux_private *aux,
struct drm_dp_aux_msg *msg)
{
u32 data;
u8 *dp;
u32 i, actual_i;
u32 len = msg->size;
data = msm_dp_read_aux(aux, REG_DP_AUX_TRANS_CTRL);
data &= ~DP_AUX_TRANS_CTRL_GO;
msm_dp_write_aux(aux, REG_DP_AUX_TRANS_CTRL, data);
data = DP_AUX_DATA_INDEX_WRITE; /* INDEX_WRITE */
data |= DP_AUX_DATA_READ; /* read */
msm_dp_write_aux(aux, REG_DP_AUX_DATA, data);
dp = msg->buffer;
/* discard first byte */
data = msm_dp_read_aux(aux, REG_DP_AUX_DATA);
for (i = 0; i < len; i++) {
data = msm_dp_read_aux(aux, REG_DP_AUX_DATA);
*dp++ = (u8)((data >> DP_AUX_DATA_OFFSET) & 0xff);
actual_i = (data >> DP_AUX_DATA_INDEX_OFFSET) & 0xFF;
if (i != actual_i)
break;
}
return i;
}
static void msm_dp_aux_update_offset_and_segment(struct msm_dp_aux_private *aux,
struct drm_dp_aux_msg *input_msg)
{
u32 edid_address = 0x50;
u32 segment_address = 0x30;
bool i2c_read = input_msg->request &
(DP_AUX_I2C_READ & DP_AUX_NATIVE_READ);
u8 *data;
if (aux->native || i2c_read || ((input_msg->address != edid_address) &&
(input_msg->address != segment_address)))
return;
data = input_msg->buffer;
if (input_msg->address == segment_address)
aux->segment = *data;
else
aux->offset = *data;
}
/**
* msm_dp_aux_transfer_helper() - helper function for EDID read transactions
*
* @aux: DP AUX private structure
* @input_msg: input message from DRM upstream APIs
* @send_seg: send the segment to sink
*
* return: void
*
* This helper function is used to fix EDID reads for non-compliant
* sinks that do not handle the i2c middle-of-transaction flag correctly.
*/
static void msm_dp_aux_transfer_helper(struct msm_dp_aux_private *aux,
struct drm_dp_aux_msg *input_msg,
bool send_seg)
{
struct drm_dp_aux_msg helper_msg;
u32 message_size = 0x10;
u32 segment_address = 0x30;
u32 const edid_block_length = 0x80;
bool i2c_mot = input_msg->request & DP_AUX_I2C_MOT;
bool i2c_read = input_msg->request &
(DP_AUX_I2C_READ & DP_AUX_NATIVE_READ);
if (!i2c_mot || !i2c_read || (input_msg->size == 0))
return;
/*
* Sending the segment value and EDID offset will be performed
* from the DRM upstream EDID driver for each block. Avoid
* duplicate AUX transactions related to this while reading the
* first 16 bytes of each block.
*/
if (!(aux->offset % edid_block_length) || !send_seg)
goto end;
aux->read = false;
aux->cmd_busy = true;
aux->no_send_addr = true;
aux->no_send_stop = true;
/*
* Send the segment address for every i2c read in which the
* middle-of-tranaction flag is set. This is required to support EDID
* reads of more than 2 blocks as the segment address is reset to 0
* since we are overriding the middle-of-transaction flag for read
* transactions.
*/
if (aux->segment) {
memset(&helper_msg, 0, sizeof(helper_msg));
helper_msg.address = segment_address;
helper_msg.buffer = &aux->segment;
helper_msg.size = 1;
msm_dp_aux_cmd_fifo_tx(aux, &helper_msg);
}
/*
* Send the offset address for every i2c read in which the
* middle-of-transaction flag is set. This will ensure that the sink
* will update its read pointer and return the correct portion of the
* EDID buffer in the subsequent i2c read trasntion triggered in the
* native AUX transfer function.
*/
memset(&helper_msg, 0, sizeof(helper_msg));
helper_msg.address = input_msg->address;
helper_msg.buffer = &aux->offset;
helper_msg.size = 1;
msm_dp_aux_cmd_fifo_tx(aux, &helper_msg);
end:
aux->offset += message_size;
if (aux->offset == 0x80 || aux->offset == 0x100)
aux->segment = 0x0; /* reset segment at end of block */
}
/*
* This function does the real job to process an AUX transaction.
* It will call aux_reset() function to reset the AUX channel,
* if the waiting is timeout.
*/
static ssize_t msm_dp_aux_transfer(struct drm_dp_aux *msm_dp_aux,
struct drm_dp_aux_msg *msg)
{
ssize_t ret;
int const aux_cmd_native_max = 16;
int const aux_cmd_i2c_max = 128;
struct msm_dp_aux_private *aux;
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
aux->native = msg->request & (DP_AUX_NATIVE_WRITE & DP_AUX_NATIVE_READ);
/* Ignore address only message */
if (msg->size == 0 || !msg->buffer) {
msg->reply = aux->native ?
DP_AUX_NATIVE_REPLY_ACK : DP_AUX_I2C_REPLY_ACK;
return msg->size;
}
/* msg sanity check */
if ((aux->native && msg->size > aux_cmd_native_max) ||
msg->size > aux_cmd_i2c_max) {
DRM_ERROR("%s: invalid msg: size(%zu), request(%x)\n",
__func__, msg->size, msg->request);
return -EINVAL;
}
ret = pm_runtime_resume_and_get(msm_dp_aux->dev);
drm/msm/dp: incorporate pm_runtime framework into DP driver Currently DP driver is executed independent of PM runtime framework. This leads msm eDP panel can not being detected by edp_panel driver during generic_edp_panel_probe() due to AUX DPCD read failed at edp panel driver. Incorporate PM runtime framework into DP driver so that host controller's power and clocks are enable/disable through PM runtime mechanism. Once PM runtime framework is incorporated into DP driver, waking up device from power up path is not necessary. Hence remove it. After incorporating pm_runtime framework into eDP/DP driver, dp_pm_suspend() to handle power off both DP phy and controller during suspend and dp_pm_resume() to handle power on both DP phy and controller during resume are not necessary. Therefore both dp_pm_suspend() and dp_pm_resume() are dropped and replace with dp_pm_runtime_suspend() and dp_pm_runtime_resume() respectively. Changes in v9: -- silent compiler warning message at dp_power_init() and dp_power_deinit() with W1 flag Changes in v7: -- add comments to dp_pm_runtime_resume() -- add comments to dp_bridge_hpd_enable() -- delete dp->hpd_state = ST_DISCONNECTED from dp_bridge_hpd_notify() Changes in v6: -- delete dp_power_client_deinit(dp->power); -- remove if (!dp->dp_display.is_edp) condition checkout at plug_handle() -- remove if (!dp->dp_display.is_edp) condition checkout at unplug_handle() -- add IRQF_NO_AUTOEN to devm_request_irq() -- add enable_irq() and disable_irq() to pm_runtime_resume()/suspend() -- del dp->hpd_state = ST_DISCONNECTED from dp_bridge_hpd_disable() Changes in v5: -- remove pm_runtime_put_autosuspend feature, use pm_runtime_put_sync() -- squash add pm_runtime_force_suspend()/resume() patch into this patch Changes in v4: -- reworded commit text to explain why pm_framework is required for edp panel -- reworded commit text to explain autosuspend is choiced -- delete EV_POWER_PM_GET and PM_EV_POWER_PUT from changes #3 -- delete dp_display_pm_get() and dp_display_pm_Put() from changes #3 -- return value from pm_runtime_resume_and_get() directly -- check return value of devm_pm_runtime_enable() -- delete pm_runtime_xxx from dp_display_remove() -- drop dp_display_host_init() from EV_HPD_INIT_SETUP -- drop both dp_pm_prepare() and dp_pm_compete() from this change -- delete ST_SUSPENDED state -- rewording commit text to add more details regrading the purpose of this change Changes in v3: -- incorporate removing pm_runtime_xx() from dp_pwer.c to this patch -- use pm_runtime_resume_and_get() instead of pm_runtime_get() -- error checking pm_runtime_resume_and_get() return value -- add EV_POWER_PM_GET and PM_EV_POWER_PUT to handle HPD_GPIO case -- replace dp_pm_suspend() with pm_runtime_force_suspend() -- replace dp_pm_resume() with pm_runtime_force_resume() Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Patchwork: https://patchwork.freedesktop.org/patch/570073/ Link: https://lore.kernel.org/r/1701472789-25951-6-git-send-email-quic_khsieh@quicinc.com Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-12-01 15:19:47 -08:00
if (ret)
return ret;
mutex_lock(&aux->mutex);
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
if (!aux->initted) {
ret = -EIO;
goto exit;
}
drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected As documented in the description of the transfer() function of "struct drm_dp_aux", the transfer() function can be called at any time regardless of the state of the DP port. Specifically if the kernel has the DP AUX character device enabled and userspace accesses "/dev/drm_dp_auxN" directly then the AUX transfer function will be called regardless of whether a DP device is connected. For eDP panels we have a special rule where we wait (with a 5 second timeout) for HPD to go high. This rule was important before all panels drivers were converted to call wait_hpd_asserted() and actually can be removed in a future commit. For external DP devices we never checked for HPD. That means that trying to access the DP AUX character device (AKA `hexdump -C /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my system: $ time hexdump -C /dev/drm_dp_aux0 hexdump: /dev/drm_dp_aux0: Connection timed out real 0m8.200s We want access to the drm_dp_auxN character device to fail faster than 8 seconds when no DP cable is plugged in. Let's add a test to make transfers fail right away if a device isn't plugged in. Rather than testing the HPD line directly, we have the dp_display module tell us when AUX transfers should be enabled so we can handle cases where HPD is signaled out of band like with Type C. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/583127/ Link: https://lore.kernel.org/r/20240315143621.v2.1.I16aff881c9fe82b5e0fc06ca312da017aa7b5b3e@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2024-03-15 14:36:29 -07:00
/*
* If we're using DP and an external display isn't connected then the
* transfer won't succeed. Return right away. If we don't do this we
* can end up with long timeouts if someone tries to access the DP AUX
* character device when no DP device is connected.
*/
if (!aux->is_edp && !aux->enable_xfers) {
ret = -ENXIO;
goto exit;
}
msm_dp_aux_update_offset_and_segment(aux, msg);
msm_dp_aux_transfer_helper(aux, msg, true);
aux->read = msg->request & (DP_AUX_I2C_READ & DP_AUX_NATIVE_READ);
aux->cmd_busy = true;
if (aux->read) {
aux->no_send_addr = true;
aux->no_send_stop = false;
} else {
aux->no_send_addr = true;
aux->no_send_stop = true;
}
ret = msm_dp_aux_cmd_fifo_tx(aux, msg);
if (ret < 0) {
if (aux->native) {
aux->retry_cnt++;
if (!(aux->retry_cnt % MAX_AUX_RETRIES))
phy_calibrate(aux->phy);
}
/* reset aux if link is in connected state */
if (msm_dp_aux_is_link_connected(msm_dp_aux))
msm_dp_aux_reset(aux);
} else {
aux->retry_cnt = 0;
switch (aux->aux_error_num) {
case DP_AUX_ERR_NONE:
if (aux->read)
ret = msm_dp_aux_cmd_fifo_rx(aux, msg);
msg->reply = aux->native ? DP_AUX_NATIVE_REPLY_ACK : DP_AUX_I2C_REPLY_ACK;
break;
case DP_AUX_ERR_DEFER:
msg->reply = aux->native ? DP_AUX_NATIVE_REPLY_DEFER : DP_AUX_I2C_REPLY_DEFER;
break;
case DP_AUX_ERR_PHY:
case DP_AUX_ERR_ADDR:
case DP_AUX_ERR_NACK:
case DP_AUX_ERR_NACK_DEFER:
msg->reply = aux->native ? DP_AUX_NATIVE_REPLY_NACK : DP_AUX_I2C_REPLY_NACK;
break;
case DP_AUX_ERR_TOUT:
ret = -ETIMEDOUT;
break;
}
}
aux->cmd_busy = false;
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
exit:
mutex_unlock(&aux->mutex);
pm_runtime_put_sync(msm_dp_aux->dev);
return ret;
}
irqreturn_t msm_dp_aux_isr(struct drm_dp_aux *msm_dp_aux, u32 isr)
{
struct msm_dp_aux_private *aux;
if (!msm_dp_aux) {
DRM_ERROR("invalid input\n");
drm/msm/dp: Return IRQ_NONE for unhandled interrupts If our interrupt handler gets called and we don't really handle the interrupt then we should return IRQ_NONE. The current interrupt handler didn't do this, so let's fix it. NOTE: for some of the cases it's clear that we should return IRQ_NONE and some cases it's clear that we should return IRQ_HANDLED. However, there are a few that fall somewhere in between. Specifically, the documentation for when to return IRQ_NONE vs. IRQ_HANDLED is probably best spelled out in the commit message of commit d9e4ad5badf4 ("Document that IRQ_NONE should be returned when IRQ not actually handled"). That commit makes it clear that we should return IRQ_HANDLED if we've done something to make the interrupt stop happening. The case where it's unclear is, for instance, in dp_aux_isr() after we've read the interrupt using dp_catalog_aux_get_irq() and confirmed that "isr" is non-zero. The function dp_catalog_aux_get_irq() not only reads the interrupts but it also "ack"s all the interrupts that are returned. For an "unknown" interrupt this has a very good chance of actually stopping the interrupt from happening. That would mean we've identified that it's our device and done something to stop them from happening and should return IRQ_HANDLED. Specifically, it should be noted that most interrupts that need "ack"ing are ones that are one-time events and doing an "ack" is enough to clear them. However, since these interrupts are unknown then, by definition, it's unknown if "ack"ing them is truly enough to clear them. It's possible that we also need to remove the original source of the interrupt. In this case, IRQ_NONE would be a better choice. Given that returning an occasional IRQ_NONE isn't the absolute end of the world, however, let's choose that course of action. The IRQ framework will forgive a few IRQ_NONE returns now and again (and it won't even log them, which is why we have to log them ourselves). This means that if we _do_ end hitting an interrupt where "ack"ing isn't enough the kernel will eventually detect the problem and shut our device down. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520660/ Link: https://lore.kernel.org/r/20230126170745.v2.2.I2d7aec2fadb9c237cd0090a47d6a8ba2054bf0f8@changeid [DB: reformatted commit message to make checkpatch happy] Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:13 -08:00
return IRQ_NONE;
}
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
drm/msm/dp: Clean up handling of DP AUX interrupts The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520658/ Link: https://lore.kernel.org/r/20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:12 -08:00
if (!aux->cmd_busy) {
DRM_ERROR("Unexpected DP AUX IRQ %#010x when not busy\n", isr);
drm/msm/dp: Return IRQ_NONE for unhandled interrupts If our interrupt handler gets called and we don't really handle the interrupt then we should return IRQ_NONE. The current interrupt handler didn't do this, so let's fix it. NOTE: for some of the cases it's clear that we should return IRQ_NONE and some cases it's clear that we should return IRQ_HANDLED. However, there are a few that fall somewhere in between. Specifically, the documentation for when to return IRQ_NONE vs. IRQ_HANDLED is probably best spelled out in the commit message of commit d9e4ad5badf4 ("Document that IRQ_NONE should be returned when IRQ not actually handled"). That commit makes it clear that we should return IRQ_HANDLED if we've done something to make the interrupt stop happening. The case where it's unclear is, for instance, in dp_aux_isr() after we've read the interrupt using dp_catalog_aux_get_irq() and confirmed that "isr" is non-zero. The function dp_catalog_aux_get_irq() not only reads the interrupts but it also "ack"s all the interrupts that are returned. For an "unknown" interrupt this has a very good chance of actually stopping the interrupt from happening. That would mean we've identified that it's our device and done something to stop them from happening and should return IRQ_HANDLED. Specifically, it should be noted that most interrupts that need "ack"ing are ones that are one-time events and doing an "ack" is enough to clear them. However, since these interrupts are unknown then, by definition, it's unknown if "ack"ing them is truly enough to clear them. It's possible that we also need to remove the original source of the interrupt. In this case, IRQ_NONE would be a better choice. Given that returning an occasional IRQ_NONE isn't the absolute end of the world, however, let's choose that course of action. The IRQ framework will forgive a few IRQ_NONE returns now and again (and it won't even log them, which is why we have to log them ourselves). This means that if we _do_ end hitting an interrupt where "ack"ing isn't enough the kernel will eventually detect the problem and shut our device down. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520660/ Link: https://lore.kernel.org/r/20230126170745.v2.2.I2d7aec2fadb9c237cd0090a47d6a8ba2054bf0f8@changeid [DB: reformatted commit message to make checkpatch happy] Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:13 -08:00
return IRQ_NONE;
drm/msm/dp: Clean up handling of DP AUX interrupts The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520658/ Link: https://lore.kernel.org/r/20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:12 -08:00
}
drm/msm/dp: Clean up handling of DP AUX interrupts The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520658/ Link: https://lore.kernel.org/r/20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:12 -08:00
/*
* The logic below assumes only one error bit is set (other than "done"
* which can apparently be set at the same time as some of the other
* bits). Warn if more than one get set so we know we need to improve
* the logic.
*/
if (hweight32(isr & ~DP_INTR_AUX_XFER_DONE) > 1)
DRM_WARN("Some DP AUX interrupts unhandled: %#010x\n", isr);
if (isr & DP_INTR_AUX_ERROR) {
aux->aux_error_num = DP_AUX_ERR_PHY;
msm_dp_aux_clear_hw_interrupts(aux);
drm/msm/dp: Clean up handling of DP AUX interrupts The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520658/ Link: https://lore.kernel.org/r/20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:12 -08:00
} else if (isr & DP_INTR_NACK_DEFER) {
aux->aux_error_num = DP_AUX_ERR_NACK_DEFER;
} else if (isr & DP_INTR_WRONG_ADDR) {
aux->aux_error_num = DP_AUX_ERR_ADDR;
} else if (isr & DP_INTR_TIMEOUT) {
aux->aux_error_num = DP_AUX_ERR_TOUT;
} else if (!aux->native && (isr & DP_INTR_I2C_NACK)) {
aux->aux_error_num = DP_AUX_ERR_NACK;
} else if (!aux->native && (isr & DP_INTR_I2C_DEFER)) {
if (isr & DP_INTR_AUX_XFER_DONE)
aux->aux_error_num = DP_AUX_ERR_NACK;
else
aux->aux_error_num = DP_AUX_ERR_DEFER;
} else if (isr & DP_INTR_AUX_XFER_DONE) {
aux->aux_error_num = DP_AUX_ERR_NONE;
} else {
DRM_WARN("Unexpected interrupt: %#010x\n", isr);
drm/msm/dp: Return IRQ_NONE for unhandled interrupts If our interrupt handler gets called and we don't really handle the interrupt then we should return IRQ_NONE. The current interrupt handler didn't do this, so let's fix it. NOTE: for some of the cases it's clear that we should return IRQ_NONE and some cases it's clear that we should return IRQ_HANDLED. However, there are a few that fall somewhere in between. Specifically, the documentation for when to return IRQ_NONE vs. IRQ_HANDLED is probably best spelled out in the commit message of commit d9e4ad5badf4 ("Document that IRQ_NONE should be returned when IRQ not actually handled"). That commit makes it clear that we should return IRQ_HANDLED if we've done something to make the interrupt stop happening. The case where it's unclear is, for instance, in dp_aux_isr() after we've read the interrupt using dp_catalog_aux_get_irq() and confirmed that "isr" is non-zero. The function dp_catalog_aux_get_irq() not only reads the interrupts but it also "ack"s all the interrupts that are returned. For an "unknown" interrupt this has a very good chance of actually stopping the interrupt from happening. That would mean we've identified that it's our device and done something to stop them from happening and should return IRQ_HANDLED. Specifically, it should be noted that most interrupts that need "ack"ing are ones that are one-time events and doing an "ack" is enough to clear them. However, since these interrupts are unknown then, by definition, it's unknown if "ack"ing them is truly enough to clear them. It's possible that we also need to remove the original source of the interrupt. In this case, IRQ_NONE would be a better choice. Given that returning an occasional IRQ_NONE isn't the absolute end of the world, however, let's choose that course of action. The IRQ framework will forgive a few IRQ_NONE returns now and again (and it won't even log them, which is why we have to log them ourselves). This means that if we _do_ end hitting an interrupt where "ack"ing isn't enough the kernel will eventually detect the problem and shut our device down. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520660/ Link: https://lore.kernel.org/r/20230126170745.v2.2.I2d7aec2fadb9c237cd0090a47d6a8ba2054bf0f8@changeid [DB: reformatted commit message to make checkpatch happy] Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:13 -08:00
return IRQ_NONE;
drm/msm/dp: Clean up handling of DP AUX interrupts The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520658/ Link: https://lore.kernel.org/r/20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:12 -08:00
}
complete(&aux->comp);
drm/msm/dp: Return IRQ_NONE for unhandled interrupts If our interrupt handler gets called and we don't really handle the interrupt then we should return IRQ_NONE. The current interrupt handler didn't do this, so let's fix it. NOTE: for some of the cases it's clear that we should return IRQ_NONE and some cases it's clear that we should return IRQ_HANDLED. However, there are a few that fall somewhere in between. Specifically, the documentation for when to return IRQ_NONE vs. IRQ_HANDLED is probably best spelled out in the commit message of commit d9e4ad5badf4 ("Document that IRQ_NONE should be returned when IRQ not actually handled"). That commit makes it clear that we should return IRQ_HANDLED if we've done something to make the interrupt stop happening. The case where it's unclear is, for instance, in dp_aux_isr() after we've read the interrupt using dp_catalog_aux_get_irq() and confirmed that "isr" is non-zero. The function dp_catalog_aux_get_irq() not only reads the interrupts but it also "ack"s all the interrupts that are returned. For an "unknown" interrupt this has a very good chance of actually stopping the interrupt from happening. That would mean we've identified that it's our device and done something to stop them from happening and should return IRQ_HANDLED. Specifically, it should be noted that most interrupts that need "ack"ing are ones that are one-time events and doing an "ack" is enough to clear them. However, since these interrupts are unknown then, by definition, it's unknown if "ack"ing them is truly enough to clear them. It's possible that we also need to remove the original source of the interrupt. In this case, IRQ_NONE would be a better choice. Given that returning an occasional IRQ_NONE isn't the absolute end of the world, however, let's choose that course of action. The IRQ framework will forgive a few IRQ_NONE returns now and again (and it won't even log them, which is why we have to log them ourselves). This means that if we _do_ end hitting an interrupt where "ack"ing isn't enough the kernel will eventually detect the problem and shut our device down. Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/520660/ Link: https://lore.kernel.org/r/20230126170745.v2.2.I2d7aec2fadb9c237cd0090a47d6a8ba2054bf0f8@changeid [DB: reformatted commit message to make checkpatch happy] Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2023-01-26 17:09:13 -08:00
return IRQ_HANDLED;
}
void msm_dp_aux_enable_xfers(struct drm_dp_aux *msm_dp_aux, bool enabled)
drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected As documented in the description of the transfer() function of "struct drm_dp_aux", the transfer() function can be called at any time regardless of the state of the DP port. Specifically if the kernel has the DP AUX character device enabled and userspace accesses "/dev/drm_dp_auxN" directly then the AUX transfer function will be called regardless of whether a DP device is connected. For eDP panels we have a special rule where we wait (with a 5 second timeout) for HPD to go high. This rule was important before all panels drivers were converted to call wait_hpd_asserted() and actually can be removed in a future commit. For external DP devices we never checked for HPD. That means that trying to access the DP AUX character device (AKA `hexdump -C /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my system: $ time hexdump -C /dev/drm_dp_aux0 hexdump: /dev/drm_dp_aux0: Connection timed out real 0m8.200s We want access to the drm_dp_auxN character device to fail faster than 8 seconds when no DP cable is plugged in. Let's add a test to make transfers fail right away if a device isn't plugged in. Rather than testing the HPD line directly, we have the dp_display module tell us when AUX transfers should be enabled so we can handle cases where HPD is signaled out of band like with Type C. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/583127/ Link: https://lore.kernel.org/r/20240315143621.v2.1.I16aff881c9fe82b5e0fc06ca312da017aa7b5b3e@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2024-03-15 14:36:29 -07:00
{
struct msm_dp_aux_private *aux;
drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected As documented in the description of the transfer() function of "struct drm_dp_aux", the transfer() function can be called at any time regardless of the state of the DP port. Specifically if the kernel has the DP AUX character device enabled and userspace accesses "/dev/drm_dp_auxN" directly then the AUX transfer function will be called regardless of whether a DP device is connected. For eDP panels we have a special rule where we wait (with a 5 second timeout) for HPD to go high. This rule was important before all panels drivers were converted to call wait_hpd_asserted() and actually can be removed in a future commit. For external DP devices we never checked for HPD. That means that trying to access the DP AUX character device (AKA `hexdump -C /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my system: $ time hexdump -C /dev/drm_dp_aux0 hexdump: /dev/drm_dp_aux0: Connection timed out real 0m8.200s We want access to the drm_dp_auxN character device to fail faster than 8 seconds when no DP cable is plugged in. Let's add a test to make transfers fail right away if a device isn't plugged in. Rather than testing the HPD line directly, we have the dp_display module tell us when AUX transfers should be enabled so we can handle cases where HPD is signaled out of band like with Type C. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/583127/ Link: https://lore.kernel.org/r/20240315143621.v2.1.I16aff881c9fe82b5e0fc06ca312da017aa7b5b3e@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2024-03-15 14:36:29 -07:00
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected As documented in the description of the transfer() function of "struct drm_dp_aux", the transfer() function can be called at any time regardless of the state of the DP port. Specifically if the kernel has the DP AUX character device enabled and userspace accesses "/dev/drm_dp_auxN" directly then the AUX transfer function will be called regardless of whether a DP device is connected. For eDP panels we have a special rule where we wait (with a 5 second timeout) for HPD to go high. This rule was important before all panels drivers were converted to call wait_hpd_asserted() and actually can be removed in a future commit. For external DP devices we never checked for HPD. That means that trying to access the DP AUX character device (AKA `hexdump -C /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my system: $ time hexdump -C /dev/drm_dp_aux0 hexdump: /dev/drm_dp_aux0: Connection timed out real 0m8.200s We want access to the drm_dp_auxN character device to fail faster than 8 seconds when no DP cable is plugged in. Let's add a test to make transfers fail right away if a device isn't plugged in. Rather than testing the HPD line directly, we have the dp_display module tell us when AUX transfers should be enabled so we can handle cases where HPD is signaled out of band like with Type C. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Patchwork: https://patchwork.freedesktop.org/patch/583127/ Link: https://lore.kernel.org/r/20240315143621.v2.1.I16aff881c9fe82b5e0fc06ca312da017aa7b5b3e@changeid Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
2024-03-15 14:36:29 -07:00
aux->enable_xfers = enabled;
}
void msm_dp_aux_reconfig(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux;
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
phy_calibrate(aux->phy);
msm_dp_aux_reset(aux);
}
void msm_dp_aux_init(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux;
if (!msm_dp_aux) {
DRM_ERROR("invalid input\n");
return;
}
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
mutex_lock(&aux->mutex);
msm_dp_aux_enable(aux);
aux->retry_cnt = 0;
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
aux->initted = true;
mutex_unlock(&aux->mutex);
}
void msm_dp_aux_deinit(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux;
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
mutex_lock(&aux->mutex);
aux->initted = false;
msm_dp_aux_disable(aux);
drm/msm/dp: Avoid unpowered AUX xfers that caused crashes If you happened to try to access `/dev/drm_dp_aux` devices provided by the MSM DP AUX driver too early at bootup you could go boom. Let's avoid that by only allowing AUX transfers when the controller is powered up. Specifically the crash that was seen (on Chrome OS 5.4 tree with relevant backports): Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 0 PID: 3131 Comm: fwupd Not tainted 5.4.144-16620-g28af11b73efb #1 Hardware name: Google Lazor (rev3+) with KB Backlight (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xac/0x124 panic+0x150/0x390 nmi_panic+0x80/0x94 arm64_serror_panic+0x78/0x84 do_serror+0x0/0x118 do_serror+0xa4/0x118 el1_error+0xbc/0x160 dp_catalog_aux_write_data+0x1c/0x3c dp_aux_cmd_fifo_tx+0xf0/0x1b0 dp_aux_transfer+0x1b0/0x2bc drm_dp_dpcd_access+0x8c/0x11c drm_dp_dpcd_read+0x64/0x10c auxdev_read_iter+0xd4/0x1c4 I did a little bit of tracing and found that: * We register the AUX device very early at bootup. * Power isn't actually turned on for my system until hpd_event_thread() -> dp_display_host_init() -> dp_power_init() * You can see that dp_power_init() calls dp_aux_init() which is where we start allowing AUX channel requests to go through. In general this patch is a bit of a bandaid but at least it gets us out of the current state where userspace acting at the wrong time can fully crash the system. * I think the more proper fix (which requires quite a bit more changes) is to power stuff on while an AUX transfer is happening. This is like the solution we did for ti-sn65dsi86. This might be required for us to move to populating the panel via the DP-AUX bus. * Another fix considered was to dynamically register / unregister. I tried that at <https://crrev.com/c/3169431/3> but it got ugly. Currently there's a bug where the pm_runtime() state isn't tracked properly and that causes us to just keep registering more and more. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Link: https://lore.kernel.org/r/20211109100403.1.I4e23470d681f7efe37e2e7f1a6466e15e9bb1d72@changeid Signed-off-by: Rob Clark <robdclark@chromium.org>
2021-11-09 10:04:18 -08:00
mutex_unlock(&aux->mutex);
}
int msm_dp_aux_register(struct drm_dp_aux *msm_dp_aux)
{
int ret;
if (!msm_dp_aux) {
DRM_ERROR("invalid input\n");
return -EINVAL;
}
ret = drm_dp_aux_register(msm_dp_aux);
if (ret) {
DRM_ERROR("%s: failed to register drm aux: %d\n", __func__,
ret);
return ret;
}
return 0;
}
void msm_dp_aux_unregister(struct drm_dp_aux *msm_dp_aux)
{
drm_dp_aux_unregister(msm_dp_aux);
}
static int msm_dp_wait_hpd_asserted(struct drm_dp_aux *msm_dp_aux,
unsigned long wait_us)
{
int ret;
struct msm_dp_aux_private *aux;
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
ret = pm_runtime_resume_and_get(aux->dev);
if (ret)
return ret;
ret = msm_dp_aux_wait_for_hpd_connect_state(aux, wait_us);
pm_runtime_put_sync(aux->dev);
return ret;
}
void msm_dp_aux_hpd_enable(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
u32 reg;
/* Configure REFTIMER and enable it */
reg = msm_dp_read_aux(aux, REG_DP_DP_HPD_REFTIMER);
reg |= DP_DP_HPD_REFTIMER_ENABLE;
msm_dp_write_aux(aux, REG_DP_DP_HPD_REFTIMER, reg);
/* Enable HPD */
msm_dp_write_aux(aux, REG_DP_DP_HPD_CTRL, DP_DP_HPD_CTRL_HPD_EN);
}
void msm_dp_aux_hpd_disable(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
u32 reg;
reg = msm_dp_read_aux(aux, REG_DP_DP_HPD_REFTIMER);
reg &= ~DP_DP_HPD_REFTIMER_ENABLE;
msm_dp_write_aux(aux, REG_DP_DP_HPD_REFTIMER, reg);
msm_dp_write_aux(aux, REG_DP_DP_HPD_CTRL, 0);
}
void msm_dp_aux_hpd_intr_enable(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
u32 reg;
reg = msm_dp_read_aux(aux, REG_DP_DP_HPD_INT_MASK);
reg |= DP_DP_HPD_INT_MASK;
msm_dp_write_aux(aux, REG_DP_DP_HPD_INT_MASK,
reg & DP_DP_HPD_INT_MASK);
}
void msm_dp_aux_hpd_intr_disable(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
u32 reg;
reg = msm_dp_read_aux(aux, REG_DP_DP_HPD_INT_MASK);
reg &= ~DP_DP_HPD_INT_MASK;
msm_dp_write_aux(aux, REG_DP_DP_HPD_INT_MASK,
reg & DP_DP_HPD_INT_MASK);
}
u32 msm_dp_aux_get_hpd_intr_status(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
int isr, mask;
isr = msm_dp_read_aux(aux, REG_DP_DP_HPD_INT_STATUS);
msm_dp_write_aux(aux, REG_DP_DP_HPD_INT_ACK,
(isr & DP_DP_HPD_INT_MASK));
mask = msm_dp_read_aux(aux, REG_DP_DP_HPD_INT_MASK);
/*
* We only want to return interrupts that are unmasked to the caller.
* However, the interrupt status field also contains other
* informational bits about the HPD state status, so we only mask
* out the part of the register that tells us about which interrupts
* are pending.
*/
return isr & (mask | ~DP_DP_HPD_INT_MASK);
}
u32 msm_dp_aux_is_link_connected(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux =
container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
u32 status;
status = msm_dp_read_aux(aux, REG_DP_DP_HPD_INT_STATUS);
status >>= DP_DP_HPD_STATE_STATUS_BITS_SHIFT;
status &= DP_DP_HPD_STATE_STATUS_BITS_MASK;
return status;
}
struct drm_dp_aux *msm_dp_aux_get(struct device *dev,
struct phy *phy,
bool is_edp,
void __iomem *aux_base)
{
struct msm_dp_aux_private *aux;
aux = devm_kzalloc(dev, sizeof(*aux), GFP_KERNEL);
if (!aux)
return ERR_PTR(-ENOMEM);
init_completion(&aux->comp);
aux->cmd_busy = false;
aux->is_edp = is_edp;
mutex_init(&aux->mutex);
aux->dev = dev;
aux->phy = phy;
aux->retry_cnt = 0;
aux->aux_base = aux_base;
/*
* Use the drm_dp_aux_init() to use the aux adapter
* before registering AUX with the DRM device so that
* msm eDP panel can be detected by generic_dep_panel_probe().
*/
aux->msm_dp_aux.name = "dpu_dp_aux";
aux->msm_dp_aux.dev = dev;
aux->msm_dp_aux.transfer = msm_dp_aux_transfer;
aux->msm_dp_aux.wait_hpd_asserted = msm_dp_wait_hpd_asserted;
drm_dp_aux_init(&aux->msm_dp_aux);
return &aux->msm_dp_aux;
}
void msm_dp_aux_put(struct drm_dp_aux *msm_dp_aux)
{
struct msm_dp_aux_private *aux;
if (!msm_dp_aux)
return;
aux = container_of(msm_dp_aux, struct msm_dp_aux_private, msm_dp_aux);
mutex_destroy(&aux->mutex);
devm_kfree(aux->dev, aux);
}