[Intel-gfx] [RFC PATCH 0/9] drm/i915/spi: discrete graphics internal spi

[Jani Nikula]

On Wed, 17 Feb 2021, "Winkler, Tomas" <tomas.winkler at intel.com> wrote:
>> 
>> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler at intel.com> wrote:
>> > Intel discrete graphic devices have internal spi storage, that holds
>> > firmware and oprom images. The spi device is exposed to the user space
>> > via mtd framework to be accessed during manufacturing.
>> > The device is hardware locked after manufacturing and only read access
>> > is provided.
>> >
>> > The i915 plays role of a multi function device (mfd) and spi device is
>> > exposed as its child device. i915_spi platform driver binds to this
>> > device.
>> >
>> > Because the graphic card may undergo reset at any time and basically
>> > hot unplug all its child devices, this series also provides a fix to
>> > the mtd framework to make the reset graceful.
>> >
>> > Tomas Winkler (9):
>> >   drm/i915/spi: add spi device for discrete graphics
>> >   drm/i915/spi: intel_spi_region map
>> >   drm/i915/spi: add driver for on-die spi device
>> >   drm/i915/spi: implement region enumeration
>> >   drm/i915/spi: implement spi access functions
>> >   drm/i915/spi: spi register with mtd
>> >   drm/i915/spi: mtd: implement access handlers
>> >   drm/i915/spi: serialize spi access
>> >   mtd: use refcount to prevent corruption
>> >
>> >  drivers/gpu/drm/i915/Kconfig             |   3 +
>> >  drivers/gpu/drm/i915/Makefile            |   6 +
>> >  drivers/gpu/drm/i915/i915_drv.c          |   9 +
>> >  drivers/gpu/drm/i915/i915_drv.h          |   4 +
>> >  drivers/gpu/drm/i915/i915_reg.h          |   1 +
>> >  drivers/gpu/drm/i915/spi/intel_spi.c     |  62 +++
>> >  drivers/gpu/drm/i915/spi/intel_spi.h     |  24 +
>> 
>> I'm open to discussion, but after glancing through the series I've got a gut
>> feeling spi/ subdir should be purely about the separate module, and the
>> above two files should be in i915/ directory instead.
>
> Maybe, I don't have strong feelings about that, it is just a decision from which point you want to look at that.

*shrug*

No strong feelings either, and I don't think the decision is carved in
stone. We can move them around later if we want.

Up to you.

BR,
Jani.


>> 
>> As it is, I think it's a bit confusing that spi/ is both about the spi kernel module
>> and a singly .c file that's really part of i915.ko. Perhaps that messes up the
>> conventional descending to subdirs in the kernel build too?
>
> The intention was to make this capsulated from the file system point of view. 
> In general the spi driver could be somewhere in mtd directory, but it doesn't really fit exactly there either.
> I don't have a strong opinion about that, if you do I yield. 
>
>
> Thanks
> Tomas
>

-- 
Jani Nikula, Intel Open Source Graphics Center