| Message ID | 20240808045407.2365733-1-nemesa.garg@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 8/8/2024 10:24 AM, 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 get enabled > together. > > Introduce a new boolean flag need_joiner which is set during dp > compute_config in joiner case and later is used to compute > panel_fitting in pipe_config. Modify pch_panel_fitting to handle > joiner pipes by adjusting crtc_hdisplay accordingly. > > v2: Address comments (Ankit) In general, it would be good to document the changes patch went through. > v3: Change flag name (Ankit) > > 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 | 11 ++++++++--- > drivers/gpu/drm/i915/display/intel_panel.c | 3 +++ > 4 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 8bbde03f2508..82b67c0a90e0 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.need_joiner) { > + 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..eb9713b088c6 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 need_joiner; > } 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 65182bf69b62..d5d9d4f21fc7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2953,9 +2953,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if ((intel_dp_is_edp(intel_dp) && fixed_mode) || > pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) { > - ret = intel_panel_fitting(pipe_config, conn_state); > - if (ret) > - return ret; > + if (!pipe_config->joiner_pipes) { > + ret = intel_panel_fitting(pipe_config, conn_state); > + if (ret) > + return ret; > + } else { > + /* Incase of joiner panel_fitting is handled during pipe_config */ 'pipe_config' seems confusing, as we already have a variable pipe_config. Also, perhaps we can explain a bit more about the rationale behind handling this at later point. Apart from the minor things above, the patch looks good to me. Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > + pipe_config->pch_pfit.need_joiner = true; > + } > } > > pipe_config->limited_color_range = > 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 Thu, 08 Aug 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 get enabled > together. > > Introduce a new boolean flag need_joiner which is set during dp > compute_config in joiner case and later is used to compute > panel_fitting in pipe_config. Modify pch_panel_fitting to handle > joiner pipes by adjusting crtc_hdisplay accordingly. So I still don't like the fact that intel_panel_fitting() is called in different ways for different connectors, controlled by a flag in crtc state. That said, I couldn't come up with a better idea either, apart from moving *all* panel fitting intel_modeset_pipe_config(). Cc: Ville, in case he has some ideas for this. Please hold off on merging until we get some input from him. Thanks, Jani. > > v2: Address comments (Ankit) > v3: Change flag name (Ankit) > > 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 | 11 ++++++++--- > drivers/gpu/drm/i915/display/intel_panel.c | 3 +++ > 4 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 8bbde03f2508..82b67c0a90e0 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.need_joiner) { > + 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..eb9713b088c6 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 need_joiner; > } 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 65182bf69b62..d5d9d4f21fc7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2953,9 +2953,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if ((intel_dp_is_edp(intel_dp) && fixed_mode) || > pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) { > - ret = intel_panel_fitting(pipe_config, conn_state); > - if (ret) > - return ret; > + if (!pipe_config->joiner_pipes) { > + ret = intel_panel_fitting(pipe_config, conn_state); > + if (ret) > + return ret; > + } else { > + /* Incase of joiner panel_fitting is handled during pipe_config */ > + pipe_config->pch_pfit.need_joiner = true; > + } > } > > pipe_config->limited_color_range = > 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, August 13, 2024 1:22 PM > To: Garg, Nemesa <nemesa.garg@intel.com>; intel-gfx@lists.freedesktop.org; > Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Garg, Nemesa <nemesa.garg@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/display: Call panel_fitting from pipe_config > > On Thu, 08 Aug 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 get enabled together. > > > > Introduce a new boolean flag need_joiner which is set during dp > > compute_config in joiner case and later is used to compute > > panel_fitting in pipe_config. Modify pch_panel_fitting to handle > > joiner pipes by adjusting crtc_hdisplay accordingly. > > So I still don't like the fact that intel_panel_fitting() is called in different ways for > different connectors, controlled by a flag in crtc state. > > That said, I couldn't come up with a better idea either, apart from moving *all* > panel fitting intel_modeset_pipe_config(). > > Cc: Ville, in case he has some ideas for this. Please hold off on merging until we > get some input from him. > > Hi Ville, Can you please suggest how should I proceed further on this patch. Thanks and Regards, Nemesa > Thanks, > Jani. > > > > > > v2: Address comments (Ankit) > > v3: Change flag name (Ankit) > > > > 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 | 11 ++++++++--- > > drivers/gpu/drm/i915/display/intel_panel.c | 3 +++ > > 4 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 8bbde03f2508..82b67c0a90e0 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.need_joiner) { > > + 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..eb9713b088c6 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 need_joiner; > > } 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 65182bf69b62..d5d9d4f21fc7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -2953,9 +2953,14 @@ intel_dp_compute_config(struct intel_encoder > > *encoder, > > > > if ((intel_dp_is_edp(intel_dp) && fixed_mode) || > > pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) > { > > - ret = intel_panel_fitting(pipe_config, conn_state); > > - if (ret) > > - return ret; > > + if (!pipe_config->joiner_pipes) { > > + ret = intel_panel_fitting(pipe_config, conn_state); > > + if (ret) > > + return ret; > > + } else { > > + /* Incase of joiner panel_fitting is handled during > pipe_config */ > > + pipe_config->pch_pfit.need_joiner = true; > > + } > > } > > > > pipe_config->limited_color_range = > > 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
On Mon, Sep 09, 2024 at 06:21:49AM +0000, Garg, Nemesa wrote: > > > > -----Original Message----- > > From: Jani Nikula <jani.nikula@linux.intel.com> > > Sent: Tuesday, August 13, 2024 1:22 PM > > To: Garg, Nemesa <nemesa.garg@intel.com>; intel-gfx@lists.freedesktop.org; > > Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Garg, Nemesa <nemesa.garg@intel.com> > > Subject: Re: [PATCH 2/2] drm/i915/display: Call panel_fitting from pipe_config > > > > On Thu, 08 Aug 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 get enabled together. > > > > > > Introduce a new boolean flag need_joiner which is set during dp > > > compute_config in joiner case and later is used to compute > > > panel_fitting in pipe_config. Modify pch_panel_fitting to handle > > > joiner pipes by adjusting crtc_hdisplay accordingly. > > > > So I still don't like the fact that intel_panel_fitting() is called in different ways for > > different connectors, controlled by a flag in crtc state. > > > > That said, I couldn't come up with a better idea either, apart from moving *all* > > panel fitting intel_modeset_pipe_config(). > > > > Cc: Ville, in case he has some ideas for this. Please hold off on merging until we > > get some input from him. > > > > Hi Ville, > Can you please suggest how should I proceed further on this patch. I think we want to move the whole thing to happen after we've computed final pipe_src and pipe_mode (which can then be used intead of adjusted_mode in the pfit calculaitons). The one annoying part of this is that we probably can only do it for the ilk+ pfit ("pch"), but the old gmch pfit probably has to stay where it is now because it may have to adjust the actual transcoder timings for the purposes of adding borders. Also I don't think we should really need any extra flags in the crtc state. The pfit code should be able to figure it all out on its own.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 8bbde03f2508..82b67c0a90e0 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.need_joiner) { + 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..eb9713b088c6 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 need_joiner; } 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 65182bf69b62..d5d9d4f21fc7 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2953,9 +2953,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, if ((intel_dp_is_edp(intel_dp) && fixed_mode) || pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) { - ret = intel_panel_fitting(pipe_config, conn_state); - if (ret) - return ret; + if (!pipe_config->joiner_pipes) { + ret = intel_panel_fitting(pipe_config, conn_state); + if (ret) + return ret; + } else { + /* Incase of joiner panel_fitting is handled during pipe_config */ + pipe_config->pch_pfit.need_joiner = true; + } } pipe_config->limited_color_range = 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 get enabled together. Introduce a new boolean flag need_joiner which is set during dp compute_config in joiner case and later is used to compute panel_fitting in pipe_config. Modify pch_panel_fitting to handle joiner pipes by adjusting crtc_hdisplay accordingly. v2: Address comments (Ankit) v3: Change flag name (Ankit) 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 | 11 ++++++++--- drivers/gpu/drm/i915/display/intel_panel.c | 3 +++ 4 files changed, 23 insertions(+), 3 deletions(-)