Re: [RFC 00/12] i915 init-time configuration.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 13 Feb 2015, Bob Paauwe <bob.j.paauwe@intel.com> wrote:
> Background:
>
> This capability is targeted at deeply embedded appliance like devices
> that make use of Intel integrated graphics.  There are a few use cases
> that are not currently supported by the i915 driver.  For example,
> they may not be running userspace code that is capable of querying and
> setting the various DRM properties and would like them set during the
> driver initialization. Also they may be using a custom firmware bootloader
> that does not include any graphics initialization or VBT information.  
>
> This level of initialization configuration has been available in
> the Intel EMGD kernel driver and a similar level of configurability will
> be expected as designs transition to the i915 driver.
>
> This patch set provides a framework that makes use of ACPI property
> tables containing configuration information.  It also includes some
> examples on how this may be applied to various aspects of the i915
> driver initialization.

The biggest issue I have with this series is the introduction of another
source of configuration in addition to VBT (and to a lesser extent ACPI
OpRegion) without an attempt to abstract them. Information from both
will get used. The mixture is completed in patch 9 that initializes some
of the same data structures as intel_bios.c but without reuse of the
code.

Maybe we need to better abstract our use of the VBT information to begin
with, so that we could plug in additional (complementary or replacement)
sources of the configuration. Offhand, I am not sure if what you propose
as intel_config.c API could be developed into such an abstraction, or if
there's something ready made in kernel we could use.

I do know we already and historically have had problems with the forward
compatibility of the VBT data. It's been getting better, but we need to
avoid the same mistakes. On a related note, I'd really appreciate it if
the specification for your data could be made public.

Oh, one other thing, this thing needs to build with CONFIG_ACPI=n. At
least that's been the case for i915 for a long time.

I'll add some random notes on the patches too.

BR,
Jani.


>
> Series description:
>
> Patch 1 creates the initial framework. It looks up a specific ACPI
> property table and builds lists containing the configuration found
> in that table.  It includes functions that can make use of that 
> configuration information.
>
> Patch 2 adds a function to i915 that provides a unique name for
> each output.  We previously had something similar to this in the
> driver for debug output, it was not be used and removed recently.
>
> Patch 3 is the first example usage.  We check the configuration for
> a CRTC bits-per-pixel value and use that if EDID does not provide
> this.
>
> Patch 4 is an example of using the configuration to specify a 
> default value for the DP panel fitter property.
>
> Patch 5 is an example of using the configuration to specify default
> values for a couple of common connector properties.
>
> Patch 6 modifies the framework slightly to better support the 
> remaining examples.
>
> Patch 7 adds a function to the framework that looks for a 
> workaround section. If found, it builds a list of workarounds that
> can be used in place of of the workarounds hardcoded in the driver.
>
> Patch 8 changes the workaround initialization code to make use
> of the workaround list from the configuration instead of the
> built-in workaround list.
>
> Patch 9 adds functions to the frame work that look for a VBT
> section and parse that information into the driver's VBT structures.
>
> Patch 10 adds an example/test ACPI property table and adds code to
> the frame to build this table into the driver.  This is mainly for
> testing the framework, but may also be useful for truly embedded
> devices as a way to embed the configuration.
>
> Patch 11 adds an example workaround section to the test ACPI property
> table.
>
> Patch 12 add an example VBT section to the test ACPI property table.
>
> Bob Paauwe (12):
>   drm/i915/config: Initial framework
>   drm/i915/config: Introduce intel_output_name
>   drm/i915/config: Add init-time configuration of bits per color.
>   drm/i915/config: Set dp panel fitter property based on init-time
>     config.
>   drm/i915/config: Set general connector properties using config.
>   drm/i915/config: Split out allocation of list nodes.
>   drm/i915/config: Get workaround information from configuration.
>   drm/i915/config: Use workarounds list from configuration.
>   drm/i915/config: Add VBT settings configuration.
>   drm/i915/config: Introduce a test table and code to make use of it.
>   drm/i915/config: Add workaround properties to ACPI table.
>   drm/i915/config: Add ACPI device examples for VBT configuration.
>
>  drivers/gpu/drm/i915/Makefile            |    3 +-
>  drivers/gpu/drm/i915/i915-properties.asl |  340 ++++++++++
>  drivers/gpu/drm/i915/i915-properties.hex |  409 ++++++++++++
>  drivers/gpu/drm/i915/i915_dma.c          |    6 +
>  drivers/gpu/drm/i915/i915_drv.h          |   18 +
>  drivers/gpu/drm/i915/i915_params.c       |    6 +
>  drivers/gpu/drm/i915/intel_config.c      | 1002 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c     |   71 +++
>  drivers/gpu/drm/i915/intel_dp.c          |    8 +-
>  drivers/gpu/drm/i915/intel_drv.h         |   33 +
>  drivers/gpu/drm/i915/intel_modes.c       |    8 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c  |   19 +-
>  12 files changed, 1909 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915-properties.asl
>  create mode 100644 drivers/gpu/drm/i915/i915-properties.hex
>  create mode 100644 drivers/gpu/drm/i915/intel_config.c
>
> -- 
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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