Re: [PATCH 02/19] drm/dp: Add support for DP tunneling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 07, 2024 at 11:02:27PM +0200, Imre Deak wrote:
> 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?

I suppose there isn't any real harm in doing it here too.

> 
> > > > +			*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;
> > > > +}
> > > > +

-- 
Ville Syrjälä
Intel