| Message ID | 20250102101552.315814-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/mst: remove mgr parameter and debug logging from drm_dp_get_vc_payload_bw() | expand |
On Thu, 02 Jan 2025, Jani Nikula <jani.nikula@intel.com> wrote: > The struct drm_dp_mst_topology_mgr *mgr parameter is only used for debug > logging in case the passed in link rate or lane count are zero. There's > no further error checking as such, and the function returns 0. > > There should be no case where the parameters are zero. The returned > value is generally used as a divisor, and if we were hitting this, we'd > be seeing division by zero. > > Just remove the debug logging altogether, along with the mgr parameter, > so that the function can be used in non-MST contexts without the > topology manager. > > v2: Also remove drm_dp_mst_helper_tests_init as unnecessary (Imre) > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Lyude Paul <lyude@redhat.com> > Reviewed-by: Imre Deak <imre.deak@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Maarten, Maxime, Thomas, ack for merging this via drm-intel along with the rest of the series? BR, Jani. > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 10 ++-------- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +-- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 +-- > drivers/gpu/drm/tests/drm_dp_mst_helper_test.c | 17 +---------------- > include/drm/display/drm_dp_mst_helper.h | 3 +-- > 5 files changed, 6 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index f8cd094efa3c..06c91c5b7f7c 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -3572,8 +3572,7 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, > } > > /** > - * drm_dp_get_vc_payload_bw - get the VC payload BW for an MST link > - * @mgr: The &drm_dp_mst_topology_mgr to use > + * drm_dp_get_vc_payload_bw - get the VC payload BW for an MTP link > * @link_rate: link rate in 10kbits/s units > * @link_lane_count: lane count > * > @@ -3584,17 +3583,12 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, > * > * Returns the BW / timeslot value in 20.12 fixed point format. > */ > -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 drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count) > { > int ch_coding_efficiency = > drm_dp_bw_channel_coding_efficiency(drm_dp_is_uhbr_rate(link_rate)); > 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, 2.7.6.3 VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ > ret.full = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_rate * link_lane_count, > ch_coding_efficiency), > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index fffd199999e0..ca091ed291d5 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -244,8 +244,7 @@ static int mst_stream_find_vcpi_slots_for_bpp(struct intel_dp *intel_dp, > crtc_state->fec_enable = !intel_dp_is_uhbr(crtc_state); > } > > - mst_state->pbn_div = drm_dp_get_vc_payload_bw(&intel_dp->mst_mgr, > - crtc_state->port_clock, > + mst_state->pbn_div = drm_dp_get_vc_payload_bw(crtc_state->port_clock, > crtc_state->lane_count); > > max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 8097249612bc..62d72b7a8d04 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -992,8 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > 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, > - outp->dp.link_bw, outp->dp.link_nr); > + mst_state->pbn_div = drm_dp_get_vc_payload_bw(outp->dp.link_bw, outp->dp.link_nr); > } > > slots = drm_dp_atomic_find_time_slots(state, &mstm->mgr, mstc->port, asyh->dp.pbn); > diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c > index 89cd9e4f4d32..9e0e2fb65944 100644 > --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c > +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c > @@ -199,10 +199,8 @@ static const struct drm_dp_mst_calc_pbn_div_test drm_dp_mst_calc_pbn_div_dp1_4_c > static void drm_test_dp_mst_calc_pbn_div(struct kunit *test) > { > const struct drm_dp_mst_calc_pbn_div_test *params = test->param_value; > - /* mgr->dev is only needed by drm_dbg_kms(), but it's not called for the test cases. */ > - struct drm_dp_mst_topology_mgr *mgr = test->priv; > > - KUNIT_EXPECT_EQ(test, drm_dp_get_vc_payload_bw(mgr, params->link_rate, params->lane_count).full, > + KUNIT_EXPECT_EQ(test, drm_dp_get_vc_payload_bw(params->link_rate, params->lane_count).full, > params->expected.full); > } > > @@ -568,21 +566,8 @@ static struct kunit_case drm_dp_mst_helper_tests[] = { > { } > }; > > -static int drm_dp_mst_helper_tests_init(struct kunit *test) > -{ > - struct drm_dp_mst_topology_mgr *mgr; > - > - mgr = kunit_kzalloc(test, sizeof(*mgr), GFP_KERNEL); > - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mgr); > - > - test->priv = mgr; > - > - return 0; > -} > - > static struct kunit_suite drm_dp_mst_helper_test_suite = { > .name = "drm_dp_mst_helper", > - .init = drm_dp_mst_helper_tests_init, > .test_cases = drm_dp_mst_helper_tests, > }; > > diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h > index a80ba457a858..e39de161c938 100644 > --- a/include/drm/display/drm_dp_mst_helper.h > +++ b/include/drm/display/drm_dp_mst_helper.h > @@ -867,8 +867,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, > struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port); > > -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 drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count); > > int drm_dp_calc_pbn_mode(int clock, int bpp);
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index f8cd094efa3c..06c91c5b7f7c 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3572,8 +3572,7 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, } /** - * drm_dp_get_vc_payload_bw - get the VC payload BW for an MST link - * @mgr: The &drm_dp_mst_topology_mgr to use + * drm_dp_get_vc_payload_bw - get the VC payload BW for an MTP link * @link_rate: link rate in 10kbits/s units * @link_lane_count: lane count * @@ -3584,17 +3583,12 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, * * Returns the BW / timeslot value in 20.12 fixed point format. */ -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 drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count) { int ch_coding_efficiency = drm_dp_bw_channel_coding_efficiency(drm_dp_is_uhbr_rate(link_rate)); 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, 2.7.6.3 VCPayload_Bandwidth_for_OneTimeSlotPer_MTP_Allocation */ ret.full = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_rate * link_lane_count, ch_coding_efficiency), diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index fffd199999e0..ca091ed291d5 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -244,8 +244,7 @@ static int mst_stream_find_vcpi_slots_for_bpp(struct intel_dp *intel_dp, crtc_state->fec_enable = !intel_dp_is_uhbr(crtc_state); } - mst_state->pbn_div = drm_dp_get_vc_payload_bw(&intel_dp->mst_mgr, - crtc_state->port_clock, + mst_state->pbn_div = drm_dp_get_vc_payload_bw(crtc_state->port_clock, crtc_state->lane_count); max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 8097249612bc..62d72b7a8d04 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -992,8 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, 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, - outp->dp.link_bw, outp->dp.link_nr); + mst_state->pbn_div = drm_dp_get_vc_payload_bw(outp->dp.link_bw, outp->dp.link_nr); } slots = drm_dp_atomic_find_time_slots(state, &mstm->mgr, mstc->port, asyh->dp.pbn); diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c index 89cd9e4f4d32..9e0e2fb65944 100644 --- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c +++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c @@ -199,10 +199,8 @@ static const struct drm_dp_mst_calc_pbn_div_test drm_dp_mst_calc_pbn_div_dp1_4_c static void drm_test_dp_mst_calc_pbn_div(struct kunit *test) { const struct drm_dp_mst_calc_pbn_div_test *params = test->param_value; - /* mgr->dev is only needed by drm_dbg_kms(), but it's not called for the test cases. */ - struct drm_dp_mst_topology_mgr *mgr = test->priv; - KUNIT_EXPECT_EQ(test, drm_dp_get_vc_payload_bw(mgr, params->link_rate, params->lane_count).full, + KUNIT_EXPECT_EQ(test, drm_dp_get_vc_payload_bw(params->link_rate, params->lane_count).full, params->expected.full); } @@ -568,21 +566,8 @@ static struct kunit_case drm_dp_mst_helper_tests[] = { { } }; -static int drm_dp_mst_helper_tests_init(struct kunit *test) -{ - struct drm_dp_mst_topology_mgr *mgr; - - mgr = kunit_kzalloc(test, sizeof(*mgr), GFP_KERNEL); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mgr); - - test->priv = mgr; - - return 0; -} - static struct kunit_suite drm_dp_mst_helper_test_suite = { .name = "drm_dp_mst_helper", - .init = drm_dp_mst_helper_tests_init, .test_cases = drm_dp_mst_helper_tests, }; diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h index a80ba457a858..e39de161c938 100644 --- a/include/drm/display/drm_dp_mst_helper.h +++ b/include/drm/display/drm_dp_mst_helper.h @@ -867,8 +867,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); -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 drm_dp_get_vc_payload_bw(int link_rate, int link_lane_count); int drm_dp_calc_pbn_mode(int clock, int bpp);