| Message ID | 20221108211822.28048-2-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup | expand |
On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote: > >From display gen12 onwards to support dual EDP two instances of pps added. > Currently backlight controller and pps instance can be mapped together > for a specific panel. Extended support for gen12 for dual EDP usage. > > v1: Iniital revision > v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- > drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ > drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- > drivers/gpu/drm/i915/display/intel_pps.c | 2 +- > 4 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index c2987f2c2b2e..fca44be9bab8 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915, > return 0; > } > > -enum panel_type { > - PANEL_TYPE_OPREGION, > - PANEL_TYPE_VBT, > - PANEL_TYPE_PNPID, > - PANEL_TYPE_FALLBACK, > -}; > - > static int get_panel_type(struct drm_i915_private *i915, > const struct intel_bios_encoder_data *devdata, > const struct edid *edid) > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h > index e375405a7828..da01b13260ae 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.h > +++ b/drivers/gpu/drm/i915/display/intel_bios.h > @@ -231,6 +231,13 @@ struct mipi_pps_data { > u16 panel_power_cycle_delay; > } __packed; > > +enum panel_type { > + PANEL_TYPE_OPREGION, > + PANEL_TYPE_VBT, > + PANEL_TYPE_PNPID, > + PANEL_TYPE_FALLBACK, > +}; > + > void intel_bios_init(struct drm_i915_private *dev_priv); > void intel_bios_init_panel(struct drm_i915_private *dev_priv, > struct intel_panel *panel, > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 7400d6b4c587..08ece347f7cb 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return false; > } > > + intel_bios_init_panel(dev_priv, &intel_connector->panel, > + encoder->devdata, NULL); > + > intel_pps_init(intel_dp); > > /* Cache DPCD and EDID for edp. */ > @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > edid = ERR_PTR(-ENOENT); > } > intel_connector->edid = edid; > - > - intel_bios_init_panel(dev_priv, &intel_connector->panel, > - encoder->devdata, IS_ERR(edid) ? NULL : edid); > + if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK) vbt.panel_type does _not_ contain enum panel_type (a bit of an unfortunate name selection by me there I guess). > + intel_bios_init_panel(dev_priv, &intel_connector->panel, > + encoder->devdata, IS_ERR(edid) ? NULL : edid); I suspect just calling this twice might leak all kinds of stuff. > > intel_panel_add_edid_fixed_modes(intel_connector, true); > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index 3949fb449353..0975e49f8d03 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > intel_dp->pps.initializing = true; > INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); > > - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) > + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) > intel_dp->get_pps_idx = bxt_power_sequencer_idx; > else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > intel_dp->get_pps_idx = vlv_power_sequencer_pipe; > -- > 2.29.0
On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote: >> >From display gen12 onwards to support dual EDP two instances of pps added. >> Currently backlight controller and pps instance can be mapped together >> for a specific panel. Extended support for gen12 for dual EDP usage. >> >> v1: Iniital revision >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] >> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Uma Shankar <uma.shankar@intel.com> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- >> drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ >> drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- >> drivers/gpu/drm/i915/display/intel_pps.c | 2 +- >> 4 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >> index c2987f2c2b2e..fca44be9bab8 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915, >> return 0; >> } >> >> -enum panel_type { >> - PANEL_TYPE_OPREGION, >> - PANEL_TYPE_VBT, >> - PANEL_TYPE_PNPID, >> - PANEL_TYPE_FALLBACK, >> -}; >> - >> static int get_panel_type(struct drm_i915_private *i915, >> const struct intel_bios_encoder_data *devdata, >> const struct edid *edid) >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h >> index e375405a7828..da01b13260ae 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bios.h >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h >> @@ -231,6 +231,13 @@ struct mipi_pps_data { >> u16 panel_power_cycle_delay; >> } __packed; >> >> +enum panel_type { >> + PANEL_TYPE_OPREGION, >> + PANEL_TYPE_VBT, >> + PANEL_TYPE_PNPID, >> + PANEL_TYPE_FALLBACK, >> +}; >> + >> void intel_bios_init(struct drm_i915_private *dev_priv); >> void intel_bios_init_panel(struct drm_i915_private *dev_priv, >> struct intel_panel *panel, >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c >> index 7400d6b4c587..08ece347f7cb 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, >> return false; >> } >> >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, >> + encoder->devdata, NULL); >> + >> intel_pps_init(intel_dp); >> >> /* Cache DPCD and EDID for edp. */ >> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, >> edid = ERR_PTR(-ENOENT); >> } >> intel_connector->edid = edid; >> - >> - intel_bios_init_panel(dev_priv, &intel_connector->panel, >> - encoder->devdata, IS_ERR(edid) ? NULL : edid); >> + if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK) > > vbt.panel_type does _not_ contain enum panel_type (a bit of an > unfortunate name selection by me there I guess). > >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, >> + encoder->devdata, IS_ERR(edid) ? NULL : edid); > > I suspect just calling this twice might leak all kinds of stuff. Yeah, what's up with this, this is some old version that was fixed way back too. I thought this was good to go: https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com > >> >> intel_panel_add_edid_fixed_modes(intel_connector, true); >> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c >> index 3949fb449353..0975e49f8d03 100644 >> --- a/drivers/gpu/drm/i915/display/intel_pps.c >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) >> intel_dp->pps.initializing = true; >> INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); >> >> - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) >> + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) >> intel_dp->get_pps_idx = bxt_power_sequencer_idx; >> else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) >> intel_dp->get_pps_idx = vlv_power_sequencer_pipe; >> -- >> 2.29.0
On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote: > On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote: > >> >From display gen12 onwards to support dual EDP two instances of pps added. > >> Currently backlight controller and pps instance can be mapped together > >> for a specific panel. Extended support for gen12 for dual EDP usage. > >> > >> v1: Iniital revision > >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] > >> > >> Cc: Jani Nikula <jani.nikula@intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Uma Shankar <uma.shankar@intel.com> > >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- > >> drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ > >> drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- > >> drivers/gpu/drm/i915/display/intel_pps.c | 2 +- > >> 4 files changed, 14 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > >> index c2987f2c2b2e..fca44be9bab8 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915, > >> return 0; > >> } > >> > >> -enum panel_type { > >> - PANEL_TYPE_OPREGION, > >> - PANEL_TYPE_VBT, > >> - PANEL_TYPE_PNPID, > >> - PANEL_TYPE_FALLBACK, > >> -}; > >> - > >> static int get_panel_type(struct drm_i915_private *i915, > >> const struct intel_bios_encoder_data *devdata, > >> const struct edid *edid) > >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h > >> index e375405a7828..da01b13260ae 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_bios.h > >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h > >> @@ -231,6 +231,13 @@ struct mipi_pps_data { > >> u16 panel_power_cycle_delay; > >> } __packed; > >> > >> +enum panel_type { > >> + PANEL_TYPE_OPREGION, > >> + PANEL_TYPE_VBT, > >> + PANEL_TYPE_PNPID, > >> + PANEL_TYPE_FALLBACK, > >> +}; > >> + > >> void intel_bios_init(struct drm_i915_private *dev_priv); > >> void intel_bios_init_panel(struct drm_i915_private *dev_priv, > >> struct intel_panel *panel, > >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > >> index 7400d6b4c587..08ece347f7cb 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > >> return false; > >> } > >> > >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> + encoder->devdata, NULL); > >> + > >> intel_pps_init(intel_dp); > >> > >> /* Cache DPCD and EDID for edp. */ > >> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > >> edid = ERR_PTR(-ENOENT); > >> } > >> intel_connector->edid = edid; > >> - > >> - intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> - encoder->devdata, IS_ERR(edid) ? NULL : edid); > >> + if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK) > > > > vbt.panel_type does _not_ contain enum panel_type (a bit of an > > unfortunate name selection by me there I guess). > > > >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> + encoder->devdata, IS_ERR(edid) ? NULL : edid); > > > > I suspect just calling this twice might leak all kinds of stuff. > > Yeah, what's up with this, this is some old version that was fixed way > back too. I thought this was good to go: > > https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com That look broken in exactly the same way wrt. enum panel_type. > > > > >> > >> intel_panel_add_edid_fixed_modes(intel_connector, true); > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > >> index 3949fb449353..0975e49f8d03 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_pps.c > >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c > >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > >> intel_dp->pps.initializing = true; > >> INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); > >> > >> - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) > >> + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) > >> intel_dp->get_pps_idx = bxt_power_sequencer_idx; > >> else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > >> intel_dp->get_pps_idx = vlv_power_sequencer_pipe; > >> -- > >> 2.29.0 > > -- > Jani Nikula, Intel Open Source Graphics Center
On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote: >> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote: >> >> >From display gen12 onwards to support dual EDP two instances of pps added. >> >> Currently backlight controller and pps instance can be mapped together >> >> for a specific panel. Extended support for gen12 for dual EDP usage. >> >> >> >> v1: Iniital revision >> >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] >> >> >> >> Cc: Jani Nikula <jani.nikula@intel.com> >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> Cc: Uma Shankar <uma.shankar@intel.com> >> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- >> >> drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ >> >> drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- >> >> drivers/gpu/drm/i915/display/intel_pps.c | 2 +- >> >> 4 files changed, 14 insertions(+), 11 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c >> >> index c2987f2c2b2e..fca44be9bab8 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915, >> >> return 0; >> >> } >> >> >> >> -enum panel_type { >> >> - PANEL_TYPE_OPREGION, >> >> - PANEL_TYPE_VBT, >> >> - PANEL_TYPE_PNPID, >> >> - PANEL_TYPE_FALLBACK, >> >> -}; >> >> - >> >> static int get_panel_type(struct drm_i915_private *i915, >> >> const struct intel_bios_encoder_data *devdata, >> >> const struct edid *edid) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h >> >> index e375405a7828..da01b13260ae 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h >> >> @@ -231,6 +231,13 @@ struct mipi_pps_data { >> >> u16 panel_power_cycle_delay; >> >> } __packed; >> >> >> >> +enum panel_type { >> >> + PANEL_TYPE_OPREGION, >> >> + PANEL_TYPE_VBT, >> >> + PANEL_TYPE_PNPID, >> >> + PANEL_TYPE_FALLBACK, >> >> +}; >> >> + >> >> void intel_bios_init(struct drm_i915_private *dev_priv); >> >> void intel_bios_init_panel(struct drm_i915_private *dev_priv, >> >> struct intel_panel *panel, >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c >> >> index 7400d6b4c587..08ece347f7cb 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, >> >> return false; >> >> } >> >> >> >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, >> >> + encoder->devdata, NULL); >> >> + >> >> intel_pps_init(intel_dp); >> >> >> >> /* Cache DPCD and EDID for edp. */ >> >> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, >> >> edid = ERR_PTR(-ENOENT); >> >> } >> >> intel_connector->edid = edid; >> >> - >> >> - intel_bios_init_panel(dev_priv, &intel_connector->panel, >> >> - encoder->devdata, IS_ERR(edid) ? NULL : edid); >> >> + if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK) >> > >> > vbt.panel_type does _not_ contain enum panel_type (a bit of an >> > unfortunate name selection by me there I guess). >> > >> >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, >> >> + encoder->devdata, IS_ERR(edid) ? NULL : edid); >> > >> > I suspect just calling this twice might leak all kinds of stuff. >> >> Yeah, what's up with this, this is some old version that was fixed way >> back too. I thought this was good to go: >> >> https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com > > That look broken in exactly the same way wrt. enum panel_type. You're right, of course. BR, Jani. > >> >> > >> >> >> >> intel_panel_add_edid_fixed_modes(intel_connector, true); >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c >> >> index 3949fb449353..0975e49f8d03 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c >> >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) >> >> intel_dp->pps.initializing = true; >> >> INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); >> >> >> >> - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) >> >> + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) >> >> intel_dp->get_pps_idx = bxt_power_sequencer_idx; >> >> else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) >> >> intel_dp->get_pps_idx = vlv_power_sequencer_pipe; >> >> -- >> >> 2.29.0 >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Wednesday, November 9, 2022 4:12 PM > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario > > On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote: > >> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote: > >> >> >From display gen12 onwards to support dual EDP two instances of pps > added. > >> >> Currently backlight controller and pps instance can be mapped > >> >> together for a specific panel. Extended support for gen12 for dual EDP > usage. > >> >> > >> >> v1: Iniital revision > >> >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. > >> >> [Jani] > >> >> > >> >> Cc: Jani Nikula <jani.nikula@intel.com> > >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> >> Cc: Uma Shankar <uma.shankar@intel.com> > >> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- > >> >> drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ > >> >> drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- > >> >> drivers/gpu/drm/i915/display/intel_pps.c | 2 +- > >> >> 4 files changed, 14 insertions(+), 11 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > >> >> b/drivers/gpu/drm/i915/display/intel_bios.c > >> >> index c2987f2c2b2e..fca44be9bab8 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct > drm_i915_private *i915, > >> >> return 0; > >> >> } > >> >> > >> >> -enum panel_type { > >> >> - PANEL_TYPE_OPREGION, > >> >> - PANEL_TYPE_VBT, > >> >> - PANEL_TYPE_PNPID, > >> >> - PANEL_TYPE_FALLBACK, > >> >> -}; > >> >> - > >> >> static int get_panel_type(struct drm_i915_private *i915, > >> >> const struct intel_bios_encoder_data *devdata, > >> >> const struct edid *edid) > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h > >> >> b/drivers/gpu/drm/i915/display/intel_bios.h > >> >> index e375405a7828..da01b13260ae 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h > >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h > >> >> @@ -231,6 +231,13 @@ struct mipi_pps_data { > >> >> u16 panel_power_cycle_delay; > >> >> } __packed; > >> >> > >> >> +enum panel_type { > >> >> + PANEL_TYPE_OPREGION, > >> >> + PANEL_TYPE_VBT, > >> >> + PANEL_TYPE_PNPID, > >> >> + PANEL_TYPE_FALLBACK, > >> >> +}; > >> >> + > >> >> void intel_bios_init(struct drm_i915_private *dev_priv); void > >> >> intel_bios_init_panel(struct drm_i915_private *dev_priv, > >> >> struct intel_panel *panel, > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > >> >> b/drivers/gpu/drm/i915/display/intel_dp.c > >> >> index 7400d6b4c587..08ece347f7cb 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c > >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c > >> >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct > intel_dp *intel_dp, > >> >> return false; > >> >> } > >> >> > >> >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> >> + encoder->devdata, NULL); > >> >> + > >> >> intel_pps_init(intel_dp); > >> >> > >> >> /* Cache DPCD and EDID for edp. */ @@ -5288,9 +5291,9 @@ static > >> >> bool intel_edp_init_connector(struct intel_dp *intel_dp, > >> >> edid = ERR_PTR(-ENOENT); > >> >> } > >> >> intel_connector->edid = edid; > >> >> - > >> >> - intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> >> - encoder->devdata, IS_ERR(edid) ? NULL : edid); > >> >> + if (intel_connector->panel.vbt.panel_type == > >> >> +PANEL_TYPE_FALLBACK) > >> > > >> > vbt.panel_type does _not_ contain enum panel_type (a bit of an > >> > unfortunate name selection by me there I guess). > >> > > >> >> + intel_bios_init_panel(dev_priv, &intel_connector->panel, > >> >> + encoder->devdata, IS_ERR(edid) ? NULL : > edid); > >> > > >> > I suspect just calling this twice might leak all kinds of stuff. > >> > >> Yeah, what's up with this, this is some old version that was fixed > >> way back too. I thought this was good to go: > >> > >> https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel. > >> com > > > > That look broken in exactly the same way wrt. enum panel_type. > > You're right, of course. My Bad, will rectify and send the new version right away .. JFYI, With the correct initial series [1] warning is seen on [2] [1] https://patchwork.freedesktop.org/series/109820/ [2] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109820v2/bat-adlp-4/igt@i915_module_load@load.html Regards, Animesh > > BR, > Jani. > > > > > >> > >> > > >> >> > >> >> intel_panel_add_edid_fixed_modes(intel_connector, true); > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > >> >> b/drivers/gpu/drm/i915/display/intel_pps.c > >> >> index 3949fb449353..0975e49f8d03 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c > >> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c > >> >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > >> >> intel_dp->pps.initializing = true; > >> >> INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, > >> >> edp_panel_vdd_work); > >> >> > >> >> - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) > >> >> + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) > >> >> +>= 12) > >> >> intel_dp->get_pps_idx = bxt_power_sequencer_idx; > >> >> else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > >> >> intel_dp->get_pps_idx = vlv_power_sequencer_pipe; > >> >> -- > >> >> 2.29.0 > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index c2987f2c2b2e..fca44be9bab8 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915, return 0; } -enum panel_type { - PANEL_TYPE_OPREGION, - PANEL_TYPE_VBT, - PANEL_TYPE_PNPID, - PANEL_TYPE_FALLBACK, -}; - static int get_panel_type(struct drm_i915_private *i915, const struct intel_bios_encoder_data *devdata, const struct edid *edid) diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h index e375405a7828..da01b13260ae 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.h +++ b/drivers/gpu/drm/i915/display/intel_bios.h @@ -231,6 +231,13 @@ struct mipi_pps_data { u16 panel_power_cycle_delay; } __packed; +enum panel_type { + PANEL_TYPE_OPREGION, + PANEL_TYPE_VBT, + PANEL_TYPE_PNPID, + PANEL_TYPE_FALLBACK, +}; + void intel_bios_init(struct drm_i915_private *dev_priv); void intel_bios_init_panel(struct drm_i915_private *dev_priv, struct intel_panel *panel, diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 7400d6b4c587..08ece347f7cb 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, return false; } + intel_bios_init_panel(dev_priv, &intel_connector->panel, + encoder->devdata, NULL); + intel_pps_init(intel_dp); /* Cache DPCD and EDID for edp. */ @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, edid = ERR_PTR(-ENOENT); } intel_connector->edid = edid; - - intel_bios_init_panel(dev_priv, &intel_connector->panel, - encoder->devdata, IS_ERR(edid) ? NULL : edid); + if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK) + intel_bios_init_panel(dev_priv, &intel_connector->panel, + encoder->devdata, IS_ERR(edid) ? NULL : edid); intel_panel_add_edid_fixed_modes(intel_connector, true); diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index 3949fb449353..0975e49f8d03 100644 --- a/drivers/gpu/drm/i915/display/intel_pps.c +++ b/drivers/gpu/drm/i915/display/intel_pps.c @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp) intel_dp->pps.initializing = true; INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work); - if (IS_GEMINILAKE(i915) || IS_BROXTON(i915)) + if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12) intel_dp->get_pps_idx = bxt_power_sequencer_idx; else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
From display gen12 onwards to support dual EDP two instances of pps added. Currently backlight controller and pps instance can be mapped together for a specific panel. Extended support for gen12 for dual EDP usage. v1: Iniital revision v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani] Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Uma Shankar <uma.shankar@intel.com> Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 7 ------- drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++ drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++--- drivers/gpu/drm/i915/display/intel_pps.c | 2 +- 4 files changed, 14 insertions(+), 11 deletions(-)