Skip to content

Commit

Permalink
drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge
Browse files Browse the repository at this point in the history 
[ Upstream commit 9a0cdcd ]

adv7533 bridge tries to dynamically switch lanes based on the
mode by detaching and attaching the mipi dsi device.

This approach is incorrect because this method of dynamic switch of
detaching and attaching the mipi dsi device also results in removing
and adding the component which is not necessary.

This approach is also prone to deadlocks. So for example, on the
db410c whenever this path is executed with lockdep enabled,
this results in a deadlock due to below ordering of locks.

-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
        lock_acquire+0x6c/0x90
        drm_modeset_acquire_init+0xf4/0x150
        drmm_mode_config_init+0x220/0x770
        msm_drm_bind+0x13c/0x654
        try_to_bring_up_aggregate_device+0x164/0x1d0
        __component_add+0xa8/0x174
        component_add+0x18/0x2c
        dsi_dev_attach+0x24/0x30
        dsi_host_attach+0x98/0x14c
        devm_mipi_dsi_attach+0x38/0xb0
        adv7533_attach_dsi+0x8c/0x110
        adv7511_probe+0x5a0/0x930
        i2c_device_probe+0x30c/0x350
        really_probe.part.0+0x9c/0x2b0
        __driver_probe_device+0x98/0x144
        driver_probe_device+0xac/0x14c
        __device_attach_driver+0xbc/0x124
        bus_for_each_drv+0x78/0xd0
        __device_attach+0xa8/0x1c0
        device_initial_probe+0x18/0x24
        bus_probe_device+0xa0/0xac
        deferred_probe_work_func+0x90/0xd0
        process_one_work+0x28c/0x6b0
        worker_thread+0x240/0x444
        kthread+0x110/0x114
        ret_from_fork+0x10/0x20

-> #0 (component_mutex){+.+.}-{3:3}:
        __lock_acquire+0x1280/0x20ac
        lock_acquire.part.0+0xe0/0x230
        lock_acquire+0x6c/0x90
        __mutex_lock+0x84/0x400
        mutex_lock_nested+0x3c/0x70
        component_del+0x34/0x170
        dsi_dev_detach+0x24/0x30
        dsi_host_detach+0x20/0x64
        mipi_dsi_detach+0x2c/0x40
        adv7533_mode_set+0x64/0x90
        adv7511_bridge_mode_set+0x210/0x214
        drm_bridge_chain_mode_set+0x5c/0x84
        crtc_set_mode+0x18c/0x1dc
        drm_atomic_helper_commit_modeset_disables+0x40/0x50
        msm_atomic_commit_tail+0x1d0/0x6e0
        commit_tail+0xa4/0x180
        drm_atomic_helper_commit+0x178/0x3b0
        drm_atomic_commit+0xa4/0xe0
        drm_client_modeset_commit_atomic+0x228/0x284
        drm_client_modeset_commit_locked+0x64/0x1d0
        drm_client_modeset_commit+0x34/0x60
        drm_fb_helper_lastclose+0x74/0xcc
        drm_lastclose+0x3c/0x80
        drm_release+0xfc/0x114
        __fput+0x70/0x224
        ____fput+0x14/0x20
        task_work_run+0x88/0x1a0
        do_exit+0x350/0xa50
        do_group_exit+0x38/0xa4
        __wake_up_parent+0x0/0x34
        invoke_syscall+0x48/0x114
        el0_svc_common.constprop.0+0x60/0x11c
        do_el0_svc+0x30/0xc0
        el0_svc+0x58/0x100
        el0t_64_sync_handler+0x1b0/0x1bc
        el0t_64_sync+0x18c/0x190

Due to above reasons, remove the dynamic lane switching
code from adv7533 bridge chip and filter out the modes
which would need different number of lanes as compared
to the initialization time using the mode_valid callback.

This can be potentially re-introduced by using the pre_enable()
callback but this needs to be evaluated first whether such an
approach will work so this will be done with a separate change.

changes since RFC:
	- Fix commit text and add TODO comment

changes in v2:
	- Fix checkpatch formatting errors

Fixes: 62b2f02 ("drm/bridge: adv7533: Change number of DSI lanes dynamically")
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/16
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Robert Foss <robert.foss@linaro.org>
Link: https://lore.kernel.org/r/1661797363-7564-1-git-send-email-quic_abhinavk@quicinc.com
Signed-off-by: Robert Foss <robert.foss@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/1665522649-3423-1-git-send-email-quic_abhinavk@quicinc.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@gregkh
Abhinav Kumar authored and gregkh committed Dec 31, 2022
1 parent 6d40a49 commit b5ec2a0
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
3 changes: 2 additions & 1 deletion drivers/gpu/drm/bridge/adv7511/adv7511.h
View file
Expand Up @@ -395,7 +395,8 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)

void adv7533_dsi_power_on(struct adv7511 *adv);
void adv7533_dsi_power_off(struct adv7511 *adv);
void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode);
enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
const struct drm_display_mode *mode);
int adv7533_patch_registers(struct adv7511 *adv);
int adv7533_patch_cec_registers(struct adv7511 *adv);
int adv7533_attach_dsi(struct adv7511 *adv);
Expand Down
18 changes: 14 additions & 4 deletions drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
View file
Expand Up @@ -697,7 +697,7 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
}

static enum drm_mode_status adv7511_mode_valid(struct adv7511 *adv7511,
struct drm_display_mode *mode)
const struct drm_display_mode *mode)
{
if (mode->clock > 165000)
return MODE_CLOCK_HIGH;
Expand Down Expand Up @@ -791,9 +791,6 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
regmap_update_bits(adv7511->regmap, 0x17,
0x60, (vsync_polarity << 6) | (hsync_polarity << 5));

if (adv7511->type == ADV7533 || adv7511->type == ADV7535)
adv7533_mode_set(adv7511, adj_mode);

drm_mode_copy(&adv7511->curr_mode, adj_mode);

/*
Expand Down Expand Up @@ -913,6 +910,18 @@ static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
adv7511_mode_set(adv, mode, adj_mode);
}

static enum drm_mode_status adv7511_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *info,
const struct drm_display_mode *mode)
{
struct adv7511 *adv = bridge_to_adv7511(bridge);

if (adv->type == ADV7533 || adv->type == ADV7535)
return adv7533_mode_valid(adv, mode);
else
return adv7511_mode_valid(adv, mode);
}

static int adv7511_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
{
Expand Down Expand Up @@ -963,6 +972,7 @@ static const struct drm_bridge_funcs adv7511_bridge_funcs = {
.enable = adv7511_bridge_enable,
.disable = adv7511_bridge_disable,
.mode_set = adv7511_bridge_mode_set,
.mode_valid = adv7511_bridge_mode_valid,
.attach = adv7511_bridge_attach,
.detect = adv7511_bridge_detect,
.get_edid = adv7511_bridge_get_edid,
Expand Down
25 changes: 13 additions & 12 deletions drivers/gpu/drm/bridge/adv7511/adv7533.c
View file
Expand Up @@ -100,26 +100,27 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
regmap_write(adv->regmap_cec, 0x27, 0x0b);
}

void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode)
enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
const struct drm_display_mode *mode)
{
int lanes;
struct mipi_dsi_device *dsi = adv->dsi;
int lanes, ret;

if (adv->num_dsi_lanes != 4)
return;

if (mode->clock > 80000)
lanes = 4;
else
lanes = 3;

if (lanes != dsi->lanes) {
mipi_dsi_detach(dsi);
dsi->lanes = lanes;
ret = mipi_dsi_attach(dsi);
if (ret)
dev_err(&dsi->dev, "failed to change host lanes\n");
}
/*
* TODO: add support for dynamic switching of lanes
* by using the bridge pre_enable() op . Till then filter
* out the modes which shall need different number of lanes
* than what was configured in the device tree.
*/
if (lanes != dsi->lanes)
return MODE_BAD;

return MODE_OK;
}

int adv7533_patch_registers(struct adv7511 *adv)
Expand Down

0 comments on commit b5ec2a0

Please sign in to comment.