Hi,
On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti
<quic_sbillaka@xxxxxxxxxxx> wrote:
>
> The source device should ensure the sink is ready before proceeding to
> read the sink capability or performing any aux transactions. The sink
s/performing/perform
> will indicate its readiness by asserting the HPD line. The controller
> driver needs to wait for the hpd line to be asserted by the sink before
> performing any aux transactions.
>
> The eDP sink is assumed to be always connected. It needs power from the
> source and its HPD line will be asserted only after the panel is powered
> on. The panel power will be enabled from the panel-edp driver and only
> after that, the hpd line will be asserted.
>
> Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd
> line gets asserted to indicate the sink is connected and ready. Hence
> there is no need to wait for the hpd line to be asserted for a DP sink.
>
> Signed-off-by: Sankeerth Billakanti <quic_sbillaka@xxxxxxxxxxx>
> ---
>
> Changes in v6:
> - Wait for hpd high only for eDP
> - Split into smaller patches
>
> drivers/gpu/drm/msm/dp/dp_aux.c | 13 ++++++++++++-
> drivers/gpu/drm/msm/dp/dp_aux.h | 3 ++-
> drivers/gpu/drm/msm/dp/dp_catalog.c | 13 +++++++++++++
> drivers/gpu/drm/msm/dp/dp_catalog.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 3 ++-
> 5 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 6d36f63..a217c80 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -36,6 +36,7 @@ struct dp_aux_private {
> bool initted;
> u32 offset;
> u32 segment;
> + bool is_edp;
>
> struct drm_dp_aux dp_aux;
> };
> @@ -337,6 +338,14 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> goto exit;
> }
>
> + if (aux->is_edp) {
Adding a comment about _why_ you're doing this just for eDP would
probably be a good idea. Like maybe:
/*
* For eDP it's important to give a reasonably long wait here for HPD
* to be asserted. This is because the panel driver may have _just_
* turned on the panel and then tried to do an AUX transfer. The panel
* driver has no way of knowing when the panel is ready, so it's up
* to us to wait. For DP we never get into this situation so let's
* avoid ever doing the extra long wait for DP.
*/
> @@ -491,7 +500,8 @@ void dp_aux_unregister(struct drm_dp_aux *dp_aux)
> drm_dp_aux_unregister(dp_aux);
> }
>
> -struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog)
> +struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
> + bool is_edp)
nit: I think your indentation of the 2nd line isn't quite right.
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
> index 82afc8d..c99aeec 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.h
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.h
> @@ -16,7 +16,8 @@ void dp_aux_init(struct drm_dp_aux *dp_aux);
> void dp_aux_deinit(struct drm_dp_aux *dp_aux);
> void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
>
> -struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog);
> +struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
> + bool is_edp);
nit: I think your indentation of the 2nd line isn't quite right.
Things are pretty much nits, so FWIW:
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
