| Message ID | 20231113201110.510724-1-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/dp: Account for channel coding efficiency on UHBR links | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre > Deak > Sent: Tuesday, November 14, 2023 1:41 AM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH 1/4] drm/i915/dp: Account for channel coding > efficiency on UHBR links > > Apply the correct BW allocation overhead and channel coding efficiency on > UHBR link rates, similarly to DP1.4 link rates. > > Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com> Thanks and Regards, Arun R Murthy -------------------- > --- > drivers/gpu/drm/i915/display/intel_display.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 3effafcbb411a..24aebdb715e7d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2398,16 +2398,6 @@ add_bw_alloc_overhead(int link_clock, int > bw_overhead, > int ch_coding_efficiency = > drm_dp_bw_channel_coding_efficiency(is_uhbr); > > - /* > - * TODO: adjust for actual UHBR channel coding efficiency and BW > - * overhead. > - */ > - if (is_uhbr) { > - *data_m = pixel_data_rate; > - *data_n = link_data_rate * 8 / 10; > - return; > - } > - > *data_m = DIV_ROUND_UP_ULL(mul_u32_u32(pixel_data_rate, > bw_overhead), > 1000000); > *data_n = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_data_rate, > ch_coding_efficiency), > -- > 2.39.2
On Mon, 13 Nov 2023, Imre Deak <imre.deak@intel.com> wrote: > Apply the correct BW allocation overhead and channel coding efficiency > on UHBR link rates, similarly to DP1.4 link rates. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 3effafcbb411a..24aebdb715e7d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2398,16 +2398,6 @@ add_bw_alloc_overhead(int link_clock, int bw_overhead, > int ch_coding_efficiency = > drm_dp_bw_channel_coding_efficiency(is_uhbr); Why do we have this and intel_dp_max_data_rate() separately? BR, Jani. > > - /* > - * TODO: adjust for actual UHBR channel coding efficiency and BW > - * overhead. > - */ > - if (is_uhbr) { > - *data_m = pixel_data_rate; > - *data_n = link_data_rate * 8 / 10; > - return; > - } > - > *data_m = DIV_ROUND_UP_ULL(mul_u32_u32(pixel_data_rate, bw_overhead), > 1000000); > *data_n = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_data_rate, ch_coding_efficiency),
On Tue, Nov 14, 2023 at 11:00:49AM +0200, Jani Nikula wrote: > On Mon, 13 Nov 2023, Imre Deak <imre.deak@intel.com> wrote: > > Apply the correct BW allocation overhead and channel coding efficiency > > on UHBR link rates, similarly to DP1.4 link rates. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 10 ---------- > > 1 file changed, 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 3effafcbb411a..24aebdb715e7d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -2398,16 +2398,6 @@ add_bw_alloc_overhead(int link_clock, int bw_overhead, > > int ch_coding_efficiency = > > drm_dp_bw_channel_coding_efficiency(is_uhbr); > > Why do we have this and intel_dp_max_data_rate() separately? This function calculates an m/n ratio for a given pixel/data rate, applying both the allocation overhead (FEC, SSC, etc.) and the channel coding efficiency. intel_dp_max_data_rate() calculates a maximum data rate applying only the channel coding efficiency. I think intel_dp_max_data_rate() could be simplified, so the two functions use the same channel coding efficiency to: DIV_ROUND_UP_ULL(mul_u32_u32(max_link_rate_kbps * max_lanes, drm_dp_bw_channel_coding_efficiency(is_uhbr)), 1000000ULL * 8) --Imre > > BR, > Jani. > > > > > > - /* > > - * TODO: adjust for actual UHBR channel coding efficiency and BW > > - * overhead. > > - */ > > - if (is_uhbr) { > > - *data_m = pixel_data_rate; > > - *data_n = link_data_rate * 8 / 10; > > - return; > > - } > > - > > *data_m = DIV_ROUND_UP_ULL(mul_u32_u32(pixel_data_rate, bw_overhead), > > 1000000); > > *data_n = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_data_rate, ch_coding_efficiency), > > -- > Jani Nikula, Intel
On Tue, Nov 14, 2023 at 03:07:52PM +0200, Imre Deak wrote: > On Tue, Nov 14, 2023 at 11:00:49AM +0200, Jani Nikula wrote: > > On Mon, 13 Nov 2023, Imre Deak <imre.deak@intel.com> wrote: > > > Apply the correct BW allocation overhead and channel coding efficiency > > > on UHBR link rates, similarly to DP1.4 link rates. > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 10 ---------- > > > 1 file changed, 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index 3effafcbb411a..24aebdb715e7d 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -2398,16 +2398,6 @@ add_bw_alloc_overhead(int link_clock, int bw_overhead, > > > int ch_coding_efficiency = > > > drm_dp_bw_channel_coding_efficiency(is_uhbr); > > > > Why do we have this and intel_dp_max_data_rate() separately? > > This function calculates an m/n ratio for a given pixel/data rate, > applying both the allocation overhead (FEC, SSC, etc.) and the channel > coding efficiency. intel_dp_max_data_rate() calculates a maximum data > rate applying only the channel coding efficiency. > > I think intel_dp_max_data_rate() could be simplified, so the two > functions use the same channel coding efficiency to: > > DIV_ROUND_UP_ULL(mul_u32_u32(max_link_rate_kbps * max_lanes, > drm_dp_bw_channel_coding_efficiency(is_uhbr)), > 1000000ULL * 8) Actually, it does make sense to reuse intel_dp_max_data_rate() in intel_link_compute_m_n() -> add_bw_alloc_overhead(), I'll send a new version with that (and the above simplification). > > --Imre > > > > > BR, > > Jani. > > > > > > > > > > - /* > > > - * TODO: adjust for actual UHBR channel coding efficiency and BW > > > - * overhead. > > > - */ > > > - if (is_uhbr) { > > > - *data_m = pixel_data_rate; > > > - *data_n = link_data_rate * 8 / 10; > > > - return; > > > - } > > > - > > > *data_m = DIV_ROUND_UP_ULL(mul_u32_u32(pixel_data_rate, bw_overhead), > > > 1000000); > > > *data_n = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_data_rate, ch_coding_efficiency), > > > > -- > > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 3effafcbb411a..24aebdb715e7d 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2398,16 +2398,6 @@ add_bw_alloc_overhead(int link_clock, int bw_overhead, int ch_coding_efficiency = drm_dp_bw_channel_coding_efficiency(is_uhbr); - /* - * TODO: adjust for actual UHBR channel coding efficiency and BW - * overhead. - */ - if (is_uhbr) { - *data_m = pixel_data_rate; - *data_n = link_data_rate * 8 / 10; - return; - } - *data_m = DIV_ROUND_UP_ULL(mul_u32_u32(pixel_data_rate, bw_overhead), 1000000); *data_n = DIV_ROUND_DOWN_ULL(mul_u32_u32(link_data_rate, ch_coding_efficiency),
Apply the correct BW allocation overhead and channel coding efficiency on UHBR link rates, similarly to DP1.4 link rates. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 10 ---------- 1 file changed, 10 deletions(-)