On Fri, 13 Feb 2015 10:08:52 +0200 Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: Thanks Jani for the quick look and comments! > On Fri, 13 Feb 2015, Bob Paauwe <bob.j.paauwe@xxxxxxxxx> 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. Yeah, I struggled with some of this too. The general ID was that if you have VBT, you'd most likely be using it and not this. But if you have firmware that doesn't have VBT, then there needs to be some way to get this information into the driver. The EMGD driver has a configuration structure and tool that that make it fairly easy to set the configuration as it only supports a sub-set of what's in the VBT. Here I tried to keep some of that by making the ACPI property table also be fairly easy to create. The alternative would be to but binary blocks in the ACPI table that correspond to the VBT blocks and send those blocks to the existing code in the driver that parses them. > > 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'll think about this some more and see if I can come up with some type of abstraction. > > 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. I don't see any reason why it couldn't be. Right now it's just a collection of ideas on areas that may be configurable at init-time and thus somewhat ad-hoc. > > 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. You're right, thanks for catching it. I'm pretty sure I had this at one point, but lost it in all the iterations/re-basing. > > 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@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
