Re: [PATCH 7/8] drm/i915/mst: change naming from fake encoders to MST stream encoders

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 11 Nov 2024, Imre Deak <imre.deak@intel.com> wrote:
> On Mon, Nov 11, 2024 at 06:58:32PM +0200, Imre Deak wrote:
>> On Thu, Nov 07, 2024 at 10:32:20PM +0200, Jani Nikula wrote:
>> > The fake encoders pretty much match individual MST streams. The encoders
>> > remain as fake as ever, but change the naming to MST stream
>> > encoders. Rename all the encoder hooks and related functions called from
>> > them to mst_stream_* to clarify what type of encoders the hooks are
>> > called on.
>> > 
>> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> Reviewed-by: Imre Deak <imre.deak@intel.com>
>> 
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 204 ++++++++++----------
>> >  1 file changed, 100 insertions(+), 104 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > index 481510819cc6..0c08212e70ac 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > @@ -53,7 +53,7 @@
>> >  #include "intel_vdsc.h"
>> >  #include "skl_scaler.h"
>> >  
>> > -/* From fake MST encoder to primary encoder */
>> > +/* From fake MST stream encoder to primary encoder */
>> >  static struct intel_encoder *to_primary_encoder(struct intel_encoder *encoder)
>> >  {
>> >  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
>> > @@ -62,7 +62,7 @@ static struct intel_encoder *to_primary_encoder(struct intel_encoder *encoder)
>> >  	return &dig_port->base;
>> >  }
>> >  
>> > -/* From fake MST encoder to primary DP */
>> > +/* From fake MST stream encoder to primary DP */
>> >  static struct intel_dp *to_primary_dp(struct intel_encoder *encoder)
>> >  {
>> >  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
>> > @@ -179,14 +179,12 @@ static int intel_dp_mst_dsc_get_slice_count(const struct intel_connector *connec
>> >  					    num_joined_pipes);
>> >  }
>> >  
>> > -static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>> > -						struct intel_crtc_state *crtc_state,
>> > -						int max_bpp,
>> > -						int min_bpp,
>> > -						struct link_config_limits *limits,
>> > -						struct drm_connector_state *conn_state,
>> > -						int step,
>> > -						bool dsc)
>> > +static int mst_stream_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>> > +					      struct intel_crtc_state *crtc_state,
>> > +					      int max_bpp, int min_bpp,
>> > +					      struct link_config_limits *limits,
>> > +					      struct drm_connector_state *conn_state,
>> > +					      int step, bool dsc)
>> >  {
>> >  	struct intel_display *display = to_intel_display(encoder);
>> >  	struct drm_atomic_state *state = crtc_state->uapi.state;
>> > @@ -333,10 +331,10 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
>> >  	return slots;
>> >  }
>> >  
>> > -static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>> > -					    struct intel_crtc_state *crtc_state,
>> > -					    struct drm_connector_state *conn_state,
>> > -					    struct link_config_limits *limits)
>> > +static int mst_stream_compute_link_config(struct intel_encoder *encoder,
>> > +					  struct intel_crtc_state *crtc_state,
>> > +					  struct drm_connector_state *conn_state,
>> > +					  struct link_config_limits *limits)
>> >  {
>> >  	int slots = -EINVAL;
>> >  
>> > @@ -344,11 +342,11 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>> >  	 * FIXME: allocate the BW according to link_bpp, which in the case of
>> >  	 * YUV420 is only half of the pipe bpp value.
>> >  	 */
>> > -	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state,
>> > -						     fxp_q4_to_int(limits->link.max_bpp_x16),
>> > -						     fxp_q4_to_int(limits->link.min_bpp_x16),
>> > -						     limits,
>> > -						     conn_state, 2 * 3, false);
>> > +	slots = mst_stream_find_vcpi_slots_for_bpp(encoder, crtc_state,
>> > +						   fxp_q4_to_int(limits->link.max_bpp_x16),
>> > +						   fxp_q4_to_int(limits->link.min_bpp_x16),
>> > +						   limits,
>> > +						   conn_state, 2 * 3, false);
>> >  
>> >  	if (slots < 0)
>> >  		return slots;
>> > @@ -356,10 +354,10 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
>> >  	return 0;
>> >  }
>> >  
>> > -static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
>> > -						struct intel_crtc_state *crtc_state,
>> > -						struct drm_connector_state *conn_state,
>> > -						struct link_config_limits *limits)
>> > +static int mst_stream_dsc_compute_link_config(struct intel_encoder *encoder,
>> > +					      struct intel_crtc_state *crtc_state,
>> > +					      struct drm_connector_state *conn_state,
>> > +					      struct link_config_limits *limits)
>> >  {
>> >  	struct intel_display *display = to_intel_display(encoder);
>> >  	struct intel_connector *connector = to_intel_connector(conn_state->connector);
>> > @@ -426,18 +424,19 @@ static int intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder,
>> >  	min_compressed_bpp = intel_dp_dsc_nearest_valid_bpp(i915, min_compressed_bpp,
>> >  							    crtc_state->pipe_bpp);
>> >  
>> > -	slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, max_compressed_bpp,
>> > -						     min_compressed_bpp, limits,
>> > -						     conn_state, 1, true);
>> > +	slots = mst_stream_find_vcpi_slots_for_bpp(encoder, crtc_state, max_compressed_bpp,
>> > +						   min_compressed_bpp, limits,
>> > +						   conn_state, 1, true);
>> >  
>> >  	if (slots < 0)
>> >  		return slots;
>> >  
>> >  	return 0;
>> >  }
>> > -static int intel_dp_mst_update_slots(struct intel_encoder *encoder,
>> > -				     struct intel_crtc_state *crtc_state,
>> > -				     struct drm_connector_state *conn_state)
>> > +
>> > +static int mst_stream_update_slots(struct intel_encoder *encoder,
>> > +				   struct intel_crtc_state *crtc_state,
>> > +				   struct drm_connector_state *conn_state)
>
> Side note: an additional thing that could be done is to pass intel_dp
> instead of intel_encoder to all the above functions, since it's clear
> what the former is (no separate stream vs. primary instances) and
> intel_encoder is used anyway only to get to intel_dp.

Right. Maybe I'll send a follow-up once these have been merged.

BR,
Jani.


-- 
Jani Nikula, Intel