[PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function

AngeloGioacchino Del Regno posted 10 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
Posted by AngeloGioacchino Del Regno 1 year, 4 months ago
Everytime we run bridge detection and/or EDID read we run a poweron
and poweroff sequence for both the AUX and the panel; moreover, this
is also done when enabling the bridge in the .atomic_enable() callback.

Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
function as to commonize it.
Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
the AUX was getting powered on but the panel was left powered off if
the DP cable wasn't plugged in while now we unconditionally send a D0
request and this is done for two reasons:
 - First, whether this request fails or not, it takes the same time
   and anyway the DP hardware won't produce any error (or, if it
   does, it's ignorable because it won't block further commands)
 - Second, training the link between a sleeping/standby/unpowered
   display makes little sense.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-------------------
 1 file changed, 30 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 64eee77452c0..8b7ded1746f3 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp *mtk_dp, bool mute)
 			   val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
 }
 
+static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool pwron)
+{
+	if (pwron) {
+		/* power on aux */
+		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
+				   DP_PWR_STATE_MASK);
+
+		/* power on panel */
+		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
+		usleep_range(2000, 5000);
+	} else {
+		/* power off panel */
+		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
+		usleep_range(2000, 3000);
+
+		/* power off aux */
+		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+				   DP_PWR_STATE_BANDGAP_TPLL,
+				   DP_PWR_STATE_MASK);
+	}
+}
+
 static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
 {
 	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
@@ -1936,16 +1959,9 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	if (!mtk_dp->train_info.cable_plugged_in)
 		return ret;
 
-	if (!enabled) {
-		/* power on aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-				   DP_PWR_STATE_MASK);
+	if (!enabled)
+		mtk_dp_aux_panel_poweron(mtk_dp, true);
 
-		/* power on panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
-	}
 	/*
 	 * Some dongles still source HPD when they do not connect to any
 	 * sink device. To avoid this, we need to read the sink count
@@ -1957,16 +1973,8 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	if (DP_GET_SINK_COUNT(sink_count))
 		ret = connector_status_connected;
 
-	if (!enabled) {
-		/* power off panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
-		usleep_range(2000, 3000);
-
-		/* power off aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL,
-				   DP_PWR_STATE_MASK);
-	}
+	if (!enabled)
+		mtk_dp_aux_panel_poweron(mtk_dp, false);
 
 	return ret;
 }
@@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
 
 	if (!enabled) {
 		drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state);
-
-		/* power on aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-				   DP_PWR_STATE_MASK);
-
-		/* power on panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
+		mtk_dp_aux_panel_poweron(mtk_dp, true);
 	}
 
 	new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
@@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
 	}
 
 	if (!enabled) {
-		/* power off panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
-		usleep_range(2000, 3000);
-
-		/* power off aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL,
-				   DP_PWR_STATE_MASK);
-
+		mtk_dp_aux_panel_poweron(mtk_dp, false);
 		drm_atomic_bridge_chain_post_disable(bridge, connector->state->state);
 	}
 
@@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 		return;
 	}
 
-	/* power on aux */
-	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-			   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-			   DP_PWR_STATE_MASK);
-
-	if (mtk_dp->train_info.cable_plugged_in) {
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
-	}
+	mtk_dp_aux_panel_poweron(mtk_dp, true);
 
 	/* Training */
 	ret = mtk_dp_training(mtk_dp);
-- 
2.40.1
Re: [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
Posted by CK Hu (胡俊光) 1 year, 3 months ago
Hi, Angelo:

On Thu, 2023-07-13 at 11:01 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Everytime we run bridge detection and/or EDID read we run a poweron
> and poweroff sequence for both the AUX and the panel; moreover, this
> is also done when enabling the bridge in the .atomic_enable()
> callback.
> 
> Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
> function as to commonize it.
> Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
> the AUX was getting powered on but the panel was left powered off if
> the DP cable wasn't plugged in while now we unconditionally send a D0
> request and this is done for two reasons:
>  - First, whether this request fails or not, it takes the same time
>    and anyway the DP hardware won't produce any error (or, if it
>    does, it's ignorable because it won't block further commands)
>  - Second, training the link between a sleeping/standby/unpowered
>    display makes little sense.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-----------------
> --
>  1 file changed, 30 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 64eee77452c0..8b7ded1746f3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp
> *mtk_dp, bool mute)
>  			   val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
>  }
>  
> +static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool
> pwron)
> +{
> +	if (pwron) {
> +		/* power on aux */
> +		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> +				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> +				   DP_PWR_STATE_MASK);
> +
> +		/* power on panel */
> +		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> +		usleep_range(2000, 5000);
> +	} else {
> +		/* power off panel */
> +		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> +		usleep_range(2000, 3000);
> +
> +		/* power off aux */
> +		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> +				   DP_PWR_STATE_BANDGAP_TPLL,
> +				   DP_PWR_STATE_MASK);
> +	}
> +}
> +
>  static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
>  {
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
> @@ -1936,16 +1959,9 @@ static enum drm_connector_status
> mtk_dp_bdg_detect(struct drm_bridge *bridge)
>  	if (!mtk_dp->train_info.cable_plugged_in)
>  		return ret;
>  
> -	if (!enabled) {
> -		/* power on aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -				   DP_PWR_STATE_MASK);
> +	if (!enabled)
> +		mtk_dp_aux_panel_poweron(mtk_dp, true);
>  
> -		/* power on panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> -	}
>  	/*
>  	 * Some dongles still source HPD when they do not connect to
> any
>  	 * sink device. To avoid this, we need to read the sink count
> @@ -1957,16 +1973,8 @@ static enum drm_connector_status
> mtk_dp_bdg_detect(struct drm_bridge *bridge)
>  	if (DP_GET_SINK_COUNT(sink_count))
>  		ret = connector_status_connected;
>  
> -	if (!enabled) {
> -		/* power off panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> -		usleep_range(2000, 3000);
> -
> -		/* power off aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL,
> -				   DP_PWR_STATE_MASK);
> -	}
> +	if (!enabled)
> +		mtk_dp_aux_panel_poweron(mtk_dp, false);
>  
>  	return ret;
>  }
> @@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct
> drm_bridge *bridge,
>  
>  	if (!enabled) {
>  		drm_atomic_bridge_chain_pre_enable(bridge, connector-
> >state->state);
> -
> -		/* power on aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -				   DP_PWR_STATE_MASK);
> -
> -		/* power on panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> +		mtk_dp_aux_panel_poweron(mtk_dp, true);
>  	}
>  
>  	new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
> @@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct
> drm_bridge *bridge,
>  	}
>  
>  	if (!enabled) {
> -		/* power off panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> -		usleep_range(2000, 3000);
> -
> -		/* power off aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL,
> -				   DP_PWR_STATE_MASK);
> -
> +		mtk_dp_aux_panel_poweron(mtk_dp, false);
>  		drm_atomic_bridge_chain_post_disable(bridge, connector-
> >state->state);
>  	}
>  
> @@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct
> drm_bridge *bridge,
>  		return;
>  	}
>  
> -	/* power on aux */
> -	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -			   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -			   DP_PWR_STATE_MASK);
> -
> -	if (mtk_dp->train_info.cable_plugged_in) {
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> -	}
> +	mtk_dp_aux_panel_poweron(mtk_dp, true);
>  
>  	/* Training */
>  	ret = mtk_dp_training(mtk_dp);
> -- 
> 2.40.1
Re: [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
Posted by Chen-Yu Tsai 1 year, 3 months ago
Hi CK,

On Tue, Jul 18, 2023 at 3:40 PM CK Hu (胡俊光) <ck.hu@mediatek.com> wrote:
>
> Hi, Angelo:
>
> On Thu, 2023-07-13 at 11:01 +0200, AngeloGioacchino Del Regno wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >  Everytime we run bridge detection and/or EDID read we run a poweron
> > and poweroff sequence for both the AUX and the panel; moreover, this
> > is also done when enabling the bridge in the .atomic_enable()
> > callback.
> >
> > Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
> > function as to commonize it.
> > Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
> > the AUX was getting powered on but the panel was left powered off if
> > the DP cable wasn't plugged in while now we unconditionally send a D0
> > request and this is done for two reasons:
> >  - First, whether this request fails or not, it takes the same time
> >    and anyway the DP hardware won't produce any error (or, if it
> >    does, it's ignorable because it won't block further commands)
> >  - Second, training the link between a sleeping/standby/unpowered
> >    display makes little sense.
>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>

There is a v6 already.

> >
> > Signed-off-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-----------------
> > --
> >  1 file changed, 30 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index 64eee77452c0..8b7ded1746f3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp
> > *mtk_dp, bool mute)
> >                          val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
> >  }
> >
> > +static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool
> > pwron)
> > +{
> > +     if (pwron) {
> > +             /* power on aux */
> > +             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > +                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > +                                DP_PWR_STATE_MASK);
> > +
> > +             /* power on panel */
> > +             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > +             usleep_range(2000, 5000);
> > +     } else {
> > +             /* power off panel */
> > +             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > +             usleep_range(2000, 3000);
> > +
> > +             /* power off aux */
> > +             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > +                                DP_PWR_STATE_BANDGAP_TPLL,
> > +                                DP_PWR_STATE_MASK);
> > +     }
> > +}
> > +
> >  static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
> >  {
> >       mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
> > @@ -1936,16 +1959,9 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >       if (!mtk_dp->train_info.cable_plugged_in)
> >               return ret;
> >
> > -     if (!enabled) {
> > -             /* power on aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                                DP_PWR_STATE_MASK);
> > +     if (!enabled)
> > +             mtk_dp_aux_panel_poweron(mtk_dp, true);
> >
> > -             /* power on panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > -     }
> >       /*
> >        * Some dongles still source HPD when they do not connect to
> > any
> >        * sink device. To avoid this, we need to read the sink count
> > @@ -1957,16 +1973,8 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >       if (DP_GET_SINK_COUNT(sink_count))
> >               ret = connector_status_connected;
> >
> > -     if (!enabled) {
> > -             /* power off panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > -             usleep_range(2000, 3000);
> > -
> > -             /* power off aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL,
> > -                                DP_PWR_STATE_MASK);
> > -     }
> > +     if (!enabled)
> > +             mtk_dp_aux_panel_poweron(mtk_dp, false);
> >
> >       return ret;
> >  }
> > @@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct
> > drm_bridge *bridge,
> >
> >       if (!enabled) {
> >               drm_atomic_bridge_chain_pre_enable(bridge, connector-
> > >state->state);
> > -
> > -             /* power on aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                                DP_PWR_STATE_MASK);
> > -
> > -             /* power on panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > +             mtk_dp_aux_panel_poweron(mtk_dp, true);
> >       }
> >
> >       new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
> > @@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct
> > drm_bridge *bridge,
> >       }
> >
> >       if (!enabled) {
> > -             /* power off panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > -             usleep_range(2000, 3000);
> > -
> > -             /* power off aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL,
> > -                                DP_PWR_STATE_MASK);
> > -
> > +             mtk_dp_aux_panel_poweron(mtk_dp, false);
> >               drm_atomic_bridge_chain_post_disable(bridge, connector-
> > >state->state);
> >       }
> >
> > @@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct
> > drm_bridge *bridge,
> >               return;
> >       }
> >
> > -     /* power on aux */
> > -     mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                        DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                        DP_PWR_STATE_MASK);
> > -
> > -     if (mtk_dp->train_info.cable_plugged_in) {
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > -     }
> > +     mtk_dp_aux_panel_poweron(mtk_dp, true);
> >
> >       /* Training */
> >       ret = mtk_dp_training(mtk_dp);
> > --
> > 2.40.1