On Wed, Feb 07, 2024 at 10:48:53PM +0200, Imre Deak wrote:
> On Wed, Feb 07, 2024 at 10:02:18PM +0200, Ville Syrjälä wrote:
> > > [...]
> > > +static int
> > > +drm_dp_tunnel_atomic_check_group_bw(struct drm_dp_tunnel_group_state *new_group_state,
> > > + u32 *failed_stream_mask)
> > > +{
> > > + struct drm_dp_tunnel_group *group = to_group(new_group_state->base.obj);
> > > + struct drm_dp_tunnel_state *new_tunnel_state;
> > > + u32 group_stream_mask = 0;
> > > + int group_bw = 0;
> > > +
> > > + for_each_tunnel_state(new_group_state, new_tunnel_state) {
> > > + struct drm_dp_tunnel *tunnel = new_tunnel_state->tunnel_ref.tunnel;
> > > + int max_dprx_bw = get_max_dprx_bw(tunnel);
> > > + int tunnel_bw = drm_dp_tunnel_atomic_get_tunnel_bw(new_tunnel_state);
> > > +
> > > + tun_dbg(tunnel,
> > > + "%sRequired %d/%d Mb/s total for tunnel.\n",
> > > + tunnel_bw > max_dprx_bw ? "Not enough BW: " : "",
> > > + DPTUN_BW_ARG(tunnel_bw),
> > > + DPTUN_BW_ARG(max_dprx_bw));
> > > +
> > > + if (tunnel_bw > max_dprx_bw) {
> >
> > I'm a bit confused why we're checking this here. Aren't we already
> > checking this somewhere else?
>
> Ah, yes this should be checked already by the encoder compute config +
> the MST link BW check. It can be removed, thanks.
Though neither of that is guaranteed for drivers in general, so
shouldn't it be here still?
> > > + *failed_stream_mask = new_tunnel_state->stream_mask;
> > > + return -ENOSPC;
> > > + }
> > > +
> > > + group_bw += min(roundup(tunnel_bw, tunnel->bw_granularity),
> > > + max_dprx_bw);
> > > + group_stream_mask |= new_tunnel_state->stream_mask;
> > > + }
> > > +
> > > + tun_grp_dbg(group,
> > > + "%sRequired %d/%d Mb/s total for tunnel group.\n",
> > > + group_bw > group->available_bw ? "Not enough BW: " : "",
> > > + DPTUN_BW_ARG(group_bw),
> > > + DPTUN_BW_ARG(group->available_bw));
> > > +
> > > + if (group_bw > group->available_bw) {
> > > + *failed_stream_mask = group_stream_mask;
> > > + return -ENOSPC;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
