diff mbox series

[2/4] drm/dp_mst: Don't cache EDIDs for physical ports

Message ID 20210201120145.350258-2-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/dp_mst: Don't report ports connected if nothing is attached to them | expand

Commit Message

Imre Deak Feb. 1, 2021, 12:01 p.m. UTC
Caching EDIDs for physical ports prevents updating the EDID if a port
gets reconnected via a Connection Status Notification message, fix this.

Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case")
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Lin, Wayne Feb. 2, 2021, 3:38 a.m. UTC | #1
[AMD Public Use]

> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Monday, February 1, 2021 8:02 PM
> To: dri-devel@lists.freedesktop.org
> Cc: Lin, Wayne <Wayne.Lin@amd.com>; Lyude Paul <lyude@redhat.com>
> Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
>
> Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message,
> fix this.
>
> Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case")
> Cc: Wayne Lin <Wayne.Lin@amd.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index deb7995f42fa..309afe61afdd 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
>  }
>
>  if (port->pdt != DP_PEER_DEVICE_NONE &&
> -    drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> +    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
> +    port->port_num >= DP_MST_LOGICAL_PORT_0) {
Hi Imre Deak,

Thanks for the patch!
Just curious that you mean we don't want to fetch the EDID of the sst monitor like below case?
    Src->MST device ->SST monitor
I thought we still need to get the EDID even the monitor is connected to the physical output port of mst device.
Maybe what we should fix here is why the EDID is not get updated once reconnected via CSN message?

Thanks!
>  port->cached_edid = drm_get_edid(port->connector,
>   &port->aux.ddc);
>  drm_connector_set_tile_property(port->connector);
> --
> 2.25.1
Regards,
Wayne Lin
Imre Deak Feb. 2, 2021, 11:22 a.m. UTC | #2
On Tue, Feb 02, 2021 at 03:38:16AM +0000, Lin, Wayne wrote:
> [AMD Public Use]
> 
> > -----Original Message-----
> > From: Imre Deak <imre.deak@intel.com>
> > Sent: Monday, February 1, 2021 8:02 PM
> > To: dri-devel@lists.freedesktop.org
> > Cc: Lin, Wayne <Wayne.Lin@amd.com>; Lyude Paul <lyude@redhat.com>
> > Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
> >
> > Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message,
> > fix this.
> >
> > Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case")
> > Cc: Wayne Lin <Wayne.Lin@amd.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index deb7995f42fa..309afe61afdd 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
> >  }
> >
> >  if (port->pdt != DP_PEER_DEVICE_NONE &&
> > -    drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> > +    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
> > +    port->port_num >= DP_MST_LOGICAL_PORT_0) {
> Hi Imre Deak,
> 
> Thanks for the patch!
> Just curious that you mean we don't want to fetch the EDID of the sst
> monitor like below case?
>
>     Src->MST device ->SST monitor

The intention of the mst cached_edid logic is to cache the EDID for
logical ports where the EDID cannot change anyway. The EDID on physical
ports is fetched during connector probing just as for any other
connector.

> I thought we still need to get the EDID even the monitor is connected
> to the physical output port of mst device.

For sinks attached to phyisical ports we get the EDID whenever probing
the corresponding connector.

> Maybe what we should fix here is why the EDID is not get updated once
> reconnected via CSN message?

This patch fixes the problem that we stopped updating the EDID for
physical connectors. After this change it will get updated when probing
such connectors.

> Thanks!
> >  port->cached_edid = drm_get_edid(port->connector,
> >   &port->aux.ddc);
> >  drm_connector_set_tile_property(port->connector);
> > --
> > 2.25.1
> Regards,
> Wayne Lin
Lin, Wayne Feb. 3, 2021, 2:56 a.m. UTC | #3
[AMD Official Use Only - Internal Distribution Only]

> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Tuesday, February 2, 2021 7:22 PM
> To: Lin, Wayne <Wayne.Lin@amd.com>
> Cc: dri-devel@lists.freedesktop.org; Lyude Paul <lyude@redhat.com>
> Subject: Re: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
>
> On Tue, Feb 02, 2021 at 03:38:16AM +0000, Lin, Wayne wrote:
> > [AMD Public Use]
> >
> > > -----Original Message-----
> > > From: Imre Deak <imre.deak@intel.com>
> > > Sent: Monday, February 1, 2021 8:02 PM
> > > To: dri-devel@lists.freedesktop.org
> > > Cc: Lin, Wayne <Wayne.Lin@amd.com>; Lyude Paul <lyude@redhat.com>
> > > Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical
> > > ports
> > >
> > > Caching EDIDs for physical ports prevents updating the EDID if a
> > > port gets reconnected via a Connection Status Notification message, fix this.
> > >
> > > Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device
> > > case")
> > > Cc: Wayne Lin <Wayne.Lin@amd.com>
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index deb7995f42fa..309afe61afdd 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct
> > > drm_dp_mst_branch *mstb,  }
> > >
> > >  if (port->pdt != DP_PEER_DEVICE_NONE &&
> > > -    drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> > > +    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
> > > +    port->port_num >= DP_MST_LOGICAL_PORT_0) {
> > Hi Imre Deak,
> >
> > Thanks for the patch!
> > Just curious that you mean we don't want to fetch the EDID of the sst
> > monitor like below case?
> >
> >     Src->MST device ->SST monitor
>
> The intention of the mst cached_edid logic is to cache the EDID for logical ports where the EDID cannot change anyway. The EDID on
> physical ports is fetched during connector probing just as for any other connector.
>
> > I thought we still need to get the EDID even the monitor is connected
> > to the physical output port of mst device.
>
> For sinks attached to phyisical ports we get the EDID whenever probing the corresponding connector.
>
> > Maybe what we should fix here is why the EDID is not get updated once
> > reconnected via CSN message?
>
> This patch fixes the problem that we stopped updating the EDID for physical connectors. After this change it will get updated when
> probing such connectors.
>
Appreciate for the explanation.
Thanks!
> > Thanks!
> > >  port->cached_edid = drm_get_edid(port->connector,
> > >   &port->aux.ddc);
> > >  drm_connector_set_tile_property(port->connector);
> > > --
> > > 2.25.1
> > Regards,
> > Wayne Lin
Regards,
Wayne Lin
Imre Deak Feb. 4, 2021, 5:54 p.m. UTC | #4
On Mon, Feb 01, 2021 at 02:01:43PM +0200, Imre Deak wrote:
> Caching EDIDs for physical ports prevents updating the EDID if a port
> gets reconnected via a Connection Status Notification message, fix this.
> 
> Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case")
> Cc: Wayne Lin <Wayne.Lin@amd.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Pushed patches 2-4 to drm-misc-next, thanks for the review.

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index deb7995f42fa..309afe61afdd 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
>  	}
>  
>  	if (port->pdt != DP_PEER_DEVICE_NONE &&
> -	    drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> +	    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
> +	    port->port_num >= DP_MST_LOGICAL_PORT_0) {
>  		port->cached_edid = drm_get_edid(port->connector,
>  						 &port->aux.ddc);
>  		drm_connector_set_tile_property(port->connector);
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index deb7995f42fa..309afe61afdd 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2302,7 +2302,8 @@  drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
 	}
 
 	if (port->pdt != DP_PEER_DEVICE_NONE &&
-	    drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
+	    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
+	    port->port_num >= DP_MST_LOGICAL_PORT_0) {
 		port->cached_edid = drm_get_edid(port->connector,
 						 &port->aux.ddc);
 		drm_connector_set_tile_property(port->connector);