| Message ID | 80f8682fbc304ccf673af20abe562ce5aedb9219.1731011435.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/mst: cleanups, renames, clarifications | expand |
On Thu, 07 Nov 2024, Jani Nikula <jani.nikula@intel.com> wrote: > It would be best to have self-explanatory code, but lacking that, add > some comments about the way the DDI encoder hooks get called from DP MST > stream encoders. The subject prefix should be "drm/i915/ddi:". > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 51 ++++++++++++++++++------ > 1 file changed, 38 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index c973b70185cf..9965d7603d6c 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2920,6 +2920,24 @@ static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state, > crtc_state, conn_state); > } > > +/* > + * Note: Also called from the ->pre_enable of the first active MST stream > + * encoder on its primary encoder. > + * > + * When called from DP MST code: > + * > + * - conn_state will be NULL > + * > + * - encoder will be the primary encoder (i.e. mst->primary) > + * > + * - the main connector associated with this port won't be active or linked to a > + * crtc > + * > + * - crtc_state will be the state of the first stream to be activated on this > + * port, and it may not be the same stream that will be deactivated last, but > + * each stream should have a state that is identical when it comes to the DP > + * link parameteres > + */ > static void intel_ddi_pre_enable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > @@ -2929,19 +2947,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum pipe pipe = crtc->pipe; > > - /* > - * When called from DP MST code: > - * - conn_state will be NULL > - * - encoder will be the main encoder (ie. mst->primary) > - * - the main connector associated with this port > - * won't be active or linked to a crtc > - * - crtc_state will be the state of the first stream to > - * be activated on this port, and it may not be the same > - * stream that will be deactivated last, but each stream > - * should have a state that is identical when it comes to > - * the DP link parameteres > - */ > - > drm_WARN_ON(&dev_priv->drm, crtc_state->has_pch_encoder); > > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); > @@ -3191,6 +3196,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, > } > } > > +/* > + * Note: Also called from the ->post_disable of the last active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void intel_ddi_post_disable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > @@ -3221,6 +3231,11 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, > old_conn_state); > } > > +/* > + * Note: Also called from the ->post_pll_disable of the last active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void intel_ddi_post_pll_disable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > @@ -3558,6 +3573,11 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state, > intel_update_active_dpll(state, pipe_crtc, encoder); > } > > +/* > + * Note: Also called from the ->pre_pll_enable of the first active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void > intel_ddi_pre_pll_enable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > @@ -3996,6 +4016,11 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder, > } > } > > +/* > + * Note: Also called from the ->get_config of the MST stream encoders on their > + * primary encoder, via the platform specific hooks here. See also the comment > + * for intel_ddi_pre_enable(). > + */ > static void intel_ddi_get_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config) > {
On Thu, Nov 07, 2024 at 10:32:21PM +0200, Jani Nikula wrote: > It would be best to have self-explanatory code, but lacking that, add > some comments about the way the DDI encoder hooks get called from DP MST > stream encoders. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 51 ++++++++++++++++++------ > 1 file changed, 38 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index c973b70185cf..9965d7603d6c 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2920,6 +2920,24 @@ static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state, > crtc_state, conn_state); > } > > +/* > + * Note: Also called from the ->pre_enable of the first active MST stream > + * encoder on its primary encoder. > + * > + * When called from DP MST code: > + * > + * - conn_state will be NULL > + * > + * - encoder will be the primary encoder (i.e. mst->primary) > + * > + * - the main connector associated with this port won't be active or linked to a > + * crtc > + * > + * - crtc_state will be the state of the first stream to be activated on this > + * port, and it may not be the same stream that will be deactivated last, but > + * each stream should have a state that is identical when it comes to the DP > + * link parameteres > + */ > static void intel_ddi_pre_enable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state, > @@ -2929,19 +2947,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum pipe pipe = crtc->pipe; > > - /* > - * When called from DP MST code: > - * - conn_state will be NULL > - * - encoder will be the main encoder (ie. mst->primary) > - * - the main connector associated with this port > - * won't be active or linked to a crtc > - * - crtc_state will be the state of the first stream to > - * be activated on this port, and it may not be the same > - * stream that will be deactivated last, but each stream > - * should have a state that is identical when it comes to > - * the DP link parameteres > - */ > - > drm_WARN_ON(&dev_priv->drm, crtc_state->has_pch_encoder); > > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); > @@ -3191,6 +3196,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, > } > } > > +/* > + * Note: Also called from the ->post_disable of the last active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void intel_ddi_post_disable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > @@ -3221,6 +3231,11 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, > old_conn_state); > } > > +/* > + * Note: Also called from the ->post_pll_disable of the last active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void intel_ddi_post_pll_disable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > @@ -3558,6 +3573,11 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state, > intel_update_active_dpll(state, pipe_crtc, encoder); > } > > +/* > + * Note: Also called from the ->pre_pll_enable of the first active MST stream > + * encoder on its primary encoder. See also the comment for > + * intel_ddi_pre_enable(). > + */ > static void > intel_ddi_pre_pll_enable(struct intel_atomic_state *state, > struct intel_encoder *encoder, > @@ -3996,6 +4016,11 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder, > } > } > > +/* > + * Note: Also called from the ->get_config of the MST stream encoders on their > + * primary encoder, via the platform specific hooks here. See also the comment > + * for intel_ddi_pre_enable(). > + */ > static void intel_ddi_get_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config) > { > -- > 2.39.5 >
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index c973b70185cf..9965d7603d6c 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -2920,6 +2920,24 @@ static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state, crtc_state, conn_state); } +/* + * Note: Also called from the ->pre_enable of the first active MST stream + * encoder on its primary encoder. + * + * When called from DP MST code: + * + * - conn_state will be NULL + * + * - encoder will be the primary encoder (i.e. mst->primary) + * + * - the main connector associated with this port won't be active or linked to a + * crtc + * + * - crtc_state will be the state of the first stream to be activated on this + * port, and it may not be the same stream that will be deactivated last, but + * each stream should have a state that is identical when it comes to the DP + * link parameteres + */ static void intel_ddi_pre_enable(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, @@ -2929,19 +2947,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum pipe pipe = crtc->pipe; - /* - * When called from DP MST code: - * - conn_state will be NULL - * - encoder will be the main encoder (ie. mst->primary) - * - the main connector associated with this port - * won't be active or linked to a crtc - * - crtc_state will be the state of the first stream to - * be activated on this port, and it may not be the same - * stream that will be deactivated last, but each stream - * should have a state that is identical when it comes to - * the DP link parameteres - */ - drm_WARN_ON(&dev_priv->drm, crtc_state->has_pch_encoder); intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); @@ -3191,6 +3196,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, } } +/* + * Note: Also called from the ->post_disable of the last active MST stream + * encoder on its primary encoder. See also the comment for + * intel_ddi_pre_enable(). + */ static void intel_ddi_post_disable(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *old_crtc_state, @@ -3221,6 +3231,11 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state, old_conn_state); } +/* + * Note: Also called from the ->post_pll_disable of the last active MST stream + * encoder on its primary encoder. See also the comment for + * intel_ddi_pre_enable(). + */ static void intel_ddi_post_pll_disable(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *old_crtc_state, @@ -3558,6 +3573,11 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state, intel_update_active_dpll(state, pipe_crtc, encoder); } +/* + * Note: Also called from the ->pre_pll_enable of the first active MST stream + * encoder on its primary encoder. See also the comment for + * intel_ddi_pre_enable(). + */ static void intel_ddi_pre_pll_enable(struct intel_atomic_state *state, struct intel_encoder *encoder, @@ -3996,6 +4016,11 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder, } } +/* + * Note: Also called from the ->get_config of the MST stream encoders on their + * primary encoder, via the platform specific hooks here. See also the comment + * for intel_ddi_pre_enable(). + */ static void intel_ddi_get_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config) {
It would be best to have self-explanatory code, but lacking that, add some comments about the way the DDI encoder hooks get called from DP MST stream encoders. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 51 ++++++++++++++++++------ 1 file changed, 38 insertions(+), 13 deletions(-)