On Mon, Aug 29, 2022 at 05:43:19PM +0300, Govindapillai, Vinod wrote:
> Hi Stan,
>
> I wonder if it is better if you reorder the 3 and 4 patches in this - move
> this 4/4 before the 3rd
> one and modify the 3rd one accordingly.
Was thiking about that, but decided to first introduce a new function, using
same code, so that
we don't mix introduction of the new functionality with code optimization, also
it then becomes
obvious why we need to remove that duplicate code.
But.. may be you are right - I could first extract that function and introduce
new DSC functionality
using it.
>
> Also, instead of getting rid of limits, keep limits and populate the limits
> according to dsc or
> normal dp_mst. What do you think?
Yeah, was wondering if someone asks this, problem is that in non DSC case
limits structure contains
exactly min, max bpps which we need, so can be passed on nicely, however in DSC
case those are not the same:
max_bpp = min_t(u8, dsc_max_bpc * 3, limits->max_bpp);
min_bpp = limits->min_bpp;
So we would need to create some other limits struct, which we will populate
with those(I guess
we shouldn't replace the ones, which were calculated for non-DSC case in
current limits), so I didn't
see much benefit in using it as an argument, if we can't pass it rightaway in
both cases.
Stan
>
> BR
> vinod
>
>
> On Mon, 2022-08-29 at 12:58 +0300, Stanislav Lisovskiy wrote:
> > We are using almost same code to loop through bpps while calling
> > drm_dp_atomic_find_vcpi_slots - lets remove this duplication by
> > introducing a new function intel_dp_mst_find_vcpi_slots_for_bpp
> >
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 88 +++--
> > 1 file changed, 46 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 94d7e7f84c51..2a524816dbfd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -44,10 +44,14 @@
> > #include "intel_hotplug.h"
> > #include "skl_scaler.h"
> >
> > -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 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)
> > {
> > struct drm_atomic_state *state = crtc_state->uapi.state;
> > struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> > @@ -58,7 +62,6 @@ static int intel_dp_mst_compute_link_config(struct
> > intel_encoder *encoder,
> > struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > const struct drm_display_mode *adjusted_mode =
> > _state->hw.adjusted_mode;
> > - bool constant_n = drm_dp_has_quirk(_dp->desc,
> > DP_DPCD_QUIRK_CONSTANT_N);
> > int bpp, slots = -EINVAL;
> > int ret = 0;
> >
> > @@ -71,19 +74,21 @@ static int intel_dp_mst_compute_link_config(struct
> > intel_encoder *encoder,
> >
> > // TODO: Handle pbn_div changes by adding a new MST helper
> > if (!mst_state->pbn_div) {
> > - mst_state->pbn_div =
> > drm_dp_get_vc_payload_bw(_dp->mst_mgr,
> > -
> > limits->max_rate,
> > -
> > limits->max_lane_count);
> > + mst_state->pbn_div = !dsc ?
> > drm_dp_get_vc_payload_bw(_dp->mst_mgr,
> > +
> > crtc_state->port_clock,
> > +
> > crtc_state->lane_count) : 0;
> > }
> >
> > - for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> > + for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
> > crtc_state->pipe_bpp = bpp;
> >
> > crtc_state->pbn =
> > drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
> > - crtc_state->pipe_bpp,
> > - false);
> > +
