On Thu, 19 Dec 2019, "Kulkarni, Vandita" <vandita.kulka...@intel.com> wrote:
>> -----Original Message-----
>> From: Jani Nikula <jani.nik...@intel.com>
>> Sent: Thursday, December 12, 2019 7:17 PM
>> To: Nikula, Jani <jani.nik...@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: Souza, Jose <jose.so...@intel.com>; Navare, Manasi D
>> <manasi.d.nav...@intel.com>; Kulkarni, Vandita
>> <vandita.kulka...@intel.com>
>> Subject: [PATCH 3/2] drm/i915/dsc: fix DSC power domains for DSI
>> 
>> Fix several issues with DSC power domains that did not take DSI transcoders
>> into account:
>> 
>> - On TGL+ we need to use PW2 for DSC on pipe A, not transcoder A. There
>>   is no longer an eDP transcoder, but there are two DSI transcoders
>>   which may be connected to pipe A.
>> 
>> - On TGL+ we need to use the pipe, not transcoder, power domains for DSC
>>   on pipes other than A. Again, there are DSI transcoders.
>> 
>> - On ICL we need to use PW2 for DSC also for DSI transcoders, not just
>>   for the eDP transcoder.
>> 
>> Using is_pipe_dsc() also adds the warning about ICL pipe A DSC, which does
>> not exist.
>> 
>> Cc: José Roberto de Souza <jose.so...@intel.com>
>> Cc: Manasi Navare <manasi.d.nav...@intel.com>
>> Cc: Vandita Kulkarni <vandita.kulka...@intel.com>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_vdsc.c | 28 ++++++++++++-----------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
>> b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> index 41718f721484..6bab08db5d75 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> @@ -469,25 +469,27 @@ int intel_dsc_compute_params(struct
>> intel_encoder *encoder,  enum intel_display_power_domain
>> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state)  {
>> -    struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>> -    enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>> +    struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>> +    struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> +    enum pipe pipe = crtc->pipe;
>> 
>>      /*
>> -     * On ICL VDSC/joining for eDP transcoder uses a separate power
>> well,
>> -     * PW2. This requires POWER_DOMAIN_TRANSCODER_VDSC_PW2
>> power domain.
>> -     * For any other transcoder, VDSC/joining uses the power well
>> associated
>> -     * with the pipe/transcoder in use. Hence another reference on the
>> -     * transcoder power domain will suffice.
>> +     * VDSC/joining uses a separate power well, PW2, and requires
>> +     * POWER_DOMAIN_TRANSCODER_VDSC_PW2 power domain in two
>> cases:
>>       *
>> -     * On TGL we have the same mapping, but for transcoder A (the
>> special
>> -     * TRANSCODER_EDP is gone).
>
> Comment in drivers/gpu/drm/i915/display/intel_display_power.h where 
> POWER_DOMAIN_TRANSCODER_VDSC_PW2 is defined misses DSI transcoder.

I'll send a follow-up patch for this.

> Rest all looks good to me.
> Reviewed-by: Vandita Kulkarni <vandita.kulka...@intel.com>

Pushed the lot, thanks for the review.

BR,
Jani.

>
> Thanks,
> Vandita
>
>> +     *  - ICL eDP/DSI transcoder
>> +     *  - TGL pipe A
>> +     *
>> +     * For any other pipe, VDSC/joining uses the power well associated
>> with
>> +     * the pipe in use. Hence another reference on the pipe power
>> domain
>> +     * will suffice. (Except no VDSC/joining on ICL pipe A.)
>>       */
>> -    if (INTEL_GEN(i915) >= 12 && cpu_transcoder == TRANSCODER_A)
>> -            return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
>> -    else if (cpu_transcoder == TRANSCODER_EDP)
>> +    if (INTEL_GEN(i915) >= 12 && pipe == PIPE_A)
>>              return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
>> +    else if (is_pipe_dsc(crtc_state))
>> +            return POWER_DOMAIN_PIPE(pipe);
>>      else
>> -            return POWER_DOMAIN_TRANSCODER(cpu_transcoder);
>> +            return POWER_DOMAIN_TRANSCODER_VDSC_PW2;
>>  }
>> 
>>  static void intel_dsc_pps_configure(struct intel_encoder *encoder,
>> --
>> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to