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@xxxxxxxxxxxxxxx>
> > Cc: Lyude Paul <lyude@xxxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > 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
