| Message ID | 20240123102850.390126-13-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add Display Port tunnel BW allocation support | expand |
On Tue, Jan 23, 2024 at 12:28:43PM +0200, Imre Deak wrote: > Add the atomic state during a modeset required to enable the DP tunnel > BW allocation mode on links where such a tunnel was detected. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_atomic.c | 8 ++++++++ > drivers/gpu/drm/i915/display/intel_display.c | 19 +++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_link_bw.c | 5 +++++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c > index 96ab37e158995..4236740ede9ed 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > @@ -260,6 +260,10 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) > if (crtc_state->post_csc_lut) > drm_property_blob_get(crtc_state->post_csc_lut); > > + if (crtc_state->dp_tunnel_ref.tunnel) > + drm_dp_tunnel_ref_get(old_crtc_state->dp_tunnel_ref.tunnel, I'd probably s/old_crtc_state/crtc_state/ here. Same pointer, but looks out of place given everyone else just operates on 'crtc_state' here. > + &crtc_state->dp_tunnel_ref); Shame we have to have this ref wrapper. But I guess no clean way to have a magic tracked pointer type that works like a normal pointer in C... > + > crtc_state->update_pipe = false; > crtc_state->update_m_n = false; > crtc_state->update_lrr = false; > @@ -311,6 +315,8 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi); > intel_crtc_free_hw_state(crtc_state); > + if (crtc_state->dp_tunnel_ref.tunnel) > + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); > kfree(crtc_state); > } > > @@ -346,6 +352,8 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) > /* state->internal not reset on purpose */ > > state->dpll_set = state->modeset = false; > + > + intel_dp_tunnel_atomic_cleanup_inherited_state(state); This seems to be in the wrong patch? > } > > struct intel_crtc_state * > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index b9f985a5e705b..46b27a32c8640 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -33,6 +33,7 @@ > #include <linux/string_helpers.h> > > #include <drm/display/drm_dp_helper.h> > +#include <drm/display/drm_dp_tunnel.h> > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_atomic_uapi.h> > @@ -73,6 +74,7 @@ > #include "intel_dp.h" > #include "intel_dp_link_training.h" > #include "intel_dp_mst.h" > +#include "intel_dp_tunnel.h" > #include "intel_dpll.h" > #include "intel_dpll_mgr.h" > #include "intel_dpt.h" > @@ -4490,6 +4492,8 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, > saved_state->crc_enabled = slave_crtc_state->crc_enabled; > > intel_crtc_free_hw_state(slave_crtc_state); > + if (slave_crtc_state->dp_tunnel_ref.tunnel) > + drm_dp_tunnel_ref_put(&slave_crtc_state->dp_tunnel_ref); > memcpy(slave_crtc_state, saved_state, sizeof(*slave_crtc_state)); > kfree(saved_state); > > @@ -4505,6 +4509,10 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, > &master_crtc_state->hw.adjusted_mode); > slave_crtc_state->hw.scaling_filter = master_crtc_state->hw.scaling_filter; > > + if (master_crtc_state->dp_tunnel_ref.tunnel) > + drm_dp_tunnel_ref_get(master_crtc_state->dp_tunnel_ref.tunnel, > + &slave_crtc_state->dp_tunnel_ref); > + > copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); > > slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed; > @@ -4533,6 +4541,13 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state, > /* free the old crtc_state->hw members */ > intel_crtc_free_hw_state(crtc_state); > > + if (crtc_state->dp_tunnel_ref.tunnel) { > + drm_dp_tunnel_atomic_set_stream_bw(&state->base, > + crtc_state->dp_tunnel_ref.tunnel, > + crtc->pipe, 0); > + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); > + } > + > /* FIXME: before the switch to atomic started, a new pipe_config was > * kzalloc'd. Code that depends on any field being zero should be > * fixed, so that the crtc_state can be safely duplicated. For now, > @@ -5374,6 +5389,10 @@ static int intel_modeset_pipe(struct intel_atomic_state *state, > if (ret) > return ret; > > + ret = intel_dp_tunnel_atomic_add_state_for_crtc(state, crtc); > + if (ret) > + return ret; > + > ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c > index 9c6d35a405a18..5b539ba996ddf 100644 > --- a/drivers/gpu/drm/i915/display/intel_link_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c > @@ -8,6 +8,7 @@ > #include "intel_atomic.h" > #include "intel_display_types.h" > #include "intel_dp_mst.h" > +#include "intel_dp_tunnel.h" > #include "intel_fdi.h" > #include "intel_link_bw.h" > > @@ -149,6 +150,10 @@ static int check_all_link_config(struct intel_atomic_state *state, > if (ret) > return ret; > > + ret = intel_dp_tunnel_atomic_check_link(state, limits); > + if (ret) > + return ret; > + > ret = intel_fdi_atomic_check_link(state, limits); > if (ret) > return ret; > -- > 2.39.2
On Mon, Feb 05, 2024 at 06:11:00PM +0200, Ville Syrjälä wrote: > On Tue, Jan 23, 2024 at 12:28:43PM +0200, Imre Deak wrote: > > Add the atomic state during a modeset required to enable the DP tunnel > > BW allocation mode on links where such a tunnel was detected. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_atomic.c | 8 ++++++++ > > drivers/gpu/drm/i915/display/intel_display.c | 19 +++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_link_bw.c | 5 +++++ > > 3 files changed, 32 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c > > index 96ab37e158995..4236740ede9ed 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > > @@ -260,6 +260,10 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) > > if (crtc_state->post_csc_lut) > > drm_property_blob_get(crtc_state->post_csc_lut); > > > > + if (crtc_state->dp_tunnel_ref.tunnel) > > + drm_dp_tunnel_ref_get(old_crtc_state->dp_tunnel_ref.tunnel, > > I'd probably s/old_crtc_state/crtc_state/ here. Same pointer, but > looks out of place given everyone else just operates on 'crtc_state' > here. Ok, will change that. > > + &crtc_state->dp_tunnel_ref); > > Shame we have to have this ref wrapper. But I guess no clean > way to have a magic tracked pointer type that works like a > normal pointer in C... I suppose returning a pointer to a kmalloced drm_dp_tunnel_ref from drm_tunnel_get() and freeing this in drm_tunnel_put() would be one way, but that imo defeats the purpose of the tracker information being valid even after put() (so that ref_tracker can print information about where a particular reference was already dropped). > > > + > > crtc_state->update_pipe = false; > > crtc_state->update_m_n = false; > > crtc_state->update_lrr = false; > > @@ -311,6 +315,8 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, > > > > __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi); > > intel_crtc_free_hw_state(crtc_state); > > + if (crtc_state->dp_tunnel_ref.tunnel) > > + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); > > kfree(crtc_state); > > } > > > > @@ -346,6 +352,8 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) > > /* state->internal not reset on purpose */ > > > > state->dpll_set = state->modeset = false; > > + > > + intel_dp_tunnel_atomic_cleanup_inherited_state(state); > > This seems to be in the wrong patch? Yes, I guess a more logical place is in [PATCH 14/19] drm/i915/dp: Compute DP tunel BW during encoder state computation where the state is added, will move it there. > > > } > > > > struct intel_crtc_state * > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index b9f985a5e705b..46b27a32c8640 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -33,6 +33,7 @@ > > #include <linux/string_helpers.h> > > > > #include <drm/display/drm_dp_helper.h> > > +#include <drm/display/drm_dp_tunnel.h> > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_atomic_uapi.h> > > @@ -73,6 +74,7 @@ > > #include "intel_dp.h" > > #include "intel_dp_link_training.h" > > #include "intel_dp_mst.h" > > +#include "intel_dp_tunnel.h" > > #include "intel_dpll.h" > > #include "intel_dpll_mgr.h" > > #include "intel_dpt.h" > > @@ -4490,6 +4492,8 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, > > saved_state->crc_enabled = slave_crtc_state->crc_enabled; > > > > intel_crtc_free_hw_state(slave_crtc_state); > > + if (slave_crtc_state->dp_tunnel_ref.tunnel) > > + drm_dp_tunnel_ref_put(&slave_crtc_state->dp_tunnel_ref); > > memcpy(slave_crtc_state, saved_state, sizeof(*slave_crtc_state)); > > kfree(saved_state); > > > > @@ -4505,6 +4509,10 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, > > &master_crtc_state->hw.adjusted_mode); > > slave_crtc_state->hw.scaling_filter = master_crtc_state->hw.scaling_filter; > > > > + if (master_crtc_state->dp_tunnel_ref.tunnel) > > + drm_dp_tunnel_ref_get(master_crtc_state->dp_tunnel_ref.tunnel, > > + &slave_crtc_state->dp_tunnel_ref); > > + > > copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); > > > > slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed; > > @@ -4533,6 +4541,13 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state, > > /* free the old crtc_state->hw members */ > > intel_crtc_free_hw_state(crtc_state); > > > > + if (crtc_state->dp_tunnel_ref.tunnel) { > > + drm_dp_tunnel_atomic_set_stream_bw(&state->base, > > + crtc_state->dp_tunnel_ref.tunnel, > > + crtc->pipe, 0); > > + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); > > + } > > + > > /* FIXME: before the switch to atomic started, a new pipe_config was > > * kzalloc'd. Code that depends on any field being zero should be > > * fixed, so that the crtc_state can be safely duplicated. For now, > > @@ -5374,6 +5389,10 @@ static int intel_modeset_pipe(struct intel_atomic_state *state, > > if (ret) > > return ret; > > > > + ret = intel_dp_tunnel_atomic_add_state_for_crtc(state, crtc); > > + if (ret) > > + return ret; > > + > > ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c > > index 9c6d35a405a18..5b539ba996ddf 100644 > > --- a/drivers/gpu/drm/i915/display/intel_link_bw.c > > +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c > > @@ -8,6 +8,7 @@ > > #include "intel_atomic.h" > > #include "intel_display_types.h" > > #include "intel_dp_mst.h" > > +#include "intel_dp_tunnel.h" > > #include "intel_fdi.h" > > #include "intel_link_bw.h" > > > > @@ -149,6 +150,10 @@ static int check_all_link_config(struct intel_atomic_state *state, > > if (ret) > > return ret; > > > > + ret = intel_dp_tunnel_atomic_check_link(state, limits); > > + if (ret) > > + return ret; > > + > > ret = intel_fdi_atomic_check_link(state, limits); > > if (ret) > > return ret; > > -- > > 2.39.2 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c index 96ab37e158995..4236740ede9ed 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic.c +++ b/drivers/gpu/drm/i915/display/intel_atomic.c @@ -260,6 +260,10 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) if (crtc_state->post_csc_lut) drm_property_blob_get(crtc_state->post_csc_lut); + if (crtc_state->dp_tunnel_ref.tunnel) + drm_dp_tunnel_ref_get(old_crtc_state->dp_tunnel_ref.tunnel, + &crtc_state->dp_tunnel_ref); + crtc_state->update_pipe = false; crtc_state->update_m_n = false; crtc_state->update_lrr = false; @@ -311,6 +315,8 @@ intel_crtc_destroy_state(struct drm_crtc *crtc, __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi); intel_crtc_free_hw_state(crtc_state); + if (crtc_state->dp_tunnel_ref.tunnel) + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); kfree(crtc_state); } @@ -346,6 +352,8 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) /* state->internal not reset on purpose */ state->dpll_set = state->modeset = false; + + intel_dp_tunnel_atomic_cleanup_inherited_state(state); } struct intel_crtc_state * diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b9f985a5e705b..46b27a32c8640 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -33,6 +33,7 @@ #include <linux/string_helpers.h> #include <drm/display/drm_dp_helper.h> +#include <drm/display/drm_dp_tunnel.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_uapi.h> @@ -73,6 +74,7 @@ #include "intel_dp.h" #include "intel_dp_link_training.h" #include "intel_dp_mst.h" +#include "intel_dp_tunnel.h" #include "intel_dpll.h" #include "intel_dpll_mgr.h" #include "intel_dpt.h" @@ -4490,6 +4492,8 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, saved_state->crc_enabled = slave_crtc_state->crc_enabled; intel_crtc_free_hw_state(slave_crtc_state); + if (slave_crtc_state->dp_tunnel_ref.tunnel) + drm_dp_tunnel_ref_put(&slave_crtc_state->dp_tunnel_ref); memcpy(slave_crtc_state, saved_state, sizeof(*slave_crtc_state)); kfree(saved_state); @@ -4505,6 +4509,10 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, &master_crtc_state->hw.adjusted_mode); slave_crtc_state->hw.scaling_filter = master_crtc_state->hw.scaling_filter; + if (master_crtc_state->dp_tunnel_ref.tunnel) + drm_dp_tunnel_ref_get(master_crtc_state->dp_tunnel_ref.tunnel, + &slave_crtc_state->dp_tunnel_ref); + copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed; @@ -4533,6 +4541,13 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state, /* free the old crtc_state->hw members */ intel_crtc_free_hw_state(crtc_state); + if (crtc_state->dp_tunnel_ref.tunnel) { + drm_dp_tunnel_atomic_set_stream_bw(&state->base, + crtc_state->dp_tunnel_ref.tunnel, + crtc->pipe, 0); + drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref); + } + /* FIXME: before the switch to atomic started, a new pipe_config was * kzalloc'd. Code that depends on any field being zero should be * fixed, so that the crtc_state can be safely duplicated. For now, @@ -5374,6 +5389,10 @@ static int intel_modeset_pipe(struct intel_atomic_state *state, if (ret) return ret; + ret = intel_dp_tunnel_atomic_add_state_for_crtc(state, crtc); + if (ret) + return ret; + ret = intel_dp_mst_add_topology_state_for_crtc(state, crtc); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c index 9c6d35a405a18..5b539ba996ddf 100644 --- a/drivers/gpu/drm/i915/display/intel_link_bw.c +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c @@ -8,6 +8,7 @@ #include "intel_atomic.h" #include "intel_display_types.h" #include "intel_dp_mst.h" +#include "intel_dp_tunnel.h" #include "intel_fdi.h" #include "intel_link_bw.h" @@ -149,6 +150,10 @@ static int check_all_link_config(struct intel_atomic_state *state, if (ret) return ret; + ret = intel_dp_tunnel_atomic_check_link(state, limits); + if (ret) + return ret; + ret = intel_fdi_atomic_check_link(state, limits); if (ret) return ret;
Add the atomic state during a modeset required to enable the DP tunnel BW allocation mode on links where such a tunnel was detected. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_atomic.c | 8 ++++++++ drivers/gpu/drm/i915/display/intel_display.c | 19 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_link_bw.c | 5 +++++ 3 files changed, 32 insertions(+)