| Message ID | 20230131150548.1614458-7-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: drm/i915/dp_mst: Fix MST payload removal during output disabling | expand |
On Tue, 2023-01-31 at 17:05 +0200, Imre Deak wrote: > Simplify the loop iterating the payloads by using a helper to get a > payload by its VCPI (keeping the list VCPI sorted). This also removes > the assumption that the biggest VCPI matches the number of payloads > (even though this holds now). > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Lyude Paul <lyude@redhat.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 45 ++++++++++++------- > 1 file changed, 28 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 8787df19f428b..0c04b96ae614c 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -4737,6 +4737,18 @@ static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr, > kfree(mst_edid); > } > > +static struct drm_dp_mst_atomic_payload * > +get_payload_by_vcpi(struct drm_dp_mst_topology_state *mst_state, int vcpi) > +{ > + struct drm_dp_mst_atomic_payload *payload; > + > + list_for_each_entry(payload, &mst_state->payloads, next) > + if (payload->vcpi == vcpi) > + return payload; > + > + return NULL; > +} > + > /** > * drm_dp_mst_dump_topology(): dump topology to seq file. > * @m: seq_file to dump output to > @@ -4748,7 +4760,6 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > struct drm_dp_mst_topology_mgr *mgr) > { > struct drm_dp_mst_topology_state *state; > - struct drm_dp_mst_atomic_payload *payload; > int i, ret; > > mutex_lock(&mgr->lock); > @@ -4768,24 +4779,24 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); > > seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | sink name |\n"); > - for (i = 0; i < mgr->max_payloads; i++) { > - list_for_each_entry(payload, &state->payloads, next) { > - char name[14]; > + for_each_set_bit(i, (unsigned long *)&state->payload_mask, > + BITS_PER_TYPE(state->payload_mask)) { Huh, TIL we have a for_each_set_bit() macro. neat. > + const struct drm_dp_mst_atomic_payload *payload = get_payload_by_vcpi(state, i + 1); > + char name[14]; > > - if (payload->vcpi != i + 1 || payload->delete) > - continue; > + if (payload->delete) > + continue; > > - fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > - seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > - i, > - payload->port->port_num, > - payload->vcpi, > - payload->vc_start_slot, > - payload->vc_start_slot + payload->time_slots - 1, > - payload->pbn, > - payload->dsc_enabled ? "Y" : "N", > - (*name != 0) ? name : "Unknown"); > - } > + fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > + seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > + i, > + payload->port->port_num, > + payload->vcpi, > + payload->vc_start_slot, > + payload->vc_start_slot + payload->time_slots - 1, > + payload->pbn, > + payload->dsc_enabled ? "Y" : "N", > + (*name != 0) ? name : "Unknown"); > } > > seq_printf(m, "\n*** DPCD Info ***\n");
On Tue, Jan 31, 2023 at 05:05:37PM +0200, Imre Deak wrote: > Simplify the loop iterating the payloads by using a helper to get a > payload by its VCPI (keeping the list VCPI sorted). This also removes > the assumption that the biggest VCPI matches the number of payloads > (even though this holds now). > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Lyude Paul <lyude@redhat.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 45 ++++++++++++------- > 1 file changed, 28 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 8787df19f428b..0c04b96ae614c 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -4737,6 +4737,18 @@ static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr, > kfree(mst_edid); > } > > +static struct drm_dp_mst_atomic_payload * > +get_payload_by_vcpi(struct drm_dp_mst_topology_state *mst_state, int vcpi) > +{ > + struct drm_dp_mst_atomic_payload *payload; > + > + list_for_each_entry(payload, &mst_state->payloads, next) > + if (payload->vcpi == vcpi) > + return payload; > + > + return NULL; > +} > + > /** > * drm_dp_mst_dump_topology(): dump topology to seq file. > * @m: seq_file to dump output to > @@ -4748,7 +4760,6 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > struct drm_dp_mst_topology_mgr *mgr) > { > struct drm_dp_mst_topology_state *state; > - struct drm_dp_mst_atomic_payload *payload; > int i, ret; > > mutex_lock(&mgr->lock); > @@ -4768,24 +4779,24 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); > > seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | sink name |\n"); > - for (i = 0; i < mgr->max_payloads; i++) { > - list_for_each_entry(payload, &state->payloads, next) { > - char name[14]; > + for_each_set_bit(i, (unsigned long *)&state->payload_mask, > + BITS_PER_TYPE(state->payload_mask)) { payload_mask is u32, unsigned long can be 64 bits. Does this actually work for both little and big endian? > + const struct drm_dp_mst_atomic_payload *payload = get_payload_by_vcpi(state, i + 1); > + char name[14]; > > - if (payload->vcpi != i + 1 || payload->delete) > - continue; > + if (payload->delete) > + continue; > > - fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > - seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > - i, > - payload->port->port_num, > - payload->vcpi, > - payload->vc_start_slot, > - payload->vc_start_slot + payload->time_slots - 1, > - payload->pbn, > - payload->dsc_enabled ? "Y" : "N", > - (*name != 0) ? name : "Unknown"); > - } > + fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > + seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > + i, > + payload->port->port_num, > + payload->vcpi, > + payload->vc_start_slot, > + payload->vc_start_slot + payload->time_slots - 1, > + payload->pbn, > + payload->dsc_enabled ? "Y" : "N", > + (*name != 0) ? name : "Unknown"); > } > > seq_printf(m, "\n*** DPCD Info ***\n"); > -- > 2.37.1
On Fri, Feb 03, 2023 at 02:22:44PM +0200, Ville Syrjälä wrote: > On Tue, Jan 31, 2023 at 05:05:37PM +0200, Imre Deak wrote: > > Simplify the loop iterating the payloads by using a helper to get a > > payload by its VCPI (keeping the list VCPI sorted). This also removes > > the assumption that the biggest VCPI matches the number of payloads > > (even though this holds now). > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Lyude Paul <lyude@redhat.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 45 ++++++++++++------- > > 1 file changed, 28 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > index 8787df19f428b..0c04b96ae614c 100644 > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > @@ -4737,6 +4737,18 @@ static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr, > > kfree(mst_edid); > > } > > > > +static struct drm_dp_mst_atomic_payload * > > +get_payload_by_vcpi(struct drm_dp_mst_topology_state *mst_state, int vcpi) > > +{ > > + struct drm_dp_mst_atomic_payload *payload; > > + > > + list_for_each_entry(payload, &mst_state->payloads, next) > > + if (payload->vcpi == vcpi) > > + return payload; > > + > > + return NULL; > > +} > > + > > /** > > * drm_dp_mst_dump_topology(): dump topology to seq file. > > * @m: seq_file to dump output to > > @@ -4748,7 +4760,6 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > > struct drm_dp_mst_topology_mgr *mgr) > > { > > struct drm_dp_mst_topology_state *state; > > - struct drm_dp_mst_atomic_payload *payload; > > int i, ret; > > > > mutex_lock(&mgr->lock); > > @@ -4768,24 +4779,24 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > > state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); > > > > seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | sink name |\n"); > > - for (i = 0; i < mgr->max_payloads; i++) { > > - list_for_each_entry(payload, &state->payloads, next) { > > - char name[14]; > > + for_each_set_bit(i, (unsigned long *)&state->payload_mask, > > + BITS_PER_TYPE(state->payload_mask)) { > > payload_mask is u32, unsigned long can be 64 bits. Does this > actually work for both little and big endian? That's a good catch, on big-endian it's bogus AFAICS. There doesn't seem to be a 32 bit version of for_each_set_bit, so would unsigned long mask = state->payload_mask; for_each_set_bit(i, &mask, BITS_PER_TYPE(mask)) be ok? > > > + const struct drm_dp_mst_atomic_payload *payload = get_payload_by_vcpi(state, i + 1); > > + char name[14]; > > > > - if (payload->vcpi != i + 1 || payload->delete) > > - continue; > > + if (payload->delete) > > + continue; > > > > - fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > > - seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > > - i, > > - payload->port->port_num, > > - payload->vcpi, > > - payload->vc_start_slot, > > - payload->vc_start_slot + payload->time_slots - 1, > > - payload->pbn, > > - payload->dsc_enabled ? "Y" : "N", > > - (*name != 0) ? name : "Unknown"); > > - } > > + fetch_monitor_name(mgr, payload->port, name, sizeof(name)); > > + seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", > > + i, > > + payload->port->port_num, > > + payload->vcpi, > > + payload->vc_start_slot, > > + payload->vc_start_slot + payload->time_slots - 1, > > + payload->pbn, > > + payload->dsc_enabled ? "Y" : "N", > > + (*name != 0) ? name : "Unknown"); > > } > > > > seq_printf(m, "\n*** DPCD Info ***\n"); > > -- > > 2.37.1 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 8787df19f428b..0c04b96ae614c 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -4737,6 +4737,18 @@ static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr, kfree(mst_edid); } +static struct drm_dp_mst_atomic_payload * +get_payload_by_vcpi(struct drm_dp_mst_topology_state *mst_state, int vcpi) +{ + struct drm_dp_mst_atomic_payload *payload; + + list_for_each_entry(payload, &mst_state->payloads, next) + if (payload->vcpi == vcpi) + return payload; + + return NULL; +} + /** * drm_dp_mst_dump_topology(): dump topology to seq file. * @m: seq_file to dump output to @@ -4748,7 +4760,6 @@ void drm_dp_mst_dump_topology(struct seq_file *m, struct drm_dp_mst_topology_mgr *mgr) { struct drm_dp_mst_topology_state *state; - struct drm_dp_mst_atomic_payload *payload; int i, ret; mutex_lock(&mgr->lock); @@ -4768,24 +4779,24 @@ void drm_dp_mst_dump_topology(struct seq_file *m, state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | sink name |\n"); - for (i = 0; i < mgr->max_payloads; i++) { - list_for_each_entry(payload, &state->payloads, next) { - char name[14]; + for_each_set_bit(i, (unsigned long *)&state->payload_mask, + BITS_PER_TYPE(state->payload_mask)) { + const struct drm_dp_mst_atomic_payload *payload = get_payload_by_vcpi(state, i + 1); + char name[14]; - if (payload->vcpi != i + 1 || payload->delete) - continue; + if (payload->delete) + continue; - fetch_monitor_name(mgr, payload->port, name, sizeof(name)); - seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", - i, - payload->port->port_num, - payload->vcpi, - payload->vc_start_slot, - payload->vc_start_slot + payload->time_slots - 1, - payload->pbn, - payload->dsc_enabled ? "Y" : "N", - (*name != 0) ? name : "Unknown"); - } + fetch_monitor_name(mgr, payload->port, name, sizeof(name)); + seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n", + i, + payload->port->port_num, + payload->vcpi, + payload->vc_start_slot, + payload->vc_start_slot + payload->time_slots - 1, + payload->pbn, + payload->dsc_enabled ? "Y" : "N", + (*name != 0) ? name : "Unknown"); } seq_printf(m, "\n*** DPCD Info ***\n");
Simplify the loop iterating the payloads by using a helper to get a payload by its VCPI (keeping the list VCPI sorted). This also removes the assumption that the biggest VCPI matches the number of payloads (even though this holds now). Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Lyude Paul <lyude@redhat.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-)