> -----Original Message----- > From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Sent: Tuesday, July 30, 2024 1:27 PM > To: Garg, Nemesa <nemesa.garg@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Garg, Nemesa <nemesa.garg@xxxxxxxxx> > Subject: Re: [PATCH 2/2] drm/i915/display: Call panel_fitting from pipe_config > > On Fri, 26 Jul 2024, Nemesa Garg <nemesa.garg@xxxxxxxxx> wrote: > > In panel fitter/pipe scaler scenario the pch_pfit configuration > > currently takes place before accounting for pipe_src width for joiner. > > This causes issue when pch_pfit and joiner gets enabled together. > > > > Introduce a new boolean flag is_required which can be filled during dp > > compute_config and later is used to compute panel_fitting in > > pipe_config. Modify pch_panel_fitting to handle joiner pipes by > > adjusting crtc_hdisplay accordingly. > > Superficially looks simple enough, but this actually adds a bunch of complexity. > It's not obvious why encoders handle panel fitting in completely different places, > or what "pch_pfit.is_required" really means. > > BR, > Jani. > Hi Jani, In joiner cases we need to do panel fitting calculations after the calculated pipe src width. So one solution I thought of is as mentioned below: For each encoder first panel_compute_config is called and then intel_panel_fitting. So to simplify this I am replacing the intel_panel_fitting with the flag and then calling it at one place in pipe_config. As done in trybot patch https://patchwork.freedesktop.org/series/136483/. Other solution can be let the flow for panel_fitting is same for non-joiner case for edp/dp and if joiner is enabled call panel_fitting again in later stage ie after pipe_config. Please suggest what can be the better solution. Thanks and Regards, Nemesa > > > > Signed-off-by: Nemesa Garg <nemesa.garg@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++++++ > > drivers/gpu/drm/i915/display/intel_display_types.h | 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++----- > > drivers/gpu/drm/i915/display/intel_panel.c | 3 +++ > > 4 files changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 8bbde03f2508..a7194a84b6b8 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -4796,6 +4796,17 @@ intel_modeset_pipe_config(struct > intel_atomic_state *state, > > return ret; > > } > > > > + for_each_new_connector_in_state(&state->base, connector, > connector_state, i) { > > + if (connector_state->crtc != &crtc->base) > > + continue; > > + > > + if (crtc_state->pch_pfit.is_required) { > > + ret = intel_panel_fitting(crtc_state, connector_state); > > + if (ret) > > + return ret; > > + } > > + } > > + > > /* Dithering seems to not pass-through bits correctly when it should, so > > * only enable it on 6bpc panels and when its not a compliance > > * test requesting 6bpc video pattern. > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index a04d52dbf6e1..a4ab26d8fa43 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1258,6 +1258,7 @@ struct intel_crtc_state { > > struct drm_rect dst; > > bool enabled; > > bool force_thru; > > + bool is_required; > > } pch_pfit; > > > > /* FDI configuration, only valid if has_pch_encoder is set. */ diff > > --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index d0d878da71ee..d892c5a429f1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -2922,11 +2922,8 @@ intel_dp_compute_config(struct intel_encoder > *encoder, > > pipe_config->has_pch_encoder = true; > > > > fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode); > > - if (intel_dp_is_edp(intel_dp) && fixed_mode) { > > - ret = intel_panel_compute_config(connector, adjusted_mode); > > - if (ret) > > - return ret; > > - } > > + if (intel_dp_is_edp(intel_dp) && fixed_mode) > > + pipe_config->pch_pfit.is_required = true; > > > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN) > > return -EINVAL; > > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c > > b/drivers/gpu/drm/i915/display/intel_panel.c > > index dd18136d1c61..0da45c2330d3 100644 > > --- a/drivers/gpu/drm/i915/display/intel_panel.c > > +++ b/drivers/gpu/drm/i915/display/intel_panel.c > > @@ -395,6 +395,9 @@ static int pch_panel_fitting(struct intel_crtc_state > *crtc_state, > > u16 crtc_hdisplay = adjusted_mode->crtc_hdisplay; > > u16 crtc_vdisplay = adjusted_mode->crtc_vdisplay; > > > > + if (crtc_state->joiner_pipes) > > + crtc_hdisplay = adjusted_mode->crtc_hdisplay / 2; > > + > > /* Native modes don't need fitting */ > > if (crtc_hdisplay == pipe_src_w && > > crtc_vdisplay == pipe_src_h && > > -- > Jani Nikula, Intel
