Re: [PATCH 1/3] drm/i915: Verify address field of PCBR register.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sun, Dec 08, 2013 at 01:52:35PM +0530, deepak.s@intel.com wrote:
> From: Deepak S <deepak.s@intel.com>
> 
> On VLV the PCBR register has other bits besides the pcbr address
> field. Verify only address field setup by BIOS to make sure we don't
> misinterpret the PCBR setup.
> 
> Signed-off-by: Deepak S <deepak.s@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e6d98fe..2e1340f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4036,7 +4036,12 @@ static void valleyview_setup_pctx(struct drm_device *dev)
>  	int pctx_size = 24*1024;
>  
>  	pcbr = I915_READ(VLV_PCBR);
> -	if (pcbr) {
> +
> +	/* PCBR Format: Bits 31:12 - Base address of Process Context

It's called power context, not process context.

> +	 * 		Bits 11:1 - Reserved

These are all 0

> +	 * 		Bit 0 - PCBR Lock

And if this is 1, then we can't change the address anyway, so I don't
think this patch makes much sense.

Have you actually seen cases where the BIOS would be buggy enough to
lock the power context addres at 0? If so, we should just scream and
run away instead of blindly trying to write a new address to PCBR and
pretending that things are fine after that.

> +	 * Check only address field if already setup by BIOS */
> +	if (pcbr >> 12) {
>  		/* BIOS set it up already, grab the pre-alloc'd space */
>  		int pcbr_offset;
>  
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC