Re: [Intel-gfx] [PATCH 2/2] drm/i915/pps: Enabled 2nd pps for dual EDP scenario

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 06 Sep 2022, Animesh Manna <animesh.manna@intel.com> 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.
>
> TODO: For dual EDP scenario and panel type invalid (=255), special condition
> check to be added to reject or initialize the panel specific stuff earlier.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>

So this is no worse than it already is for GLK and BXT... but they're
actually already broken. :(

As I tried to explain before, since commit 3cf050762534 ("drm/i915/bios:
Split VBT data into per-panel vs. global parts") the VBT
connector->panel.vbt.backlight.controller member gets initialized via
intel_bios_init_panel(). But intel_pps_init() uses that *before*
intel_bios_init_panel() gets called.

Mind you, I don't think the dual backlight controller thing worked
before that either, but the controller field was more likely to be
correct for the first panel. Now, it's only correct by coincidence, as
it's uninitialized.

So it's not only about PNPID panel identification (panel type 255). But
that's related, since we can't move the PNPID identification earlier,
because that needs EDID, and EDID needs panel power, and panel power
needs the PPS index. Which needs PNPID panel identification.

We'll need to do something like:

- intel_bios_panel_init w/o PNPID
- intel_pps_init
- EDID read
- intel_bios_panel_init w/ PNPID

I don't know how exactly this is supposed to work, but I'm also kind of
not tasked to figure it out either right now. ;)

HTH,
Jani.


> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 4e770218e29f..a9ed1214a167 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -1427,7 +1427,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;

-- 
Jani Nikula, Intel Open Source Graphics Center