| Message ID | 20240726095357.1261804-3-nemesa.garg@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Consider joiner calculation for panel fitting | expand |
On 7/26/2024 3:23 PM, Nemesa Garg 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 typo: get > 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. > > Signed-off-by: Nemesa Garg <nemesa.garg@intel.com> Lets add version history from previous revision : https://patchwork.freedesktop.org/patch/602385/?series=135791&rev=1 > --- > 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; Although joiner is currently used only for eDP/DP, if we are changing it for DP, we need to do this for other interfaces also, where intel_panel_compute_config is called. Regards, Ankit > > 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 &&
On Fri, 26 Jul 2024, Nemesa Garg <nemesa.garg@intel.com> 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. > > Signed-off-by: Nemesa Garg <nemesa.garg@intel.com> > --- > 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 &&
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Tuesday, July 30, 2024 1:27 PM > To: Garg, Nemesa <nemesa.garg@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Garg, Nemesa <nemesa.garg@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/display: Call panel_fitting from pipe_config > > On Fri, 26 Jul 2024, Nemesa Garg <nemesa.garg@intel.com> 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@intel.com> > > --- > > 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
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 &&
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. Signed-off-by: Nemesa Garg <nemesa.garg@intel.com> --- 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(-)