On Thu, 14 Jun 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> wrote:
> On Thu, 2018-06-14 at 16:56 +0000, Nagaraju, Vathsala wrote:
>> + Ashutosh(VBT team) + maulik
>>
>> 209 is confirmed version on kbl both by vbt team (Maulik) and google,
>> so we had used it.
>>
>> DK's suggestion is
>> if ((bdb->version >= 203 && IS_GEN9_BC(dev_priv)) ||
>> IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
>> /* new mapping */
>>
>> As per Ashutosh, 203 Is not the right version, 205 is fine , but user
>> can still provide decimal value for SKL.
>
> I am confused, why does the commit message mention 203 then?
Whatever the version, I want that to be required always. i.e.
if (version >= N && (bunch of other conditions))
*not*
if (version >= N || (bunch of other conditions))
BR,
Jani.
>
>
>> Jani/Rodrigo, should we use 205 for SKL or drop SKL from the new
>> mapping?
>>
>> -----Original Message-----
>> From: Pandiyan, Dhinakaran
>> Sent: Thursday, June 14, 2018 9:30 PM
>> To: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Nikula, Jani <jani.nikula
>> @intel.com>; Nagaraju, Vathsala <vathsala.nagaraju@xxxxxxxxx>
>> Cc: puthik@xxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH] drm/i915/psr: Adds psrwake options for all
>> platforms
>>
>> On Thu, 2018-06-14 at 11:59 +0530, Nagaraju, Vathsala wrote:
>> >
>> >
>> > On 6/13/2018 11:10 PM, Dhinakaran Pandiyan wrote:
>> > >
>> > >
>> > > On Wed, 2018-06-13 at 10:32 -0700, Dhinakaran Pandiyan wrote:
>> > > >
>> > > >
>> > > > On Wed, 2018-06-13 at 09:41 +0300, Jani Nikula wrote:
>> > > > >
>> > > > >
>> > > > > On Wed, 13 Jun 2018, "Nagaraju, Vathsala" <vathsala.nagaraju@
>> > > > > in
>> > > > > tel.
>> > > > > co
>> > > > > m> wrote:
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > On 6/12/2018 2:30 PM, Jani Nikula wrote:
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > On Tue, 12 Jun 2018, vathsala nagaraju <vathsala.nagaraju
>> > > > > > > @i
>> > > > > > > ntel .c
>> > > > > > > om> wrote:
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > From: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx>
>> > > > > > > >
>> > > > > > > > Adds new psrwake options defined in the below table.
>> > > > > > > > Platform PSR wake options vbt version
>> > > > > > > > KBL/CFL/WHL All
>> > > > > > > > SKL All PV releases (Check for 203+
>> > > > > > > > might
>> > > > > > > > help
>> > > > > > > > but cannot be foolproof)
>> > > > > > > > BXT Uses old interpretation.
>> > > > > > > > CNL/ICL+ All
>> > > > > > > > GLK All
>> > > > > > > >
>> > > > > > > > For SKL, we will continue to use older interpretation
>> > > > > > > > for
>> > > > > > > > the above reason.
>> > > > > > > >
>> > > > > > > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> > > > > > > > Cc: Puthikorn Voravootivat <puthik@xxxxxxxxxxxx>
>> > > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
>> > > > > > > >
>> > > > > > > > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@int
>> > > > > > > > el
>> > > > > > > > .com
>> > > > > > > > ---
>> > > > > > > > drivers/gpu/drm/i915/intel_bios.c | 3 ++-
>> > > > > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
>> > > > > > > >
>> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> > > > > > > > b/drivers/gpu/drm/i915/intel_bios.c
>> > > > > > > > index 465dff4..010ff68 100644
>> > > > > > > > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > > > > > > > @@ -710,7 +710,8 @@ static int
>> > > > > > > > intel_bios_ssc_frequency(struct drm_i915_private
>> > > > > > > > *dev_priv,
>> > > > > > > > * New psr options 0=500us, 1=100us,
>> > > > > > > > 2=2500us, 3=0us
>> > > > > > > > * Old decimal value is wake up time in
>> > > > > > > > multiples of
>> > > > > > > > 100 us.
>> > > > > > > > */
>> > > > > > > > - if (bdb->version >= 209 &&
>> > > > > > > > IS_GEN9_BC(dev_priv))
>> > > > > > > > {
>> > > > > > > > + if ((INTEL_GEN(dev_priv) >= 10) ||
>> > > > > > > > + (IS_GEN9_BC(dev_priv) &&
>> > > > > > > > !IS_SKYLAKE(dev_priv)))
>> > > > > > > > {
>> > > > > > > Please keep the version check.
>> > > > > > Sure. For SKL , shall we use older interpretation for all
>> > > > > > bdb
>> > > > > > version as vbt team cannot confirm bdb version for SKL?
>> > > > > I guess.
>> > > > >
>> > > > Why not change the version check to >= 203, if that's what PV
>> > > > releases
>> > > > had as per your commit message? With the current code, Linux
>> > > > and
>> > > > Windows set 500 us and 2.5 ms respectively on my laptop.
>> > > Said laptop is a SKL with bdb version 205.
>> > + ashutosh(VBT team)
>> > Since VBT team cannot confirm version for SKL ,so skipped for
>> > skylake.
>> > I did a copy paste of the table provided by vbt team, will edit for
>> > skylake.
>> >
>> We are not going to get this right for all combinations, the best we
>> can do is make sure things work in most cases. I prefer to err on the
>> side of using the new mapping because when translated incorrectly, 3
>> out of 4 values lead to >= intended training time. Given the fact
>> that
>> SKL PV releases also used the new mapping, I suggest you do this
>>
>> if ((bdb->version >= 203 && IS_GEN9_BC(dev_priv)) ||
>> IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
>> /* new mapping */
>>
>> }
>>
>> I don't know what the basis for the current check for version 209 is,
>> other than it was the version on the KBL you tested.
>>
>> -DK
>>
>>
>>
>>
>>
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
