diff mbox series

[v2,1/2] drm/msm/dp: don't touch DP subconnector property in eDP case

Message ID 20231025092711.851168-2-dmitry.baryshkov@linaro.org (mailing list archive)
State Not Applicable
Headers show
Series drm/msm/dp: fix DP subconnector handling | expand

Commit Message

Dmitry Baryshkov Oct. 25, 2023, 9:23 a.m. UTC 
From: Abel Vesa <abel.vesa@linaro.org>

In case of the eDP connection there is no subconnetor and as such no
subconnector property. Put drm_dp_set_subconnector_property() calls
under the !is_edp condition.

Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type")
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Abhinav Kumar Oct. 27, 2023, 9:02 p.m. UTC | #1 
On 10/25/2023 2:23 AM, Dmitry Baryshkov wrote:
> From: Abel Vesa <abel.vesa@linaro.org>
> 
> In case of the eDP connection there is no subconnetor and as such no
> subconnector property. Put drm_dp_set_subconnector_property() calls
> under the !is_edp condition.
> 
> Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type")
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 

We still need to unify the calls to drm_dp_set_subconnector_property() 
for the hpd connect/disconnect places preferably in 
dp_display_send_hpd_notification().

That way, we would have had to make this change only in one location.

If you want to pursue that as a separate patch, I am fine as well.

Hence,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Dmitry Baryshkov Oct. 28, 2023, 12:37 a.m. UTC | #2 
On Sat, 28 Oct 2023 at 00:02, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 10/25/2023 2:23 AM, Dmitry Baryshkov wrote:
> > From: Abel Vesa <abel.vesa@linaro.org>
> >
> > In case of the eDP connection there is no subconnetor and as such no
> > subconnector property. Put drm_dp_set_subconnector_property() calls
> > under the !is_edp condition.
> >
> > Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type")
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> >
>
> We still need to unify the calls to drm_dp_set_subconnector_property()
> for the hpd connect/disconnect places preferably in
> dp_display_send_hpd_notification().
>
> That way, we would have had to make this change only in one location.

Good point, I'd like to take another look at the HPD handling in the
DP driver after we land the pending pm_runtime changes. As a part of
that I'll check the drm_dp_set_subconnector_property() calls.

> If you want to pursue that as a separate patch, I am fine as well.
>
> Hence,
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Jessica Zhang Nov. 1, 2023, 4:19 p.m. UTC | #3 
On 10/25/2023 2:23 AM, Dmitry Baryshkov wrote:
> From: Abel Vesa <abel.vesa@linaro.org>
> 
> In case of the eDP connection there is no subconnetor and as such no
> subconnector property. Put drm_dp_set_subconnector_property() calls
> under the !is_edp condition.
> 
> Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type")
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Tested-by: Jessica Zhang <quic_jesszhan@quicinc.com> # SC7280

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e329e03e068d..1b88fb52726f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -365,9 +365,11 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp,
>   	/* reset video pattern flag on disconnect */
>   	if (!hpd) {
>   		dp->panel->video_test = false;
> -		drm_dp_set_subconnector_property(dp->dp_display.connector,
> -						 connector_status_disconnected,
> -						 dp->panel->dpcd, dp->panel->downstream_ports);
> +		if (!dp->dp_display.is_edp)
> +			drm_dp_set_subconnector_property(dp->dp_display.connector,
> +							 connector_status_disconnected,
> +							 dp->panel->dpcd,
> +							 dp->panel->downstream_ports);
>   	}
>   
>   	dp->dp_display.is_connected = hpd;
> @@ -396,8 +398,11 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
>   
>   	dp_link_process_request(dp->link);
>   
> -	drm_dp_set_subconnector_property(dp->dp_display.connector, connector_status_connected,
> -					 dp->panel->dpcd, dp->panel->downstream_ports);
> +	if (!dp->dp_display.is_edp)
> +		drm_dp_set_subconnector_property(dp->dp_display.connector,
> +						 connector_status_connected,
> +						 dp->panel->dpcd,
> +						 dp->panel->downstream_ports);
>   
>   	edid = dp->panel->edid;
>   
> -- 
> 2.42.0
>
Johan Hovold Nov. 15, 2023, 8:08 a.m. UTC | #4 
On Wed, Oct 25, 2023 at 12:23:09PM +0300, Dmitry Baryshkov wrote:
> From: Abel Vesa <abel.vesa@linaro.org>
> 
> In case of the eDP connection there is no subconnetor and as such no
> subconnector property. Put drm_dp_set_subconnector_property() calls
> under the !is_edp condition.
> 
> Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type")
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Johan Hovold <johan+linaro@kernel.org>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e329e03e068d..1b88fb52726f 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -365,9 +365,11 @@  static int dp_display_send_hpd_notification(struct dp_display_private *dp,
 	/* reset video pattern flag on disconnect */
 	if (!hpd) {
 		dp->panel->video_test = false;
-		drm_dp_set_subconnector_property(dp->dp_display.connector,
-						 connector_status_disconnected,
-						 dp->panel->dpcd, dp->panel->downstream_ports);
+		if (!dp->dp_display.is_edp)
+			drm_dp_set_subconnector_property(dp->dp_display.connector,
+							 connector_status_disconnected,
+							 dp->panel->dpcd,
+							 dp->panel->downstream_ports);
 	}
 
 	dp->dp_display.is_connected = hpd;
@@ -396,8 +398,11 @@  static int dp_display_process_hpd_high(struct dp_display_private *dp)
 
 	dp_link_process_request(dp->link);
 
-	drm_dp_set_subconnector_property(dp->dp_display.connector, connector_status_connected,
-					 dp->panel->dpcd, dp->panel->downstream_ports);
+	if (!dp->dp_display.is_edp)
+		drm_dp_set_subconnector_property(dp->dp_display.connector,
+						 connector_status_connected,
+						 dp->panel->dpcd,
+						 dp->panel->downstream_ports);
 
 	edid = dp->panel->edid;