All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/8] Add a mfd IPUv3 driver
Date: Tue, 01 Mar 2011 10:27:49 +0000	[thread overview]
Message-ID: <201103011127.51605.arnd@arndb.de> (raw)
In-Reply-To: <20110301091544.GK29521@pengutronix.de>

On Tuesday 01 March 2011, Sascha Hauer wrote:

> When turning this into a kms driver moving the source code will be the
> smallest problem I guess.
> I have no preference where to put this code. First it was in
> drivers/mfd/ and it felt wrong because there seems to be too much code
> in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
> to be wrong because this code will probably support cameras which belong
> to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
> happily put it there.
> What directory do you suggest? drivers/gpu/ or some subdirectory
> (drm/vga)?

I'd suggest a subdirectory of drivers/gpu/, e.g.
drivers/gpu/embedded/imx-ipu/. Alan is currently adding a driver
for the Intel GMA500, and there are others (TI, ST-Ericsson, ...)
that fit in a similar category of complex graphics subsystems
without an actual GPU core. I think they should all go to the same
place.

> > The interrupt logic needs some comments. What are you trying to achieve here?
> 
> The IPU has 463 status bits which can trigger an interrupt. Most
> of them are associated to channels, some are general interrupts. My
> problem is that I don't know how the interrupts will be used by drivers
> later. Most drivers will probably use only one interrupt but others
> will be interested in a larger group of interrupts. So I decided to
> use a bitmap allowing each driver to register for groups of event
> it is interested in.

Ok, I see. So you essentially have a huge nested interrupt controller
and try to be clever about the possible use cases, which may be the
right choice, but apparently you don't know that yet because not
all the drivers have been written at this point.

Taking one step back from this, have you considered making this
a regular interrupt controller? That would make the client drivers
more standard -- you could define the interrupt numbers as resources
of a platform device or in the device tree, for instance.
The cost might be more complex code, e.g. when a device requires
many interrupts, but I think it will be at least as efficient
at run-time, and less surprising for readers and authors of
client drivers.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/8] Add a mfd IPUv3 driver
Date: Tue, 1 Mar 2011 11:27:49 +0100	[thread overview]
Message-ID: <201103011127.51605.arnd@arndb.de> (raw)
In-Reply-To: <20110301091544.GK29521@pengutronix.de>

On Tuesday 01 March 2011, Sascha Hauer wrote:

> When turning this into a kms driver moving the source code will be the
> smallest problem I guess.
> I have no preference where to put this code. First it was in
> drivers/mfd/ and it felt wrong because there seems to be too much code
> in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
> to be wrong because this code will probably support cameras which belong
> to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
> happily put it there.
> What directory do you suggest? drivers/gpu/ or some subdirectory
> (drm/vga)?

I'd suggest a subdirectory of drivers/gpu/, e.g.
drivers/gpu/embedded/imx-ipu/. Alan is currently adding a driver
for the Intel GMA500, and there are others (TI, ST-Ericsson, ...)
that fit in a similar category of complex graphics subsystems
without an actual GPU core. I think they should all go to the same
place.

> > The interrupt logic needs some comments. What are you trying to achieve here?
> 
> The IPU has 463 status bits which can trigger an interrupt. Most
> of them are associated to channels, some are general interrupts. My
> problem is that I don't know how the interrupts will be used by drivers
> later. Most drivers will probably use only one interrupt but others
> will be interested in a larger group of interrupts. So I decided to
> use a bitmap allowing each driver to register for groups of event
> it is interested in.

Ok, I see. So you essentially have a huge nested interrupt controller
and try to be clever about the possible use cases, which may be the
right choice, but apparently you don't know that yet because not
all the drivers have been written at this point.

Taking one step back from this, have you considered making this
a regular interrupt controller? That would make the client drivers
more standard -- you could define the interrupt numbers as resources
of a platform device or in the device tree, for instance.
The cost might be more complex code, e.g. when a device requires
many interrupts, but I think it will be at least as efficient
at run-time, and less surprising for readers and authors of
client drivers.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org,
	Paul Mundt <lethal@linux-sh.org>,
	linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
	Samuel Ortiz <sameo@linux.intel.com>,
	Alan Cox <alan@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 3/8] Add a mfd IPUv3 driver
Date: Tue, 1 Mar 2011 11:27:49 +0100	[thread overview]
Message-ID: <201103011127.51605.arnd@arndb.de> (raw)
In-Reply-To: <20110301091544.GK29521@pengutronix.de>

On Tuesday 01 March 2011, Sascha Hauer wrote:

> When turning this into a kms driver moving the source code will be the
> smallest problem I guess.
> I have no preference where to put this code. First it was in
> drivers/mfd/ and it felt wrong because there seems to be too much code
> in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
> to be wrong because this code will probably support cameras which belong
> to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
> happily put it there.
> What directory do you suggest? drivers/gpu/ or some subdirectory
> (drm/vga)?

I'd suggest a subdirectory of drivers/gpu/, e.g.
drivers/gpu/embedded/imx-ipu/. Alan is currently adding a driver
for the Intel GMA500, and there are others (TI, ST-Ericsson, ...)
that fit in a similar category of complex graphics subsystems
without an actual GPU core. I think they should all go to the same
place.

> > The interrupt logic needs some comments. What are you trying to achieve here?
> 
> The IPU has 463 status bits which can trigger an interrupt. Most
> of them are associated to channels, some are general interrupts. My
> problem is that I don't know how the interrupts will be used by drivers
> later. Most drivers will probably use only one interrupt but others
> will be interested in a larger group of interrupts. So I decided to
> use a bitmap allowing each driver to register for groups of event
> it is interested in.

Ok, I see. So you essentially have a huge nested interrupt controller
and try to be clever about the possible use cases, which may be the
right choice, but apparently you don't know that yet because not
all the drivers have been written at this point.

Taking one step back from this, have you considered making this
a regular interrupt controller? That would make the client drivers
more standard -- you could define the interrupt numbers as resources
of a platform device or in the device tree, for instance.
The cost might be more complex code, e.g. when a device requires
many interrupts, but I think it will be at least as efficient
at run-time, and less surprising for readers and authors of
client drivers.

	Arnd

  reply	other threads:[~2011-03-01 10:27 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-28 10:00 [PATCH] i.MX51 Framebuffer support Sascha Hauer
2011-02-28 10:00 ` [PATCH 1/8] fb: export fb mode db table Sascha Hauer
2011-02-28 10:00   ` Sascha Hauer
2011-02-28 10:00   ` Sascha Hauer
2011-02-28 10:00 ` [PATCH 2/8] ARM i.MX51: setup mipi Sascha Hauer
2011-02-28 10:00 ` [PATCH 3/8] Add a mfd IPUv3 driver Sascha Hauer
2011-02-28 10:00   ` Sascha Hauer
2011-02-28 17:11   ` Arnd Bergmann
2011-02-28 17:11     ` Arnd Bergmann
2011-02-28 17:11     ` Arnd Bergmann
2011-03-01  9:15     ` Sascha Hauer
2011-03-01  9:15       ` Sascha Hauer
2011-03-01  9:15       ` Sascha Hauer
2011-03-01 10:27       ` Arnd Bergmann [this message]
2011-03-01 10:27         ` Arnd Bergmann
2011-03-01 10:27         ` Arnd Bergmann
2011-03-01 11:12         ` Sascha Hauer
2011-03-01 11:12           ` Sascha Hauer
2011-03-01 11:12           ` Sascha Hauer
2011-03-01 11:43           ` Arnd Bergmann
2011-03-01 11:43             ` Arnd Bergmann
2011-03-01 11:43             ` Arnd Bergmann
2011-03-01 14:09             ` Thomas Gleixner
2011-03-01 14:09               ` Thomas Gleixner
2011-03-01 14:09               ` Thomas Gleixner
2011-02-28 18:33   ` Thomas Gleixner
2011-02-28 18:33     ` Thomas Gleixner
2011-02-28 18:33     ` Thomas Gleixner
2011-03-01  9:39     ` Sascha Hauer
2011-03-01  9:39       ` Sascha Hauer
2011-03-01  9:39       ` Sascha Hauer
2011-03-01 10:00       ` Thomas Gleixner
2011-03-01 10:00         ` Thomas Gleixner
2011-03-01 10:00         ` Thomas Gleixner
2011-03-01 11:31         ` Sascha Hauer
2011-03-01 11:31           ` Sascha Hauer
2011-03-01 11:31           ` Sascha Hauer
2011-02-28 10:00 ` [PATCH 4/8] Add i.MX5 framebuffer driver Sascha Hauer
2011-02-28 10:00   ` Sascha Hauer
2011-02-28 10:00   ` Sascha Hauer
2011-02-28 10:00 ` [PATCH 5/8] ARM i.MX51: Add IPU device support Sascha Hauer
2011-02-28 10:00 ` [PATCH 6/8] ARM i.MX5: Allow to increase max zone order Sascha Hauer
2011-02-28 10:04   ` Russell King - ARM Linux
2011-02-28 10:18     ` Sascha Hauer
2011-02-28 10:00 ` [PATCH 7/8] ARM i.MX5: increase dma consistent size for IPU support Sascha Hauer
2011-02-28 10:00 ` [PATCH 8/8] ARM i.MX51 babbage: Add framebuffer support Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201103011127.51605.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.