[PATCH] CDCLOCK Sanitization continued for SKL

[PATCH] CDCLOCK Sanitization continued for SKL

[Shobhit Kumar]

The cdclock sanitization patch reviewed and merged at -
http://patchwork.freedesktop.org/patch/msgid/1445344992-14658-1-git-send-email-shobhit.kumar@intel.com

made the assumptions that DPLL should not be enabled when pre-os does not enable display and if it does then verify that the cdclock is corectly programmed as well. The BIOS was actually enabling DPLL as well while not following BSPEC sequence and writing cdclk register directly. I was working with BIOS team to correct this and found that due to a WA needed where audio codec will not be enumerated in OS if BIOS did not program the audio verbs which needed PG2 and DPLL enabling. More discussion revealed the following logic - 

1. BIOS puts max cdclk for the platform in CDCLK_CTL. VBIOS/GOP reads that value and then programs cdclk to desired value.
2. It also then sets SWF18 to indicate to the OS that it has enabled display. Used for fastmodeset actually in windows.
3. It also sets SWF06 with this max cdclock(from what bios programmed in CDCLK_CTL) for OS to know.

This patch uses point 2 above while sanitizing the cdclk. We can also update our logic for deciding max cdclock based on SWF06 if pre-os enables else directly from CDCLK_CTL (no pre-os display). That is not part of this patch.

Shobhit Kumar (1):
  drm/i915/skl: While sanitizing cdclock check the SWF18 as well

 drivers/gpu/drm/i915/i915_reg.h      | 3 +++
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 2 files changed, 11 insertions(+)

-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Shobhit Kumar]

SWF18 is set if the display has been intialized by the pre-os. It
also gives what configuration is enabled on which pipe. The DPLL and
CDCLK verification checks can fail as the pre-os does initialize the
DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
then follow through with other DPLL and CDCLK verification.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 3 +++
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ee9481..bd476ff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
 #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
 #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
 
+/* VBIOS flag for display initialized status */
+#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
+
 /* Pipe B */
 #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
 #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 103cacb..0ecb35c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	uint32_t cdctl = I915_READ(CDCLK_CTL);
 	int freq = dev_priv->skl_boot_cdclk;
 
+	/*
+	 * check if the pre-os intialized the display
+	 * There is SWF18 scratchpad register defined which is set by the
+	 * pre-os which can be used by the OS drivers to check the status
+	 */
+	if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
+		goto sanitize;
+
 	/* Is PLL enabled and locked ? */
 	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
 		goto sanitize;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Jani Nikula]

On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> SWF18 is set if the display has been intialized by the pre-os. It
> also gives what configuration is enabled on which pipe. The DPLL and
> CDCLK verification checks can fail as the pre-os does initialize the
> DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
> then follow through with other DPLL and CDCLK verification.

Can we universally trust all bios/gop/bootloader/whatnot to have
initialized this? What if it's not set?

BR,
Jani.



>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ee9481..bd476ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>  #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>  #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>  
> +/* VBIOS flag for display initialized status */
> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
> +
>  /* Pipe B */
>  #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
>  #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 103cacb..0ecb35c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	uint32_t cdctl = I915_READ(CDCLK_CTL);
>  	int freq = dev_priv->skl_boot_cdclk;
>  
> +	/*
> +	 * check if the pre-os intialized the display
> +	 * There is SWF18 scratchpad register defined which is set by the
> +	 * pre-os which can be used by the OS drivers to check the status
> +	 */
> +	if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
> +		goto sanitize;
> +
>  	/* Is PLL enabled and locked ? */
>  	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>  		goto sanitize;

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

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Kumar, Shobhit]

On 11/02/2015 06:40 PM, Jani Nikula wrote:
> On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> SWF18 is set if the display has been intialized by the pre-os. It
>> also gives what configuration is enabled on which pipe. The DPLL and
>> CDCLK verification checks can fail as the pre-os does initialize the
>> DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
>> then follow through with other DPLL and CDCLK verification.
>
> Can we universally trust all bios/gop/bootloader/whatnot to have
> initialized this? What if it's not set?
>

As per my discussion with gop team, this has been enabled in main stream 
for quite sometime including VLV, CHT, BDW, SKL+ and is common for 
GOP/VBIOS across chrome/windows/android. So yes I think we can 
universally trust as of now.

Regards
Shobhit

> BR,
> Jani.
>
>
>
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 9ee9481..bd476ff 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>>   #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>>   #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>>
>> +/* VBIOS flag for display initialized status */
>> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
>> +
>>   /* Pipe B */
>>   #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
>>   #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 103cacb..0ecb35c 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>>   	uint32_t cdctl = I915_READ(CDCLK_CTL);
>>   	int freq = dev_priv->skl_boot_cdclk;
>>
>> +	/*
>> +	 * check if the pre-os intialized the display
>> +	 * There is SWF18 scratchpad register defined which is set by the
>> +	 * pre-os which can be used by the OS drivers to check the status
>> +	 */
>> +	if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
>> +		goto sanitize;
>> +
>>   	/* Is PLL enabled and locked ? */
>>   	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>>   		goto sanitize;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Thulasimani, Sivakumar]

On 11/2/2015 6:49 PM, Kumar, Shobhit wrote:
> On 11/02/2015 06:40 PM, Jani Nikula wrote:
>> On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>>> SWF18 is set if the display has been intialized by the pre-os. It
>>> also gives what configuration is enabled on which pipe. The DPLL and
>>> CDCLK verification checks can fail as the pre-os does initialize the
>>> DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
>>> then follow through with other DPLL and CDCLK verification.
>>
>> Can we universally trust all bios/gop/bootloader/whatnot to have
>> initialized this? What if it's not set?
>>
>
> As per my discussion with gop team, this has been enabled in main 
> stream for quite sometime including VLV, CHT, BDW, SKL+ and is common 
> for GOP/VBIOS across chrome/windows/android. So yes I think we can 
> universally trust as of now.
This has been added since IVB timeframe and should be part of VBT spec. 
but i just encountered
an issue in Android Charging OS where there is no modeset and is using 
the displays
enabled by GOP/VBIOS. This patch might break such expectations.
>
> Regards
> Shobhit
>
>> BR,
>> Jani.
>>
>>
>>
>>>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>>>   2 files changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 9ee9481..bd476ff 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>>>   #define SWF1(i)    (dev_priv->info.display_mmio_offset + 0x71410 + 
>>> (i) * 4)
>>>   #define SWF3(i)    (dev_priv->info.display_mmio_offset + 0x72414 + 
>>> (i) * 4)
>>>
>>> +/* VBIOS flag for display initialized status */
>>> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
>>> +
>>>   /* Pipe B */
>>>   #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000)
>>>   #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008)
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 103cacb..0ecb35c 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct 
>>> drm_i915_private *dev_priv)
>>>       uint32_t cdctl = I915_READ(CDCLK_CTL);
>>>       int freq = dev_priv->skl_boot_cdclk;
>>>
>>> +    /*
>>> +     * check if the pre-os intialized the display
>>> +     * There is SWF18 scratchpad register defined which is set by the
>>> +     * pre-os which can be used by the OS drivers to check the status
>>> +     */
>>> +    if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
>>> +        goto sanitize;
>>> +
>>>       /* Is PLL enabled and locked ? */
>>>       if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>>>           goto sanitize;
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Kumar, Shobhit]

On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote:
>
>
> On 11/2/2015 6:49 PM, Kumar, Shobhit wrote:
>> On 11/02/2015 06:40 PM, Jani Nikula wrote:
>>> On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>>>> SWF18 is set if the display has been intialized by the pre-os. It
>>>> also gives what configuration is enabled on which pipe. The DPLL and
>>>> CDCLK verification checks can fail as the pre-os does initialize the
>>>> DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
>>>> then follow through with other DPLL and CDCLK verification.
>>>
>>> Can we universally trust all bios/gop/bootloader/whatnot to have
>>> initialized this? What if it's not set?
>>>
>>
>> As per my discussion with gop team, this has been enabled in main
>> stream for quite sometime including VLV, CHT, BDW, SKL+ and is common
>> for GOP/VBIOS across chrome/windows/android. So yes I think we can
>> universally trust as of now.
> This has been added since IVB timeframe and should be part of VBT spec.
> but i just encountered
> an issue in Android Charging OS where there is no modeset and is using
> the displays
> enabled by GOP/VBIOS. This patch might break such expectations.

Why would this break anything in Android charging UI. Basically this 
patch only says do cdclock sanitization if display is not enabled by 
pre-os. In this use case it is already enabled by GOP/VBIOS and the 
driver any way expects it to be programmed by pre-os in general. So in 
this scenario, the sanitization logic will not do anything at all 
because SWF18 will be set and CDCLOCK and DPLL will be properly enabled 
already and just return false. Fast-modeset should not be broken by this 
at all.

>>
>> Regards
>> Shobhit
>>
>>> BR,
>>> Jani.
>>>
>>>
>>>
>>>>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>>>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>>>>   2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>> index 9ee9481..bd476ff 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>>>>   #define SWF1(i)    (dev_priv->info.display_mmio_offset + 0x71410 +
>>>> (i) * 4)
>>>>   #define SWF3(i)    (dev_priv->info.display_mmio_offset + 0x72414 +
>>>> (i) * 4)
>>>>
>>>> +/* VBIOS flag for display initialized status */
>>>> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
>>>> +
>>>>   /* Pipe B */
>>>>   #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000)
>>>>   #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008)
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>> index 103cacb..0ecb35c 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct
>>>> drm_i915_private *dev_priv)
>>>>       uint32_t cdctl = I915_READ(CDCLK_CTL);
>>>>       int freq = dev_priv->skl_boot_cdclk;
>>>>
>>>> +    /*
>>>> +     * check if the pre-os intialized the display
>>>> +     * There is SWF18 scratchpad register defined which is set by the
>>>> +     * pre-os which can be used by the OS drivers to check the status
>>>> +     */
>>>> +    if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
>>>> +        goto sanitize;
>>>> +
>>>>       /* Is PLL enabled and locked ? */
>>>>       if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>>>>           goto sanitize;
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Thulasimani, Sivakumar]

On 11/2/2015 11:17 PM, Kumar, Shobhit wrote:
> On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote:
>>
>>
>> On 11/2/2015 6:49 PM, Kumar, Shobhit wrote:
>>> On 11/02/2015 06:40 PM, Jani Nikula wrote:
>>>> On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>>>>> SWF18 is set if the display has been intialized by the pre-os. It
>>>>> also gives what configuration is enabled on which pipe. The DPLL and
>>>>> CDCLK verification checks can fail as the pre-os does initialize the
>>>>> DPLL for Audio codec initialization. So fisrt check if SWF18 is 
>>>>> set and
>>>>> then follow through with other DPLL and CDCLK verification.
>>>>
>>>> Can we universally trust all bios/gop/bootloader/whatnot to have
>>>> initialized this? What if it's not set?
>>>>
>>>
>>> As per my discussion with gop team, this has been enabled in main
>>> stream for quite sometime including VLV, CHT, BDW, SKL+ and is common
>>> for GOP/VBIOS across chrome/windows/android. So yes I think we can
>>> universally trust as of now.
>> This has been added since IVB timeframe and should be part of VBT spec.
>> but i just encountered
>> an issue in Android Charging OS where there is no modeset and is using
>> the displays
>> enabled by GOP/VBIOS. This patch might break such expectations.
>
> Why would this break anything in Android charging UI. Basically this 
> patch only says do cdclock sanitization if display is not enabled by 
> pre-os. In this use case it is already enabled by GOP/VBIOS and the 
> driver any way expects it to be programmed by pre-os in general. So in 
> this scenario, the sanitization logic will not do anything at all 
> because SWF18 will be set and CDCLOCK and DPLL will be properly 
> enabled already and just return false. Fast-modeset should not be 
> broken by this at all.
>
my bad :( should review carefully when sitting late night.
>>>
>>> Regards
>>> Shobhit
>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>
>>>>>
>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>>>>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>>>>>   2 files changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>>> index 9ee9481..bd476ff 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>>>>>   #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 +
>>>>> (i) * 4)
>>>>>   #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 +
>>>>> (i) * 4)
>>>>>
>>>>> +/* VBIOS flag for display initialized status */
>>>>> +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060)
>>>>> +
>>>>>   /* Pipe B */
>>>>>   #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000)
>>>>>   #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008)
>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>>> index 103cacb..0ecb35c 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct
>>>>> drm_i915_private *dev_priv)
>>>>>       uint32_t cdctl = I915_READ(CDCLK_CTL);
>>>>>       int freq = dev_priv->skl_boot_cdclk;
>>>>>
>>>>> +    /*
>>>>> +     * check if the pre-os intialized the display
>>>>> +     * There is SWF18 scratchpad register defined which is set by 
>>>>> the
>>>>> +     * pre-os which can be used by the OS drivers to check the 
>>>>> status
>>>>> +     */
>>>>> +    if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
>>>>> +        goto sanitize;
>>>>> +
>>>>>       /* Is PLL enabled and locked ? */
>>>>>       if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & 
>>>>> LCPLL_PLL_LOCK)))
>>>>>           goto sanitize;
>>>>
can you share bit more details on when GOP/VBIOS sets DPLL but does not 
enable
display ? (atleast that is what i understood from the commit message)

regards,
Sivakumar
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Kumar, Shobhit]

On 11/02/2015 11:48 PM, Thulasimani, Sivakumar wrote:
>
>
> On 11/2/2015 11:17 PM, Kumar, Shobhit wrote:
>> On 11/02/2015 10:07 PM, Thulasimani, Sivakumar wrote:
>>>
>>>
>>> On 11/2/2015 6:49 PM, Kumar, Shobhit wrote:
>>>> On 11/02/2015 06:40 PM, Jani Nikula wrote:
>>>>> On Mon, 02 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>>>>>> SWF18 is set if the display has been intialized by the pre-os. It
>>>>>> also gives what configuration is enabled on which pipe. The DPLL and
>>>>>> CDCLK verification checks can fail as the pre-os does initialize the
>>>>>> DPLL for Audio codec initialization. So fisrt check if SWF18 is
>>>>>> set and
>>>>>> then follow through with other DPLL and CDCLK verification.
>>>>>
>>>>> Can we universally trust all bios/gop/bootloader/whatnot to have
>>>>> initialized this? What if it's not set?
>>>>>
>>>>
>>>> As per my discussion with gop team, this has been enabled in main
>>>> stream for quite sometime including VLV, CHT, BDW, SKL+ and is common
>>>> for GOP/VBIOS across chrome/windows/android. So yes I think we can
>>>> universally trust as of now.
>>> This has been added since IVB timeframe and should be part of VBT spec.
>>> but i just encountered
>>> an issue in Android Charging OS where there is no modeset and is using
>>> the displays
>>> enabled by GOP/VBIOS. This patch might break such expectations.
>>
>> Why would this break anything in Android charging UI. Basically this
>> patch only says do cdclock sanitization if display is not enabled by
>> pre-os. In this use case it is already enabled by GOP/VBIOS and the
>> driver any way expects it to be programmed by pre-os in general. So in
>> this scenario, the sanitization logic will not do anything at all
>> because SWF18 will be set and CDCLOCK and DPLL will be properly
>> enabled already and just return false. Fast-modeset should not be
>> broken by this at all.
>>
> my bad :( should review carefully when sitting late night.
>>>>
>>>> Regards
>>>> Shobhit
>>>>
>>>>> BR,
>>>>> Jani.
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>>>>>>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>>>>>>   2 files changed, 11 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> index 9ee9481..bd476ff 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>>>>>>   #define SWF1(i) (dev_priv->info.display_mmio_offset + 0x71410 +
>>>>>> (i) * 4)
>>>>>>   #define SWF3(i) (dev_priv->info.display_mmio_offset + 0x72414 +
>>>>>> (i) * 4)
>>>>>>
>>>>>> +/* VBIOS flag for display initialized status */
>>>>>> +#define GEN6_SWF18 (dev_priv->info.display_mmio_offset + 0x4F060)
>>>>>> +
>>>>>>   /* Pipe B */
>>>>>>   #define _PIPEBDSL (dev_priv->info.display_mmio_offset + 0x71000)
>>>>>>   #define _PIPEBCONF (dev_priv->info.display_mmio_offset + 0x71008)
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>>>> index 103cacb..0ecb35c 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>>> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct
>>>>>> drm_i915_private *dev_priv)
>>>>>>       uint32_t cdctl = I915_READ(CDCLK_CTL);
>>>>>>       int freq = dev_priv->skl_boot_cdclk;
>>>>>>
>>>>>> +    /*
>>>>>> +     * check if the pre-os intialized the display
>>>>>> +     * There is SWF18 scratchpad register defined which is set by
>>>>>> the
>>>>>> +     * pre-os which can be used by the OS drivers to check the
>>>>>> status
>>>>>> +     */
>>>>>> +    if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
>>>>>> +        goto sanitize;
>>>>>> +
>>>>>>       /* Is PLL enabled and locked ? */
>>>>>>       if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 &
>>>>>> LCPLL_PLL_LOCK)))
>>>>>>           goto sanitize;
>>>>>
> can you share bit more details on when GOP/VBIOS sets DPLL but does not
> enable
> display ? (atleast that is what i understood from the commit message)
>

This patch has to be looked in continuation with the patch for sanitize 
cdclk which I gave in cover letter to get the full context. Basic point 
here is that GOP/VBIOS does not enable DPLL as that is not even loaded 
in certain use cases. It is the FSP/BIOS that loads the GOP and executes 
it to enable display. In this case, FSP does not load GOP/VBIOS but 
still programs DPLL itself for some audio codec initialization. The 
sanitize cdclock function has to decide when to do the sanitization. 
That was based on DPLL check and CDCLK verification, but both are done 
by BIOS and hence will assume everything is fine when it is not. That is 
why additional check of SWF18 is added.

I can clarify a bit more in the commit message.

Regards
Shobhit

> regards,
> Sivakumar
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] CDCLOCK Sanitization continued for SKL

[Kumar, Shobhit]

On 11/02/2015 05:25 PM, Shobhit Kumar wrote:
> The cdclock sanitization patch reviewed and merged at -
> http://patchwork.freedesktop.org/patch/msgid/1445344992-14658-1-git-send-email-shobhit.kumar@intel.com
>
> made the assumptions that DPLL should not be enabled when pre-os does not enable display and if it does then verify that the cdclock is corectly programmed as well. The BIOS was actually enabling DPLL as well while not following BSPEC sequence and writing cdclk register directly. I was working with BIOS team to correct this and found that due to a WA needed where audio codec will not be enumerated in OS if BIOS did not program the audio verbs which needed PG2 and DPLL enabling. More discussion revealed the following logic -
>
> 1. BIOS puts max cdclk for the platform in CDCLK_CTL. VBIOS/GOP reads that value and then programs cdclk to desired value.
> 2. It also then sets SWF18 to indicate to the OS that it has enabled display. Used for fastmodeset actually in windows.
> 3. It also sets SWF06 with this max cdclock(from what bios programmed in CDCLK_CTL) for OS to know.
>
> This patch uses point 2 above while sanitizing the cdclk. We can also update our logic for deciding max cdclock based on SWF06 if pre-os enables else directly from CDCLK_CTL (no pre-os display). That is not part of this patch.
>

One open that comes up now is that with the sanitize implementation, 
cdclock on SKL is at max 675 MHz when pre-os does not enable display. 
Given that we do not have dynamic cdclk support yet, this will burn more 
power in general on lower resolution. Most of the resolutions can be 
supported on SKL with 337.5 MHz cdclk including 4k@30. IIRC, Ville you 
were doing something on dynamic cdclk. Is there a plan on supporting that.

Regards
Shobhit

> Shobhit Kumar (1):
>    drm/i915/skl: While sanitizing cdclock check the SWF18 as well
>
>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>   2 files changed, 11 insertions(+)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Kumar, Shobhit]

Hi Ville,

On 11/02/2015 05:25 PM, Shobhit Kumar wrote:
> SWF18 is set if the display has been intialized by the pre-os. It
> also gives what configuration is enabled on which pipe. The DPLL and
> CDCLK verification checks can fail as the pre-os does initialize the
> DPLL for Audio codec initialization. So fisrt check if SWF18 is set and
> then follow through with other DPLL and CDCLK verification.
>

Can you have a quick look at this one.


> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ee9481..bd476ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>   #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>   #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>
> +/* VBIOS flag for display initialized status */
> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
> +
>   /* Pipe B */
>   #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
>   #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 103cacb..0ecb35c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>   	uint32_t cdctl = I915_READ(CDCLK_CTL);
>   	int freq = dev_priv->skl_boot_cdclk;
>
> +	/*
> +	 * check if the pre-os intialized the display
> +	 * There is SWF18 scratchpad register defined which is set by the
> +	 * pre-os which can be used by the OS drivers to check the status
> +	 */
> +	if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
> +		goto sanitize;
> +
>   	/* Is PLL enabled and locked ? */
>   	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>   		goto sanitize;
>

Regards
Shobhit
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v2] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Shobhit Kumar]

SWF18 is set if the display has been intialized by the pre-os. It also
gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
enables DPLL for integrated audio codec related programming.
So fisrt check if SWF18 is set and then follow through with other DPLL
and CDCLK verification. If not set then for sure we need to sanitize the
cdclock.

v2: Update the commit message for clarity (Siva)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 3 +++
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ee9481..bd476ff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
 #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
 #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
 
+/* VBIOS flag for display initialized status */
+#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
+
 /* Pipe B */
 #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
 #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 103cacb..0ecb35c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	uint32_t cdctl = I915_READ(CDCLK_CTL);
 	int freq = dev_priv->skl_boot_cdclk;
 
+	/*
+	 * check if the pre-os intialized the display
+	 * There is SWF18 scratchpad register defined which is set by the
+	 * pre-os which can be used by the OS drivers to check the status
+	 */
+	if ((I915_READ(GEN6_SWF18) & 0x00FFFF) == 0)
+		goto sanitize;
+
 	/* Is PLL enabled and locked ? */
 	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
 		goto sanitize;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v3] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Shobhit Kumar]

SWF18 is set if the display has been intialized by the pre-os. It also
gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
enables DPLL for integrated audio codec related programming.
So fisrt check if SWF18 is set and then follow through with other DPLL
and CDCLK verification. If not set then for sure we need to sanitize the
cdclock.

v2: Update the commit message for clarity (Siva)
v3: Correct the mask to check for bits[23:0] instead of only bits[16:0].
    Had missed checking for PIPE C altogether. Remaining are reserved (Siva)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 3 +++
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ee9481..bd476ff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
 #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
 #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
 
+/* VBIOS flag for display initialized status */
+#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
+
 /* Pipe B */
 #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
 #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 103cacb..81668b0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	uint32_t cdctl = I915_READ(CDCLK_CTL);
 	int freq = dev_priv->skl_boot_cdclk;
 
+	/*
+	 * check if the pre-os intialized the display
+	 * There is SWF18 scratchpad register defined which is set by the
+	 * pre-os which can be used by the OS drivers to check the status
+	 */
+	if ((I915_READ(GEN6_SWF18) & 0x00FFFFFF) == 0)
+		goto sanitize;
+
 	/* Is PLL enabled and locked ? */
 	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
 		goto sanitize;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v3] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Thulasimani, Sivakumar]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On 11/5/2015 3:05 PM, Shobhit Kumar wrote:
> SWF18 is set if the display has been intialized by the pre-os. It also
> gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
> the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
> enables DPLL for integrated audio codec related programming.
> So fisrt check if SWF18 is set and then follow through with other DPLL
> and CDCLK verification. If not set then for sure we need to sanitize the
> cdclock.
>
> v2: Update the commit message for clarity (Siva)
> v3: Correct the mask to check for bits[23:0] instead of only bits[16:0].
>      Had missed checking for PIPE C altogether. Remaining are reserved (Siva)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>   drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ee9481..bd476ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>   #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>   #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>   
> +/* VBIOS flag for display initialized status */
> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)
> +
>   /* Pipe B */
>   #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
>   #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 103cacb..81668b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>   	uint32_t cdctl = I915_READ(CDCLK_CTL);
>   	int freq = dev_priv->skl_boot_cdclk;
>   
> +	/*
> +	 * check if the pre-os intialized the display
> +	 * There is SWF18 scratchpad register defined which is set by the
> +	 * pre-os which can be used by the OS drivers to check the status
> +	 */
> +	if ((I915_READ(GEN6_SWF18) & 0x00FFFFFF) == 0)
> +		goto sanitize;
> +
>   	/* Is PLL enabled and locked ? */
>   	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>   		goto sanitize;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v3] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Ville Syrjälä]

On Thu, Nov 05, 2015 at 03:05:58PM +0530, Shobhit Kumar wrote:
> SWF18 is set if the display has been intialized by the pre-os. It also
> gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
> the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
> enables DPLL for integrated audio codec related programming.
> So fisrt check if SWF18 is set and then follow through with other DPLL
> and CDCLK verification. If not set then for sure we need to sanitize the
> cdclock.
> 
> v2: Update the commit message for clarity (Siva)
> v3: Correct the mask to check for bits[23:0] instead of only bits[16:0].
>     Had missed checking for PIPE C altogether. Remaining are reserved (Siva)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 3 +++
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ee9481..bd476ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5006,6 +5006,9 @@ enum skl_disp_power_wells {
>  #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>  #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
>  
> +/* VBIOS flag for display initialized status */
> +#define GEN6_SWF18  (dev_priv->info.display_mmio_offset + 0x4F060)

Could you steal the ILK_SWF() register define from my patch at?
http://lists.freedesktop.org/archives/intel-gfx/2015-November/079480.html
Would avoid a bit of churn at least.

Otherwise this seems reasonable enough to me.

> +
>  /* Pipe B */
>  #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
>  #define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 103cacb..81668b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	uint32_t cdctl = I915_READ(CDCLK_CTL);
>  	int freq = dev_priv->skl_boot_cdclk;
>  
> +	/*
> +	 * check if the pre-os intialized the display
> +	 * There is SWF18 scratchpad register defined which is set by the
> +	 * pre-os which can be used by the OS drivers to check the status
> +	 */
> +	if ((I915_READ(GEN6_SWF18) & 0x00FFFFFF) == 0)
> +		goto sanitize;
> +
>  	/* Is PLL enabled and locked ? */
>  	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>  		goto sanitize;
> -- 
> 2.4.3

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v4] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Shobhit Kumar]

SWF18 is set if the display has been intialized by the pre-os. It also
gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
enables DPLL for integrated audio codec related programming.
So fisrt check if SWF18 is set and then follow through with other DPLL
and CDCLK verification. If not set then for sure we need to sanitize the
cdclock.

v2: Update the commit message for clarity (Siva)
v3: Correct the mask to check for bits[23:0] instead of only bits[16:0].
    Had missed checking for PIPE C altogether. Remaining are reserved (Siva)
v4: Use ILK_SWF macro for SWF register definitions. Taken from Ville's patch
    http://lists.freedesktop.org/archives/intel-gfx/2015-November/079480.html

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ee9481..e8f1d42 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5005,6 +5005,7 @@ enum skl_disp_power_wells {
 #define SWF0(i)	(dev_priv->info.display_mmio_offset + 0x70410 + (i) * 4)
 #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
 #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
+#define SWF_ILK(i)	(0x4F000 + (i) * 4)
 
 /* Pipe B */
 #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 103cacb..512747a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	uint32_t cdctl = I915_READ(CDCLK_CTL);
 	int freq = dev_priv->skl_boot_cdclk;
 
+	/*
+	 * check if the pre-os intialized the display
+	 * There is SWF18 scratchpad register defined which is set by the
+	 * pre-os which can be used by the OS drivers to check the status
+	 */
+	if ((I915_READ(SWF_ILK(0x18)) & 0x00FFFFFF) == 0)
+		goto sanitize;
+
 	/* Is PLL enabled and locked ? */
 	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
 		goto sanitize;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4] drm/i915/skl: While sanitizing cdclock check the SWF18 as well

[Jani Nikula]

On Thu, 05 Nov 2015, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> SWF18 is set if the display has been intialized by the pre-os. It also
> gives what configuration is enabled on which pipe. In skl_sanitize_cdclk,
> the DPLL sanity check can pass even if GOP/VBIOS is not loaded as BIOS
> enables DPLL for integrated audio codec related programming.
> So fisrt check if SWF18 is set and then follow through with other DPLL
> and CDCLK verification. If not set then for sure we need to sanitize the
> cdclock.
>
> v2: Update the commit message for clarity (Siva)
> v3: Correct the mask to check for bits[23:0] instead of only bits[16:0].
>     Had missed checking for PIPE C altogether. Remaining are reserved (Siva)
> v4: Use ILK_SWF macro for SWF register definitions. Taken from Ville's patch
>     http://lists.freedesktop.org/archives/intel-gfx/2015-November/079480.html
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Pushed to drm-intel-next-queued, thanks for the patch and review.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ee9481..e8f1d42 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5005,6 +5005,7 @@ enum skl_disp_power_wells {
>  #define SWF0(i)	(dev_priv->info.display_mmio_offset + 0x70410 + (i) * 4)
>  #define SWF1(i)	(dev_priv->info.display_mmio_offset + 0x71410 + (i) * 4)
>  #define SWF3(i)	(dev_priv->info.display_mmio_offset + 0x72414 + (i) * 4)
> +#define SWF_ILK(i)	(0x4F000 + (i) * 4)
>  
>  /* Pipe B */
>  #define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 103cacb..512747a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5761,6 +5761,14 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	uint32_t cdctl = I915_READ(CDCLK_CTL);
>  	int freq = dev_priv->skl_boot_cdclk;
>  
> +	/*
> +	 * check if the pre-os intialized the display
> +	 * There is SWF18 scratchpad register defined which is set by the
> +	 * pre-os which can be used by the OS drivers to check the status
> +	 */
> +	if ((I915_READ(SWF_ILK(0x18)) & 0x00FFFFFF) == 0)
> +		goto sanitize;
> +
>  	/* Is PLL enabled and locked ? */
>  	if (!((lcpll1 & LCPLL_PLL_ENABLE) && (lcpll1 & LCPLL_PLL_LOCK)))
>  		goto sanitize;

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