On Thu, 2018-05-17 at 11:02 +0300, Jani Nikula wrote: > On Wed, 16 May 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.c > om> wrote: > > > > On Wed, 2018-05-16 at 11:08 +0300, Jani Nikula wrote: > > > > > > I think the patch is now the way it should be. We should not > > > change > > > our interpretation based on the value. > > Is it correct to infer, from your response, that VBT values are not > > always set based on hardware capability as documented in bspec? > Correct. I think it was the good intention of the VBT change to only > allow values that map to valid hardware values, but I think it has > caused much more trouble than it has helped. > > First, the change was not universally tied to a VBT version, and I've > seen VBTs in the wild with version >= 209 that still use multiples of > 100 us. (And the spec is still in contradiction with itself.) > > Second, regardless of mapping or multiples of 100 us we need to > verify > our input. Because I've seen VBTs in the wild that are bogus > regardless > of how they're supposed to be interpreted. > > Third, we already had the code in place to map multiples of 100 us to > hardware. We'll need to keep that practically forever. And now we > need > to have *another* mapping. > > Fourth, the multiples of 100 us does not require *any* spec change > when > hardware changes to support different values. The new mapping > requires > changes throughout the stack that looks at the values. (Basically I > object to any VBT specification that says anything platform > dependent. It should be generic.) > > Now, the patch at hand uses the best guesses we can make to translate > whatever the VBT has to microseconds, in intel_bios.c. That part does > not care about hardware capability. For validity, it only looks at > what > we think is according to VBT spec. It should be hardware agnostic, > except for the IS_GEN9_BC() thing, which should probably include > gen10+ > too. > > The code in intel_psr.c gets the microseconds as input, and maps that > to > hardware capability as best we can, erring towards longer delays. > > Two different abstractions for two different things. One to abstract > VBT, another to abstract hardware. This is in line with the direction > I've tried to take intel_bios.c and VBT parsing and the VBT spec (the > little I've had influence for that) for the longest time. We must not > let the VBT abstractions to leak into the driver code. Thanks for the detailed explanation of the problem and the idea behind the design. -DK > > BR, > Jani. > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
