Re: [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present

From: Lukas Wunner <lukas@wunner.de>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	ankitprasad.r.sharma@intel.com, intel-gfx@lists.freedesktop.org,
	akash.goel@intel.com, shashidhar.hiremath@intel.com
Subject: Re: [PATCH 10/10] drm/i915: Disable use of stolen area by User when Intel RST is present
Date: Thu, 11 Feb 2016 20:08:19 +0100	[thread overview]
Message-ID: <20160211190819.GA15110@wunner.de> (raw)
In-Reply-To: <20160204164317.GA13773@wunner.de>

Hi Ankitprasad,

On Thu, Feb 04, 2016 at 05:43:17PM +0100, Lukas Wunner wrote:
> On Thu, Feb 04, 2016 at 04:05:04PM +0000, Chris Wilson wrote:
> > We could #define INTEL_RAPID_START "INT3392" for
> > if (IS_ENABLED(CONFIG_SUSPEND) && acpi_dev_present(INTEL_RAPID_START))
> > but Anki wanted to keep the acpi details themselves out of stolen (hence
> > the current split).
> 
> Less code is almost always better, it's more work for a reader to
> follow the logic if things are split across multiple files.
> 
> If you absolutely positively want to keep the current split,
> the "static const struct acpi_device_id irst_ids[]" data structure
> should be replaced by a "static const char*" in order to not waste
> memory.

As I've learned the hard way yesterday, acpi_dev_present() is undefined
if the kernel is compiled without CONFIG_ACPI, so you made the right
call splitting that out into intel_acpi.c and my suggested simplification
was wrong. Sorry for the noise. :-/

I'd still suggest to invoke acpi_dev_present() with the string literal
"INT3392" in intel_acpi.c:intel_detect_acpi_rst() though, or at least
replace "static const struct acpi_device_id irst_ids[]" with
"static const char*".

Best regards,

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