<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 2/16/2016 3:35 PM, Ville Syrjälä wrote: </div> <blockquote cite="mid:20160216100521.GD23290@intel.com" type="cite"> <pre wrap="">On Tue, Feb 16, 2016 at 07:13:05AM +0530, Thulasimani, Sivakumar wrote: </pre> <blockquote type="cite"> <pre wrap=""> On 2/12/2016 8:36 PM, <a class="moz-txt-link-abbreviated" href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a> wrote: </pre> <blockquote type="cite"> <pre wrap="">From: Ville Syrjälä <a class="moz-txt-link-rfc2396E" href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a> Bspec tells us that we can allow cdclk up to 540Mhz on BDW ULX, or up to 675 MHz on ULT, bu only if extra cooling is provided. There don't seem to be any strap or VBT bits to tells us this however. But I did spot something potentially relevant in VBIOS_GOP_Driver_SWF_Registers.pdf. Apparently VBIOS/GOP can pass the max cdclk frequeny in SWF06 to the driver. Let's assume the firmware knows what its doing and trust the max cdclk in SWF06 if it's higher than the basic limit specified in Bspec. To avoid regressing anything let's ignore SWF06 if it indicates a lower limit than Bspec. Cc: Clint Taylor <a class="moz-txt-link-rfc2396E" href="mailto:clinton.a.taylor@intel.com"><clinton.a.taylor@intel.com></a> Cc: "Thulasimani, Sivakumar" <a class="moz-txt-link-rfc2396E" href="mailto:sivakumar.thulasimani@intel.com"><sivakumar.thulasimani@intel.com></a> Signed-off-by: Ville Syrjälä <a class="moz-txt-link-rfc2396E" href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a> --- I'm not at all sure if this is the right way to go about it. Sivakumar, since you seem to have some ideas on this maybe you can have a look. I'm not aware of any complaints so far that people can't get the cdclk as high is they should on specific machines, so not sure if this is really even needed. The other open question is what we should do if the VBIOS limit is lower than what we'd expect based on BSpec. Should we still trust it? Sadly we can't verify the SWF06 cdclk value in any way since it only has two bits and we have four possible cdclk values. </pre> </blockquote> <pre wrap="">The value stored in SWF06 is the CD Clock VBIOS/GOP sees during boot. so it just backs up the CD Clock before it optimizes for the available LFP. if we are trusting for higher value we should trust it for lower value too. The OEM might have did this to either reduce max resolution for cheaper products or might have removed some cooling mechanisms for thinner designs since we cannot say which of the reasons we should trust the lower value too. now comes to tough part :(, this SWF06 is saved by VBIOS/GOP driver from intel alone(it is not programmed from VBT), since GOP driver can be implemented by anyone and if anyone implements their own GOP driver we cannot be sure if the value is valid or not. please check if we can check for "Intel GOP driver". And if "Intel GOP driver" was used during boot, we can trust the value 100%. i am not sure how this can be done, so i would recommend trusting the values with clear debug messages as done below already. </pre> </blockquote> <pre wrap=""> We definitely need a way to validate the register value before we trust it for lower values. I suppose we might be able to look at bits 31:16 since those should store the current cdclk "decimal" value. If that part looks reasonable, we might be able to trust the "max cdclk" bits as well. </pre> </blockquote> it seems the bits 31:16 are used only by VBIOS and not GOP, so that wont help. we need to come up with some other method to confirm the value or verify if intel gop is loaded. i will get back if i can find such a mechanism. <blockquote cite="mid:20160216100521.GD23290@intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap=""> As mentioned in another thread, this needs to be done for SKL too. i dont have a SKL system so if no one else can make a patch for it i will have to share an untested patch for it :). </pre> </blockquote> <pre wrap=""> For SKL it seems a bit easier, since it apparently just stores the max cdclk in 10.1 decimal fixed point in bits 10:0. So invalid/uninitialized values would be easier to spot. Hmm. 11 bits and 10.1 fixed point format, I wonder if it's really decimal and not binary fixed point... </pre> </blockquote> it uses the same value programmed in CDCLK_CTL register. i.e 01 0011 0011 1b = 308.57MHz regards, Sivakumar <span style="color: rgb(0, 113, 197); font-family: Arial, sans-serif; font-size: 13.3333px; font-style: normal; font-variant: normal; font-weight: bold; letter-spacing: normal; line-height: normal; orphans: auto; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px; display: inline !important; float: none; background-color: rgb(225, 240, 252);"> <blockquote cite="mid:20160216100521.GD23290@intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap=""> drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 836bbdc239b6..1d70f6bf2934 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5407,21 +5407,55 @@ static void intel_update_max_cdclk(struct drm_device *dev) dev_priv->max_cdclk_freq = 450000; else dev_priv->max_cdclk_freq = 337500; - } else if (IS_BROADWELL(dev)) { + } else if (IS_BROADWELL(dev)) { + int bios_max_cdclk_freq, max_cdclk_freq; + /* - * FIXME with extra cooling we can allow - * 540 MHz for ULX and 675 Mhz for ULT. - * How can we know if extra cooling is - * available? PCI ID, VTB, something else? + * With extra cooling we can allow 540 MHz for + * ULX and 675 Mhz for ULT. Assume VBIOS/GOP + * passes that information in SWF06. */ - if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT) - dev_priv->max_cdclk_freq = 450000; - else if (IS_BDW_ULX(dev)) - dev_priv->max_cdclk_freq = 450000; - else if (IS_BDW_ULT(dev)) - dev_priv->max_cdclk_freq = 540000; - else - dev_priv->max_cdclk_freq = 675000; + switch (I915_READ(SWF_ILK(0x06)) & 0x3) { + case 0: + bios_max_cdclk_freq = 450000; + break; + case 1: + bios_max_cdclk_freq = 540000; + break; + case 2: + bios_max_cdclk_freq = 337500; + break; + case 3: + bios_max_cdclk_freq = 675000; + break; + } + + if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT) { + if (WARN_ON(bios_max_cdclk_freq != 450000)) + bios_max_cdclk_freq = 450000; + max_cdclk_freq = 450000; + } else if (IS_BDW_ULX(dev_priv)) { + if (WARN_ON(bios_max_cdclk_freq > 540000)) + bios_max_cdclk_freq = 540000; + max_cdclk_freq = 450000; + } else if (IS_BDW_ULT(dev_priv)) { + max_cdclk_freq = 540000; + } else { + max_cdclk_freq = 675000; + } + + if (bios_max_cdclk_freq > max_cdclk_freq) { + DRM_DEBUG_KMS("VBIOS/GOP max cdclk (%d kHz) higher than basic limit (%d kHz), " + "assuming extra cooling is present\n", + bios_max_cdclk_freq, max_cdclk_freq); + max_cdclk_freq = bios_max_cdclk_freq; + } else if (bios_max_cdclk_freq < max_cdclk_freq) { + DRM_DEBUG_KMS("VBIOS/GOP max cdclk (%d kHz) lower than basic limit (%d kHz), " + "ignoring it\n", + bios_max_cdclk_freq, max_cdclk_freq); + } </pre> </blockquote> <pre wrap="">for the reasons mentioned above, i would favor trusting lower values too. regards, Sivakumar </pre> <blockquote type="cite"> <pre wrap="">+ + dev_priv->max_cdclk_freq = max_cdclk_freq; } else if (IS_CHERRYVIEW(dev)) { dev_priv->max_cdclk_freq = 320000; } else if (IS_VALLEYVIEW(dev)) { </pre> </blockquote> </blockquote> <pre wrap=""> </pre> </blockquote> </body> </html>