On Thu, Nov 28, 2019 at 04:37:40PM +0100, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Ensure that a runtime PM reference is acquired each time the DPAUX registers are accessed. Otherwise the code may end up running without the controller being powered, out-of-reset or clocked in some corner cases, resulting in a crash.
Signed-off-by: Thierry Reding treding@nvidia.com
On patches 4,5,7 in this series Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
On this one here I'm very confused.
- Why do you drop the runtime pm between enable and disable? Is that just how the hw works, i.e. the pad config stays, just the registers go away?
- I'm not seeing any locking between the different users (dp aux and pinctrl). We might want to change drm_dp_aux->hw_mutex to a pointer to make this easier (but I'm not super fond of that pattern from i2c).
- Your drm_dp_aux_enable/disable needs to be moved into the ->transfer callback, otherwise the various userspace interface (dp aux, but also i2c on top of that) won't work. Some pre/post_transfer functions like i2c has might be useful for stuff like this.
Cheers, Daniel
drivers/gpu/drm/tegra/dpaux.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index 622cdf1ad246..4b2b86aed1a5 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -434,8 +434,13 @@ static int tegra_dpaux_set_mux(struct pinctrl_dev *pinctrl, unsigned int function, unsigned int group) { struct tegra_dpaux *dpaux = pinctrl_dev_get_drvdata(pinctrl);
- int err;
- pm_runtime_get_sync(dpaux->dev);
- err = tegra_dpaux_pad_config(dpaux, function);
- pm_runtime_put(dpaux->dev);
- return tegra_dpaux_pad_config(dpaux, function);
- return err;
}
static const struct pinmux_ops tegra_dpaux_pinmux_ops = { @@ -809,15 +814,22 @@ enum drm_connector_status drm_dp_aux_detect(struct drm_dp_aux *aux) int drm_dp_aux_enable(struct drm_dp_aux *aux) { struct tegra_dpaux *dpaux = to_dpaux(aux);
- int err;
- pm_runtime_get_sync(dpaux->dev);
- err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_AUX);
- pm_runtime_put(dpaux->dev);
- return tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_AUX);
- return err;
}
int drm_dp_aux_disable(struct drm_dp_aux *aux) { struct tegra_dpaux *dpaux = to_dpaux(aux);
pm_runtime_get_sync(dpaux->dev); tegra_dpaux_pad_power_down(dpaux);
pm_runtime_put(dpaux->dev);
return 0;
}
2.23.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel