| Message ID | 20231116131841.1588781-2-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix UHBR data, link M/N/TU and PBN values | expand |
On Thu, Nov 16, 2023 at 03:18:31PM +0200, Imre Deak wrote: > On UHBR links the PBN divider is a fractional number, accordingly store > it in fixed point format. For now drm_dp_get_vc_payload_bw() always > returns a whole number and all callers will use only the integer part of > it which should preserve the current behavior. The next patch will fix > drm_dp_get_vc_payload_bw() for UHBR rates returning a fractional number > for those (also accounting for the channel coding efficiency correctly). > > Cc: Lyude Paul <lyude@redhat.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Wayne Lin <wayne.lin@amd.com> > Cc: amd-gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++-- > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 ++- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +++-- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++------ > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +++-- > include/drm/display/drm_dp_mst_helper.h | 7 +++--- > 7 files changed, 33 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 74f9f02abcdec..12346b21d0b05 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -85,6 +85,7 @@ > #include <drm/drm_atomic_uapi.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_blend.h> > +#include <drm/drm_fixed.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_edid.h> > #include <drm/drm_eld.h> > @@ -6909,8 +6910,8 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, > if (IS_ERR(mst_state)) > return PTR_ERR(mst_state); > > - if (!mst_state->pbn_div) > - mst_state->pbn_div = dm_mst_get_pbn_divider(aconnector->mst_root->dc_link); > + if (!mst_state->pbn_div.full) > + mst_state->pbn_div.full = dfixed_const(dm_mst_get_pbn_divider(aconnector->mst_root->dc_link)); Why doesn't that dfixed stuff return the correct type? Anyways looks mostly mechanical Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > if (!state->duplicated) { > int max_bpc = conn_state->max_requested_bpc; > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index ed784cf27d396..63024393b516e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -31,6 +31,7 @@ > #include <drm/drm_probe_helper.h> > #include <drm/amdgpu_drm.h> > #include <drm/drm_edid.h> > +#include <drm/drm_fixed.h> > > #include "dm_services.h" > #include "amdgpu.h" > @@ -210,7 +211,7 @@ static void dm_helpers_construct_old_payload( > struct drm_dp_mst_atomic_payload *old_payload) > { > struct drm_dp_mst_atomic_payload *pos; > - int pbn_per_slot = mst_state->pbn_div; > + int pbn_per_slot = dfixed_trunc(mst_state->pbn_div); > u8 next_payload_vc_start = mgr->next_start_slot; > u8 payload_vc_start = new_payload->vc_start_slot; > u8 allocated_time_slots; > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 9a58e1a4c5f49..d1ba3ae228b08 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -27,6 +27,7 @@ > #include <drm/display/drm_dp_mst_helper.h> > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_fixed.h> > #include "dm_services.h" > #include "amdgpu.h" > #include "amdgpu_dm.h" > @@ -941,10 +942,10 @@ static int increase_dsc_bpp(struct drm_atomic_state *state, > link_timeslots_used = 0; > > for (i = 0; i < count; i++) > - link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, mst_state->pbn_div); > + link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, dfixed_trunc(mst_state->pbn_div)); > > fair_pbn_alloc = > - (63 - link_timeslots_used) / remaining_to_increase * mst_state->pbn_div; > + (63 - link_timeslots_used) / remaining_to_increase * dfixed_trunc(mst_state->pbn_div); > > if (initial_slack[next_index] > fair_pbn_alloc) { > vars[next_index].pbn += fair_pbn_alloc; > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index 4d72c9a32026e..000d05e80352a 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -43,6 +43,7 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_drv.h> > #include <drm/drm_edid.h> > +#include <drm/drm_fixed.h> > #include <drm/drm_print.h> > #include <drm/drm_probe_helper.h> > > @@ -3578,16 +3579,22 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, > * value is in units of PBNs/(timeslots/1 MTP). This value can be used to > * convert the number of PBNs required for a given stream to the number of > * timeslots this stream requires in each MTP. > + * > + * Returns the BW / timeslot value in 20.12 fixed point format. > */ > -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > - int link_rate, int link_lane_count) > +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > + int link_rate, int link_lane_count) > { > + fixed20_12 ret; > + > if (link_rate == 0 || link_lane_count == 0) > drm_dbg_kms(mgr->dev, "invalid link rate/lane count: (%d / %d)\n", > link_rate, link_lane_count); > > /* See DP v2.0 2.6.4.2, VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ > - return link_rate * link_lane_count / 54000; > + ret.full = dfixed_const(link_rate * link_lane_count / 54000); > + > + return ret; > } > EXPORT_SYMBOL(drm_dp_get_vc_payload_bw); > > @@ -4335,7 +4342,7 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state, > } > } > > - req_slots = DIV_ROUND_UP(pbn, topology_state->pbn_div); > + req_slots = DIV_ROUND_UP(pbn, dfixed_trunc(topology_state->pbn_div)); > > drm_dbg_atomic(mgr->dev, "[CONNECTOR:%d:%s] [MST PORT:%p] TU %d -> %d\n", > port->connector->base.id, port->connector->name, > @@ -4872,7 +4879,8 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > state = to_drm_dp_mst_topology_state(mgr->base.state); > seq_printf(m, "\n*** Atomic state info ***\n"); > seq_printf(m, "payload_mask: %x, max_payloads: %d, start_slot: %u, pbn_div: %d\n", > - state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); > + state->payload_mask, mgr->max_payloads, state->start_slot, > + dfixed_trunc(state->pbn_div)); > > seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | status | sink name |\n"); > for (i = 0; i < mgr->max_payloads; i++) { > @@ -5330,10 +5338,10 @@ drm_dp_mst_atomic_check_payload_alloc_limits(struct drm_dp_mst_topology_mgr *mgr > } > > if (!payload_count) > - mst_state->pbn_div = 0; > + mst_state->pbn_div.full = dfixed_const(0); > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p TU pbn_div=%d avail=%d used=%d\n", > - mgr, mst_state, mst_state->pbn_div, avail_slots, > + mgr, mst_state, dfixed_trunc(mst_state->pbn_div), avail_slots, > mst_state->total_avail_slots - avail_slots); > > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 0cb9405f59eaa..e5d6b811c22ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -26,6 +26,7 @@ > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_edid.h> > +#include <drm/drm_fixed.h> > #include <drm/drm_probe_helper.h> > > #include "i915_drv.h" > @@ -202,7 +203,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > */ > drm_WARN_ON(&i915->drm, remote_m_n.tu < crtc_state->dp_m_n.tu); > crtc_state->dp_m_n.tu = remote_m_n.tu; > - crtc_state->pbn = remote_m_n.tu * mst_state->pbn_div; > + crtc_state->pbn = remote_m_n.tu * dfixed_trunc(mst_state->pbn_div); > > slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr, > connector->port, > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 493fe4660f651..11fe75b68e95c 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -40,6 +40,7 @@ > #include <drm/drm_edid.h> > #include <drm/drm_eld.h> > #include <drm/drm_fb_helper.h> > +#include <drm/drm_fixed.h> > #include <drm/drm_probe_helper.h> > #include <drm/drm_vblank.h> > > @@ -946,7 +947,8 @@ nv50_msto_prepare(struct drm_atomic_state *state, > if (ret == 0) { > nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, > payload->vc_start_slot, payload->time_slots, > - payload->pbn, payload->time_slots * mst_state->pbn_div); > + payload->pbn, > + payload->time_slots * dfixed_trunc(mst_state->pbn_div)); > } else { > nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, 0, 0, 0, 0); > } > @@ -990,7 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > if (IS_ERR(mst_state)) > return PTR_ERR(mst_state); > > - if (!mst_state->pbn_div) { > + if (!mst_state->pbn_div.full) { > struct nouveau_encoder *outp = mstc->mstm->outp; > > mst_state->pbn_div = drm_dp_get_vc_payload_bw(&mstm->mgr, > diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h > index a4aad6df71f18..9b19d8bd520af 100644 > --- a/include/drm/display/drm_dp_mst_helper.h > +++ b/include/drm/display/drm_dp_mst_helper.h > @@ -25,6 +25,7 @@ > #include <linux/types.h> > #include <drm/display/drm_dp_helper.h> > #include <drm/drm_atomic.h> > +#include <drm/drm_fixed.h> > > #if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > #include <linux/stackdepot.h> > @@ -617,7 +618,7 @@ struct drm_dp_mst_topology_state { > * @pbn_div: The current PBN divisor for this topology. The driver is expected to fill this > * out itself. > */ > - int pbn_div; > + fixed20_12 pbn_div; > }; > > #define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base) > @@ -839,8 +840,8 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, > struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port); > > -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > - int link_rate, int link_lane_count); > +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > + int link_rate, int link_lane_count); > > int drm_dp_calc_pbn_mode(int clock, int bpp); > > -- > 2.39.2
On Fri, Nov 17, 2023 at 12:56:36PM +0200, Ville Syrjälä wrote: > On Thu, Nov 16, 2023 at 03:18:31PM +0200, Imre Deak wrote: > > On UHBR links the PBN divider is a fractional number, accordingly store > > it in fixed point format. For now drm_dp_get_vc_payload_bw() always > > returns a whole number and all callers will use only the integer part of > > it which should preserve the current behavior. The next patch will fix > > drm_dp_get_vc_payload_bw() for UHBR rates returning a fractional number > > for those (also accounting for the channel coding efficiency correctly). > > > > Cc: Lyude Paul <lyude@redhat.com> > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > Cc: Wayne Lin <wayne.lin@amd.com> > > Cc: amd-gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++-- > > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 ++- > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +++-- > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++------ > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +++-- > > include/drm/display/drm_dp_mst_helper.h | 7 +++--- > > 7 files changed, 33 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > index 74f9f02abcdec..12346b21d0b05 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -85,6 +85,7 @@ > > #include <drm/drm_atomic_uapi.h> > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_blend.h> > > +#include <drm/drm_fixed.h> > > #include <drm/drm_fourcc.h> > > #include <drm/drm_edid.h> > > #include <drm/drm_eld.h> > > @@ -6909,8 +6910,8 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, > > if (IS_ERR(mst_state)) > > return PTR_ERR(mst_state); > > > > - if (!mst_state->pbn_div) > > - mst_state->pbn_div = dm_mst_get_pbn_divider(aconnector->mst_root->dc_link); > > + if (!mst_state->pbn_div.full) > > + mst_state->pbn_div.full = dfixed_const(dm_mst_get_pbn_divider(aconnector->mst_root->dc_link)); > > Why doesn't that dfixed stuff return the correct type? AFAICS a follow up change could make dfixed_init() return the correct type and then change all fp.full = dfixed_const(A) to fp = dfixed_init(A) > > Anyways looks mostly mechanical > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > if (!state->duplicated) { > > int max_bpc = conn_state->max_requested_bpc; > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > index ed784cf27d396..63024393b516e 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > @@ -31,6 +31,7 @@ > > #include <drm/drm_probe_helper.h> > > #include <drm/amdgpu_drm.h> > > #include <drm/drm_edid.h> > > +#include <drm/drm_fixed.h> > > > > #include "dm_services.h" > > #include "amdgpu.h" > > @@ -210,7 +211,7 @@ static void dm_helpers_construct_old_payload( > > struct drm_dp_mst_atomic_payload *old_payload) > > { > > struct drm_dp_mst_atomic_payload *pos; > > - int pbn_per_slot = mst_state->pbn_div; > > + int pbn_per_slot = dfixed_trunc(mst_state->pbn_div); > > u8 next_payload_vc_start = mgr->next_start_slot; > > u8 payload_vc_start = new_payload->vc_start_slot; > > u8 allocated_time_slots; > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > index 9a58e1a4c5f49..d1ba3ae228b08 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > > @@ -27,6 +27,7 @@ > > #include <drm/display/drm_dp_mst_helper.h> > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > +#include <drm/drm_fixed.h> > > #include "dm_services.h" > > #include "amdgpu.h" > > #include "amdgpu_dm.h" > > @@ -941,10 +942,10 @@ static int increase_dsc_bpp(struct drm_atomic_state *state, > > link_timeslots_used = 0; > > > > for (i = 0; i < count; i++) > > - link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, mst_state->pbn_div); > > + link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, dfixed_trunc(mst_state->pbn_div)); > > > > fair_pbn_alloc = > > - (63 - link_timeslots_used) / remaining_to_increase * mst_state->pbn_div; > > + (63 - link_timeslots_used) / remaining_to_increase * dfixed_trunc(mst_state->pbn_div); > > > > if (initial_slack[next_index] > fair_pbn_alloc) { > > vars[next_index].pbn += fair_pbn_alloc; > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > index 4d72c9a32026e..000d05e80352a 100644 > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > @@ -43,6 +43,7 @@ > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_drv.h> > > #include <drm/drm_edid.h> > > +#include <drm/drm_fixed.h> > > #include <drm/drm_print.h> > > #include <drm/drm_probe_helper.h> > > > > @@ -3578,16 +3579,22 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, > > * value is in units of PBNs/(timeslots/1 MTP). This value can be used to > > * convert the number of PBNs required for a given stream to the number of > > * timeslots this stream requires in each MTP. > > + * > > + * Returns the BW / timeslot value in 20.12 fixed point format. > > */ > > -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > > - int link_rate, int link_lane_count) > > +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > > + int link_rate, int link_lane_count) > > { > > + fixed20_12 ret; > > + > > if (link_rate == 0 || link_lane_count == 0) > > drm_dbg_kms(mgr->dev, "invalid link rate/lane count: (%d / %d)\n", > > link_rate, link_lane_count); > > > > /* See DP v2.0 2.6.4.2, VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ > > - return link_rate * link_lane_count / 54000; > > + ret.full = dfixed_const(link_rate * link_lane_count / 54000); > > + > > + return ret; > > } > > EXPORT_SYMBOL(drm_dp_get_vc_payload_bw); > > > > @@ -4335,7 +4342,7 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state, > > } > > } > > > > - req_slots = DIV_ROUND_UP(pbn, topology_state->pbn_div); > > + req_slots = DIV_ROUND_UP(pbn, dfixed_trunc(topology_state->pbn_div)); > > > > drm_dbg_atomic(mgr->dev, "[CONNECTOR:%d:%s] [MST PORT:%p] TU %d -> %d\n", > > port->connector->base.id, port->connector->name, > > @@ -4872,7 +4879,8 @@ void drm_dp_mst_dump_topology(struct seq_file *m, > > state = to_drm_dp_mst_topology_state(mgr->base.state); > > seq_printf(m, "\n*** Atomic state info ***\n"); > > seq_printf(m, "payload_mask: %x, max_payloads: %d, start_slot: %u, pbn_div: %d\n", > > - state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); > > + state->payload_mask, mgr->max_payloads, state->start_slot, > > + dfixed_trunc(state->pbn_div)); > > > > seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | status | sink name |\n"); > > for (i = 0; i < mgr->max_payloads; i++) { > > @@ -5330,10 +5338,10 @@ drm_dp_mst_atomic_check_payload_alloc_limits(struct drm_dp_mst_topology_mgr *mgr > > } > > > > if (!payload_count) > > - mst_state->pbn_div = 0; > > + mst_state->pbn_div.full = dfixed_const(0); > > > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p TU pbn_div=%d avail=%d used=%d\n", > > - mgr, mst_state, mst_state->pbn_div, avail_slots, > > + mgr, mst_state, dfixed_trunc(mst_state->pbn_div), avail_slots, > > mst_state->total_avail_slots - avail_slots); > > > > return 0; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 0cb9405f59eaa..e5d6b811c22ef 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -26,6 +26,7 @@ > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_edid.h> > > +#include <drm/drm_fixed.h> > > #include <drm/drm_probe_helper.h> > > > > #include "i915_drv.h" > > @@ -202,7 +203,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > */ > > drm_WARN_ON(&i915->drm, remote_m_n.tu < crtc_state->dp_m_n.tu); > > crtc_state->dp_m_n.tu = remote_m_n.tu; > > - crtc_state->pbn = remote_m_n.tu * mst_state->pbn_div; > > + crtc_state->pbn = remote_m_n.tu * dfixed_trunc(mst_state->pbn_div); > > > > slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr, > > connector->port, > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > > index 493fe4660f651..11fe75b68e95c 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > > @@ -40,6 +40,7 @@ > > #include <drm/drm_edid.h> > > #include <drm/drm_eld.h> > > #include <drm/drm_fb_helper.h> > > +#include <drm/drm_fixed.h> > > #include <drm/drm_probe_helper.h> > > #include <drm/drm_vblank.h> > > > > @@ -946,7 +947,8 @@ nv50_msto_prepare(struct drm_atomic_state *state, > > if (ret == 0) { > > nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, > > payload->vc_start_slot, payload->time_slots, > > - payload->pbn, payload->time_slots * mst_state->pbn_div); > > + payload->pbn, > > + payload->time_slots * dfixed_trunc(mst_state->pbn_div)); > > } else { > > nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, 0, 0, 0, 0); > > } > > @@ -990,7 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > > if (IS_ERR(mst_state)) > > return PTR_ERR(mst_state); > > > > - if (!mst_state->pbn_div) { > > + if (!mst_state->pbn_div.full) { > > struct nouveau_encoder *outp = mstc->mstm->outp; > > > > mst_state->pbn_div = drm_dp_get_vc_payload_bw(&mstm->mgr, > > diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h > > index a4aad6df71f18..9b19d8bd520af 100644 > > --- a/include/drm/display/drm_dp_mst_helper.h > > +++ b/include/drm/display/drm_dp_mst_helper.h > > @@ -25,6 +25,7 @@ > > #include <linux/types.h> > > #include <drm/display/drm_dp_helper.h> > > #include <drm/drm_atomic.h> > > +#include <drm/drm_fixed.h> > > > > #if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) > > #include <linux/stackdepot.h> > > @@ -617,7 +618,7 @@ struct drm_dp_mst_topology_state { > > * @pbn_div: The current PBN divisor for this topology. The driver is expected to fill this > > * out itself. > > */ > > - int pbn_div; > > + fixed20_12 pbn_div; > > }; > > > > #define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base) > > @@ -839,8 +840,8 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, > > struct drm_dp_mst_topology_mgr *mgr, > > struct drm_dp_mst_port *port); > > > > -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > > - int link_rate, int link_lane_count); > > +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, > > + int link_rate, int link_lane_count); > > > > int drm_dp_calc_pbn_mode(int clock, int bpp); > > > > -- > > 2.39.2 > > -- > Ville Syrjälä > Intel
On Thu, Nov 16, 2023 at 03:18:31PM +0200, Imre Deak wrote: [...] > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index ed784cf27d396..63024393b516e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -31,6 +31,7 @@ > #include <drm/drm_probe_helper.h> > #include <drm/amdgpu_drm.h> > #include <drm/drm_edid.h> > +#include <drm/drm_fixed.h> > > #include "dm_services.h" > #include "amdgpu.h" > @@ -210,7 +211,7 @@ static void dm_helpers_construct_old_payload( > struct drm_dp_mst_atomic_payload *old_payload) > { > struct drm_dp_mst_atomic_payload *pos; > - int pbn_per_slot = mst_state->pbn_div; > + int pbn_per_slot = dfixed_trunc(mst_state->pbn_div); > u8 next_payload_vc_start = mgr->next_start_slot; > u8 payload_vc_start = new_payload->vc_start_slot; > u8 allocated_time_slots; I'm planning to merge this patchset today via drm-intel-next and for that I'll need to rebase the above change to: @@ -205,13 +206,14 @@ void dm_helpers_dp_update_branch_info( static void dm_helpers_construct_old_payload( struct dc_link *link, - int pbn_per_slot, + fixed20_12 pbn_per_slot_fp, struct drm_dp_mst_atomic_payload *new_payload, struct drm_dp_mst_atomic_payload *old_payload) { struct link_mst_stream_allocation_table current_link_table = link->mst_stream_alloc_table; struct link_mst_stream_allocation *dc_alloc; + int pbn_per_slot = dfixed_trunc(pbn_per_slot_fp); int i; *old_payload = *new_payload; and then apply the original changes in the patch above while merging drm-intel-next to drm-tip. This is required due to commit 9031e0013f819c ("drm/amd/display: Fix mst hub unplug warning") being only in drm-misc-next, but not yet in drm-intel-next. Let me know if you have a concern with this. --Imre
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 74f9f02abcdec..12346b21d0b05 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -85,6 +85,7 @@ #include <drm/drm_atomic_uapi.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_blend.h> +#include <drm/drm_fixed.h> #include <drm/drm_fourcc.h> #include <drm/drm_edid.h> #include <drm/drm_eld.h> @@ -6909,8 +6910,8 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, if (IS_ERR(mst_state)) return PTR_ERR(mst_state); - if (!mst_state->pbn_div) - mst_state->pbn_div = dm_mst_get_pbn_divider(aconnector->mst_root->dc_link); + if (!mst_state->pbn_div.full) + mst_state->pbn_div.full = dfixed_const(dm_mst_get_pbn_divider(aconnector->mst_root->dc_link)); if (!state->duplicated) { int max_bpc = conn_state->max_requested_bpc; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index ed784cf27d396..63024393b516e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -31,6 +31,7 @@ #include <drm/drm_probe_helper.h> #include <drm/amdgpu_drm.h> #include <drm/drm_edid.h> +#include <drm/drm_fixed.h> #include "dm_services.h" #include "amdgpu.h" @@ -210,7 +211,7 @@ static void dm_helpers_construct_old_payload( struct drm_dp_mst_atomic_payload *old_payload) { struct drm_dp_mst_atomic_payload *pos; - int pbn_per_slot = mst_state->pbn_div; + int pbn_per_slot = dfixed_trunc(mst_state->pbn_div); u8 next_payload_vc_start = mgr->next_start_slot; u8 payload_vc_start = new_payload->vc_start_slot; u8 allocated_time_slots; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 9a58e1a4c5f49..d1ba3ae228b08 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -27,6 +27,7 @@ #include <drm/display/drm_dp_mst_helper.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_fixed.h> #include "dm_services.h" #include "amdgpu.h" #include "amdgpu_dm.h" @@ -941,10 +942,10 @@ static int increase_dsc_bpp(struct drm_atomic_state *state, link_timeslots_used = 0; for (i = 0; i < count; i++) - link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, mst_state->pbn_div); + link_timeslots_used += DIV_ROUND_UP(vars[i + k].pbn, dfixed_trunc(mst_state->pbn_div)); fair_pbn_alloc = - (63 - link_timeslots_used) / remaining_to_increase * mst_state->pbn_div; + (63 - link_timeslots_used) / remaining_to_increase * dfixed_trunc(mst_state->pbn_div); if (initial_slack[next_index] > fair_pbn_alloc) { vars[next_index].pbn += fair_pbn_alloc; diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 4d72c9a32026e..000d05e80352a 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -43,6 +43,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> #include <drm/drm_edid.h> +#include <drm/drm_fixed.h> #include <drm/drm_print.h> #include <drm/drm_probe_helper.h> @@ -3578,16 +3579,22 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, * value is in units of PBNs/(timeslots/1 MTP). This value can be used to * convert the number of PBNs required for a given stream to the number of * timeslots this stream requires in each MTP. + * + * Returns the BW / timeslot value in 20.12 fixed point format. */ -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, - int link_rate, int link_lane_count) +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, + int link_rate, int link_lane_count) { + fixed20_12 ret; + if (link_rate == 0 || link_lane_count == 0) drm_dbg_kms(mgr->dev, "invalid link rate/lane count: (%d / %d)\n", link_rate, link_lane_count); /* See DP v2.0 2.6.4.2, VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ - return link_rate * link_lane_count / 54000; + ret.full = dfixed_const(link_rate * link_lane_count / 54000); + + return ret; } EXPORT_SYMBOL(drm_dp_get_vc_payload_bw); @@ -4335,7 +4342,7 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state, } } - req_slots = DIV_ROUND_UP(pbn, topology_state->pbn_div); + req_slots = DIV_ROUND_UP(pbn, dfixed_trunc(topology_state->pbn_div)); drm_dbg_atomic(mgr->dev, "[CONNECTOR:%d:%s] [MST PORT:%p] TU %d -> %d\n", port->connector->base.id, port->connector->name, @@ -4872,7 +4879,8 @@ void drm_dp_mst_dump_topology(struct seq_file *m, state = to_drm_dp_mst_topology_state(mgr->base.state); seq_printf(m, "\n*** Atomic state info ***\n"); seq_printf(m, "payload_mask: %x, max_payloads: %d, start_slot: %u, pbn_div: %d\n", - state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div); + state->payload_mask, mgr->max_payloads, state->start_slot, + dfixed_trunc(state->pbn_div)); seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | status | sink name |\n"); for (i = 0; i < mgr->max_payloads; i++) { @@ -5330,10 +5338,10 @@ drm_dp_mst_atomic_check_payload_alloc_limits(struct drm_dp_mst_topology_mgr *mgr } if (!payload_count) - mst_state->pbn_div = 0; + mst_state->pbn_div.full = dfixed_const(0); drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p TU pbn_div=%d avail=%d used=%d\n", - mgr, mst_state, mst_state->pbn_div, avail_slots, + mgr, mst_state, dfixed_trunc(mst_state->pbn_div), avail_slots, mst_state->total_avail_slots - avail_slots); return 0; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 0cb9405f59eaa..e5d6b811c22ef 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -26,6 +26,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_edid.h> +#include <drm/drm_fixed.h> #include <drm/drm_probe_helper.h> #include "i915_drv.h" @@ -202,7 +203,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, */ drm_WARN_ON(&i915->drm, remote_m_n.tu < crtc_state->dp_m_n.tu); crtc_state->dp_m_n.tu = remote_m_n.tu; - crtc_state->pbn = remote_m_n.tu * mst_state->pbn_div; + crtc_state->pbn = remote_m_n.tu * dfixed_trunc(mst_state->pbn_div); slots = drm_dp_atomic_find_time_slots(state, &intel_dp->mst_mgr, connector->port, diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 493fe4660f651..11fe75b68e95c 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -40,6 +40,7 @@ #include <drm/drm_edid.h> #include <drm/drm_eld.h> #include <drm/drm_fb_helper.h> +#include <drm/drm_fixed.h> #include <drm/drm_probe_helper.h> #include <drm/drm_vblank.h> @@ -946,7 +947,8 @@ nv50_msto_prepare(struct drm_atomic_state *state, if (ret == 0) { nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, payload->vc_start_slot, payload->time_slots, - payload->pbn, payload->time_slots * mst_state->pbn_div); + payload->pbn, + payload->time_slots * dfixed_trunc(mst_state->pbn_div)); } else { nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, 0, 0, 0, 0); } @@ -990,7 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, if (IS_ERR(mst_state)) return PTR_ERR(mst_state); - if (!mst_state->pbn_div) { + if (!mst_state->pbn_div.full) { struct nouveau_encoder *outp = mstc->mstm->outp; mst_state->pbn_div = drm_dp_get_vc_payload_bw(&mstm->mgr, diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index a4aad6df71f18..9b19d8bd520af 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -25,6 +25,7 @@ #include <linux/types.h> #include <drm/display/drm_dp_helper.h> #include <drm/drm_atomic.h> +#include <drm/drm_fixed.h> #if IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) #include <linux/stackdepot.h> @@ -617,7 +618,7 @@ struct drm_dp_mst_topology_state { * @pbn_div: The current PBN divisor for this topology. The driver is expected to fill this * out itself. */ - int pbn_div; + fixed20_12 pbn_div; }; #define to_dp_mst_topology_mgr(x) container_of(x, struct drm_dp_mst_topology_mgr, base) @@ -839,8 +840,8 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); -int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, - int link_rate, int link_lane_count); +fixed20_12 drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr, + int link_rate, int link_lane_count); int drm_dp_calc_pbn_mode(int clock, int bpp);
On UHBR links the PBN divider is a fractional number, accordingly store it in fixed point format. For now drm_dp_get_vc_payload_bw() always returns a whole number and all callers will use only the integer part of it which should preserve the current behavior. The next patch will fix drm_dp_get_vc_payload_bw() for UHBR rates returning a fractional number for those (also accounting for the channel coding efficiency correctly). Cc: Lyude Paul <lyude@redhat.com> Cc: Harry Wentland <harry.wentland@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Wayne Lin <wayne.lin@amd.com> Cc: amd-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Imre Deak <imre.deak@intel.com> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++-- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 ++- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 +++-- drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++------ drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +++-- include/drm/display/drm_dp_mst_helper.h | 7 +++--- 7 files changed, 33 insertions(+), 18 deletions(-)