* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 5:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929162814.GE4081@lukather>
[-- Attachment #1: Type: text/plain, Size: 6789 bytes --]
On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote:
> > > On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote:
> > > > On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote:
> > > > > On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote:
> > > > [...]
> > > > > > simplefb doesn't deal at all with hardware details. It simply uses what
> > > > > > firmware has set up, which is the only reason why it will work for many
> > > > > > people. What is passed in via its device tree node is the minimum amount
> > > > > > of information needed to draw something into the framebuffer. Also note
> > > > > > that the simplefb device tree node is not statically added to a DTS file
> > > > > > but needs to be dynamically generated by firmware at runtime.
> > > > >
> > > > > Which makes the whole even simpler, since the firmware already knows
> > > > > all about which clocks it had to enable.
> > > >
> > > > It makes things very complicated in the firmware because it now needs to
> > > > be able to generate DTB content that we would otherwise be able to do
> > > > much easier with a text editor.
> > >
> > > Didn't you just say that it was dynamically generated at runtime? So
> > > we can just ignore the "text editor" case.
> >
> > Perhaps read the sentence again. I said "that we would *otherwise* be
> > able to do much easier with a text editor.".
> >
> > My point remains that there shouldn't be a need to generate DTB content
> > of this complexity at all.
> >
> > > > Why do you think what I proposed isn't going to work or be reliable?
> > > > I don't see any arguments in the thread that would imply that.
> > >
> > > The fact that it broke in the first place?
> >
> > That's exactly the point. And it's going to break again and again as
> > simplefb is extended with new things. Generally DT bindings should be
> > backwards compatible. When extended they should provide a way to fall
> > back to a reasonable default. There's simply no way you can do that
> > with simplefb.
>
> This one *is* backward compatible. It doesn't even change the simplefb
> behaviour, compared to what it was doing before, if you don't have
> this clocks property. What can be a more reasonable default that the
> way it used to behave?
My point is that if we decide not to consider all resources handled by
firmware then we need to go all the way. At that point you start having
problems as evidenced by the Snow example.
> > What happened in the Snow example is that regulators that were
> > previously on would all of a sudden be automatically disabled on boot
> > because there was now a driver that registered them with a generic
> > framework.
> >
> > The same thing is going to happen with simplefb for your device. If you
> > later realize that you need a regulator to keep the panel going, you'll
> > have to add code to your firmware to populate the corresponding
> > properties, otherwise the regulator will end up unused and will be
> > automatically disabled. At the same time you're going to break upstream
> > for all users of your old firmware because it doesn't add that property
> > yet.
> >
> > And the same will continue to happen for every new type of resource
> > you're going to add.
>
> Sure, we can add any resources we will need. Regulators, reset lines,
> pm domains, allocated memory, but I'm not really sure this is what you
> want, right?
No it's not what I want. *You* want to add resource management to this
driver. What I'm saying is that if we start adding clocks then we can no
longer say no to resets or regulators or power domains either.
> > > > > You know that you are going to call that for regulator, reset, power
> > > > > domains, just as you would have needed to with the proper API, unless
> > > > > that with this kind of solution, you would have to modify *every*
> > > > > framework that might interact with any resource involved in getting
> > > > > simplefb running?
> > > >
> > > > We have to add handling for every kind of resource either way. Also if
> > > > this evolves into a common pattern we can easily wrap it up in a single
> > > > function call.
> > >
> > > Unless that in one case, we already have everything needed to handle
> > > everything properly, and in another, you keep hacking more and more
> > > into the involved frameworks.
> >
> > This is a fundamental issue that we are facing and I'm trying to come up
> > with a solution that is future-proof and will work for drivers other
> > than simplefb.
> >
> > Just because we currently lack this functionality doesn't make it a hack
> > trying to add it.
>
> Or maybe it's just showing that the trend to rely more and more on the
> firmware is a bad idea?
If you think it's a bad idea to rely on firmware then you shouldn't be
using simplefb in the first place.
> I really start to consider adding a sunxi-uboot-fb, that would just
> duplicate the source code of simplefb but also taking the
> clocks. Somehow, I feel like it will be easier (and definitely less of
> a hack than using the generic common clock API).
You're not getting it are you? What makes you think sunxi-uboot-fb is
going to be any different? This isn't about a name.
> > > > > Plus, speaking more specifically about the clocks, that won't prevent
> > > > > your clock to be shut down as a side effect of a later clk_disable
> > > > > call from another driver.
> > > >
> > > > If we need to prevent that, then that's something that could be fixed,
> > > > too.
> > >
> > > See, you keep hacking it more...
> >
> > If you don't see this as a problem that exists beyond simplefb then I
> > would of course not expect you to think that anything needs fixing.
>
> Then please point me to something else that needs fixing, so that I
> can see the whole picture.
We've been over this before. I'm tired of having to repeat myself.
> > > > so hopefully concurrent users aren't the problem because that would
> > > > cause the real driver to break too.
> > > >
> > > > Also note that if some other driver could call clk_disable() it could
> > > > just as easily call clk_set_rate() and break simplefb.
> > >
> > > This is not true, see my other reply.
> >
> > If you mean that you could register a clock notifier to prevent clock
> > changes, then that's going to lead to other drivers failing since they
> > can now no longer set the rate that they need.
>
> But they can recover. Something that simplefb cannot do.
Can they? That's a pretty bold assumption.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 5:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929220250.GD5599@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 2572 bytes --]
On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote:
> On Mon, Sep 29, 2014 at 06:58:42PM +0200, Luc Verhaegen wrote:
> >
> > In the 2nd round of this discussion, i stated that another fb or even a
> > drm driver altogether seemed to be the sensible way out of this mess.
> >
> > I suggest drm_rescue.
> >
> > Very early on, now almost two months back, i used the word "denialfb".
> > rpifb is the real name of this thing though, but then the dt binding
> > names would have to change and whatnot.
> >
> > I don't get the resistance, at least not from a technical point of view.
> > And i do not care enough to get properly involved in this pointless and
> > endless discussion. drm_rescue it is.
> >
> > Luc Verhaegen.
>
> So Thierry, let's review what we have achieved here.
>
> 1) you keep simplefb absolutely true to the name. Congratulations.
> 2) Simplefb will only have a single user: the rpi. As the only other
> users i can think of, which does not have a full driver and which does
> not have clocks automatically disabled, are discrete cards. And they do
> not really tend to happen with dt or platform devices.
> 3) a competing driver will be created, which will do these dt-ishy
> things.
You clearly haven't bothered to even try and understand what I wanted to
achieve. If you had you'd realize that creating a competing driver isn't
going to change anything.
> 4) it's just a matter of time before the rpi either gets a full driver,
> or switches over to the driver that everyone else is actually using. And
> then the misnomer gets deprecated.
>
> Was that the outcome you were looking for? I think not.
I had, perhaps naively, expected that you guys would be willing to look
beyond your own nose. You clearly aren't. You've been outright
dismissing any proposals beyond what you originally posted.
You keep bringing up the Raspberry Pi for some reason and suggest that
it is somehow inferior to sunxi. What makes you think it's less entitled
to be supported on Linux than sunxi? I don't care about the Raspberry Pi
and I equally don't care about sunxi. I don't own a Raspberry Pi and I
don't own any Allwinner hardware. What I do care about is Linux and I
want it to work well for all SoCs equally.
Perhaps if you could put aside your crusade against the Raspberry Pi for
just a second you'll realize that we're all on the same team. This isn't
a competition and I'm not trying to put a spoke in your wheel. On the
contrary, I'm actually trying to help you.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 6:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929161101.GS16977@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 5421 bytes --]
On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote:
> On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote:
> > On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote:
>
> > > With that said I think that Luc's approach is very sensible. I'm not
> > > sure what purpose in the universe DT is supposed to serve if not for
> > > _this_exact_case_. We have a nice abstracted driver, usable by many
> > > people. The hardware details of how it is hooked up at the board level
> > > can all be hidden behind stable DT bindings that everyone already uses.
>
> > simplefb doesn't deal at all with hardware details. It simply uses what
> > firmware has set up, which is the only reason why it will work for many
> > people. What is passed in via its device tree node is the minimum amount
> > of information needed to draw something into the framebuffer. Also note
> > that the simplefb device tree node is not statically added to a DTS file
> > but needs to be dynamically generated by firmware at runtime.
>
> > If we start extending the binding with board-level details we end up
> > duplicating the device tree node for the proper video device. Also note
> > that it won't stop at clocks. Other setups will require regulators to be
> > listed in this device tree node as well so that they don't get disabled
> > at late_initcall. And the regulator bindings don't provide a method to
> > list an arbitrary number of clocks in a single property in the way that
> > the clocks property works.
>
> Not really thought this through fully yet but would having phandles to
> the relevant devices do the job? Kind of lines up with Grant's idea of
> having dummy drivers.
One of the arguments that came up during the discussion of the sunxi
patches is that simplefb is going to be used precisely because there is
no real driver for the display part of the SoC yet and nobody knows what
the binding will look like. So there's nothing to point at currently and
for the same reason having a dummy driver won't work. There's simply no
definition of what resources are needed.
> > There may be also resets involved. Fortunately the reset framework is
> > minimalistic enough not to care about asserting all unused resets at
> > late_initcall. And other things like power domains may also need to be
> > kept on.
>
> We might want to do that in the future, though it's not always the case
> that reset is the lowest power state.
That proves my point. If we ever decide to assert resets by default
we'll have yet another subsystem that can potentially break existing
DTs.
In the end it brings us back to the very fundamental principles of DT
that's been causing so much pain. For things to work properly and in a
stable way you have to get the bindings right and complete from the
start. That is, it needs to describe every aspect of the hardware block
and all links to other components.
But there is no way you can do that for a virtual device like simplefb
because it is a generic abstraction for hardware that varies wildly. The
only way to make it work is to abstract away all the resource management
and consider it to be done elsewhere.
> > So how about instead of requiring resources to be explicitly claimed we
> > introduce something like the below patch? The intention being to give
> > "firmware device" drivers a way of signalling to the clock framework
> > that they need rely on clocks set up by firmware and when they no longer
> > need them. This implements essentially what Mark (CC'ing again on this
> > subthread) suggested earlier in this thread. Basically, it will allow
> > drivers to determine the time when unused clocks are really unused. It
> > will of course only work when used correctly by drivers. For the case of
> > simplefb I'd expect its .probe() implementation to call the new
> > clk_ignore_unused() function and once it has handed over control of the
> > display hardware to the real driver it can call clk_unignore_unused() to
> > signal that all unused clocks that it cares about have now been claimed.
> > This is "reference counted" and can therefore be used by more than a
> > single driver if necessary. Similar functionality could be added for
> > other resource subsystems as needed.
>
> One thing that makes me a bit nervous about this approach in the context
> of the regulator API is the frequency with which one finds shared
> supplies. I'm not sure if it's actually a big problem in practice but
> it makes me worry a bit. We could probably just do something like make
> refcounting down to zero not actually disable anything for standard
> regulators to deal with it which might be an idea anyway in the context
> of this sort of dodge.
Yes, that's sort of how I expected clk_ignore_unused to work. The way I
understood it, it would cause all unused clocks to be ignored (that is
stay enabled if they are).
Of course as Geert pointed out in another subthread, taking this all the
way means that we have to disable all power management because the
firmware device may be sharing resources with other devices and which
therefore are not unused. That's a pretty strong argument and I don't
have a solution for that. It is only really a problem for cases where
the firmware virtual device isn't taken over by a proper driver at some
point, though.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-09-30 7:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930045957.GA29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 4647 bytes --]
On Tue, Sep 30, 2014 at 06:59:59AM +0200, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
> > On Mon, Sep 29, 2014 at 03:54:00PM +0200, Thierry Reding wrote:
> > > On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote:
> > > > On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote:
> > > > > > >> Plus, speaking more specifically about the clocks, that won't prevent
> > > > > > >> your clock to be shut down as a side effect of a later clk_disable
> > > > > > >> call from another driver.
> > > > > >
> > > > > > > Furthermore isn't it a bug for a driver to call clk_disable() before a
> > > > > > > preceding clk_enable()? There are patches being worked on that will
> > > > > > > enable per-user clocks and as I understand it they will specifically
> > > > > > > disallow drivers to disable the hardware clock if other drivers are
> > > > > > > still keeping them on via their own referenc.
> > > > > >
> > > > > > Calling clk_disable() preceding clk_enable() is a bug.
> > > > > >
> > > > > > Calling clk_disable() after clk_enable() will disable the clock (and
> > > > > > its parents)
> > > > > > if the clock subsystem thinks there are no other users, which is what will
> > > > > > happen here.
> > > > >
> > > > > Right. I'm not sure this is really applicable to this situation, though.
> > > >
> > > > It's actually very easy to do. Have a driver that probes, enables its
> > > > clock, fails to probe for any reason, call clk_disable in its exit
> > > > path. If there's no other user at that time of this particular clock
> > > > tree, it will be shut down. Bam. You just lost your framebuffer.
> > > >
> > > > Really, it's just that simple, and relying on the fact that some other
> > > > user of the same clock tree will always be their is beyond fragile.
> > >
> > > Perhaps the meaning clk_ignore_unused should be revised, then. What you
> > > describe isn't at all what I'd expect from such an option. And it does
> > > not match the description in Documentation/kernel-parameters.txt either.
> >
> > Well, it never says that it also prevent them from being disabled
> > later on. But it's true it's not very explicit.
>
> It says:
>
> Keep all clocks already enabled by bootloader on,
> even if no driver has claimed them. ...
>
> There's no "until" or anything there, so I interpret that as
> indefinitely.
Well, then, sorry, but that's not how it works.
> > > > > Either way, if there are other users of a clock then they will just as
> > > > > likely want to modify the rate at which point simplefb will break just
> > > > > as badly.
> > > >
> > > > And this can be handled just as well. Register a clock notifier,
> > > > refuse any rate change, done. But of course, that would require having
> > > > a clock handle.
> > > >
> > > > Now, how would *you* prevent such a change?
> > >
> > > Like I said in the other thread. If you have two drivers that use the
> > > same clock but need different frequencies you've lost anyway.
> >
> > Except that the driver that has the most logic (ie not simplefb) will
> > have a way to recover and deal with that.
>
> You're making an assumption here. Why would the driver have such logic
> if nothing ever prevented it from setting the rate properly before.
I'm not saying it has, but it something that can be done. You usually
have several strategies, which depending on the device, might or might
not be possible, such as reparenting, trying to use an additional
divider.
Worst case scenario, you're indeed doomed. But you do have a best case
scenario, which isn't the case with your approach. And you didn't
screw the framebuffer silently.
> > But sure, you can still try to point new issues, get an obvious and
> > robust solution, and then discard the issue when the solution doesn't
> > go your way...
>
> And you've already proven that you're completely unwilling to even
> consider any other solution than what was originally proposed, so I
> really don't see how discussing this further with you is going to be
> productive.
You haven't express *what* you wanted to achieve for quite some time,
but only *how*. And your how has some deficiencies that have already
been pointed out numerous times.
However, I do come to the same conclusion. I really don't see how we
can be productive. Just like I really don't see how we will ever be
able to get any DRM/KMS driver in when the time comes.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-09-30 7:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930052110.GC29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 7728 bytes --]
On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> > > On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote:
> > > > On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote:
> > > > > On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote:
> > > > > > On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote:
> > > > > [...]
> > > > > > > simplefb doesn't deal at all with hardware details. It simply uses what
> > > > > > > firmware has set up, which is the only reason why it will work for many
> > > > > > > people. What is passed in via its device tree node is the minimum amount
> > > > > > > of information needed to draw something into the framebuffer. Also note
> > > > > > > that the simplefb device tree node is not statically added to a DTS file
> > > > > > > but needs to be dynamically generated by firmware at runtime.
> > > > > >
> > > > > > Which makes the whole even simpler, since the firmware already knows
> > > > > > all about which clocks it had to enable.
> > > > >
> > > > > It makes things very complicated in the firmware because it now needs to
> > > > > be able to generate DTB content that we would otherwise be able to do
> > > > > much easier with a text editor.
> > > >
> > > > Didn't you just say that it was dynamically generated at runtime? So
> > > > we can just ignore the "text editor" case.
> > >
> > > Perhaps read the sentence again. I said "that we would *otherwise* be
> > > able to do much easier with a text editor.".
> > >
> > > My point remains that there shouldn't be a need to generate DTB content
> > > of this complexity at all.
> > >
> > > > > Why do you think what I proposed isn't going to work or be reliable?
> > > > > I don't see any arguments in the thread that would imply that.
> > > >
> > > > The fact that it broke in the first place?
> > >
> > > That's exactly the point. And it's going to break again and again as
> > > simplefb is extended with new things. Generally DT bindings should be
> > > backwards compatible. When extended they should provide a way to fall
> > > back to a reasonable default. There's simply no way you can do that
> > > with simplefb.
> >
> > This one *is* backward compatible. It doesn't even change the simplefb
> > behaviour, compared to what it was doing before, if you don't have
> > this clocks property. What can be a more reasonable default that the
> > way it used to behave?
>
> My point is that if we decide not to consider all resources handled by
> firmware then we need to go all the way. At that point you start having
> problems as evidenced by the Snow example.
Agreed.
> > > What happened in the Snow example is that regulators that were
> > > previously on would all of a sudden be automatically disabled on boot
> > > because there was now a driver that registered them with a generic
> > > framework.
> > >
> > > The same thing is going to happen with simplefb for your device. If you
> > > later realize that you need a regulator to keep the panel going, you'll
> > > have to add code to your firmware to populate the corresponding
> > > properties, otherwise the regulator will end up unused and will be
> > > automatically disabled. At the same time you're going to break upstream
> > > for all users of your old firmware because it doesn't add that property
> > > yet.
> > >
> > > And the same will continue to happen for every new type of resource
> > > you're going to add.
> >
> > Sure, we can add any resources we will need. Regulators, reset lines,
> > pm domains, allocated memory, but I'm not really sure this is what you
> > want, right?
>
> No it's not what I want. *You* want to add resource management to this
> driver. What I'm saying is that if we start adding clocks then we can no
> longer say no to resets or regulators or power domains either.
Yes, because resource management can be more than just "keep the thing
enabled". It might also be about not modifying anything, just like we
saw for the clocks, but that might also apply to regulators voltage.
And the only way I can think of to deal with that properly is to have
resources management in the driver.
> > > > > > You know that you are going to call that for regulator, reset, power
> > > > > > domains, just as you would have needed to with the proper API, unless
> > > > > > that with this kind of solution, you would have to modify *every*
> > > > > > framework that might interact with any resource involved in getting
> > > > > > simplefb running?
> > > > >
> > > > > We have to add handling for every kind of resource either way. Also if
> > > > > this evolves into a common pattern we can easily wrap it up in a single
> > > > > function call.
> > > >
> > > > Unless that in one case, we already have everything needed to handle
> > > > everything properly, and in another, you keep hacking more and more
> > > > into the involved frameworks.
> > >
> > > This is a fundamental issue that we are facing and I'm trying to come up
> > > with a solution that is future-proof and will work for drivers other
> > > than simplefb.
> > >
> > > Just because we currently lack this functionality doesn't make it a hack
> > > trying to add it.
> >
> > Or maybe it's just showing that the trend to rely more and more on the
> > firmware is a bad idea?
>
> If you think it's a bad idea to rely on firmware then you shouldn't be
> using simplefb in the first place.
>
> > I really start to consider adding a sunxi-uboot-fb, that would just
> > duplicate the source code of simplefb but also taking the
> > clocks. Somehow, I feel like it will be easier (and definitely less of
> > a hack than using the generic common clock API).
>
> You're not getting it are you? What makes you think sunxi-uboot-fb is
> going to be any different? This isn't about a name.
At least, we would get to do any binding and resource management we
want. And that's a big win.
> > > > > > Plus, speaking more specifically about the clocks, that won't prevent
> > > > > > your clock to be shut down as a side effect of a later clk_disable
> > > > > > call from another driver.
> > > > >
> > > > > If we need to prevent that, then that's something that could be fixed,
> > > > > too.
> > > >
> > > > See, you keep hacking it more...
> > >
> > > If you don't see this as a problem that exists beyond simplefb then I
> > > would of course not expect you to think that anything needs fixing.
> >
> > Then please point me to something else that needs fixing, so that I
> > can see the whole picture.
>
> We've been over this before. I'm tired of having to repeat myself.
>
> > > > > so hopefully concurrent users aren't the problem because that would
> > > > > cause the real driver to break too.
> > > > >
> > > > > Also note that if some other driver could call clk_disable() it could
> > > > > just as easily call clk_set_rate() and break simplefb.
> > > >
> > > > This is not true, see my other reply.
> > >
> > > If you mean that you could register a clock notifier to prevent clock
> > > changes, then that's going to lead to other drivers failing since they
> > > can now no longer set the rate that they need.
> >
> > But they can recover. Something that simplefb cannot do.
>
> Can they? That's a pretty bold assumption.
* They might recover. Something that simplefb cannot do.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-09-30 8:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930053900.GD29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]
On Tue, Sep 30, 2014 at 07:39:02AM +0200, Thierry Reding wrote:
> You keep bringing up the Raspberry Pi for some reason and suggest that
> it is somehow inferior to sunxi. What makes you think it's less entitled
> to be supported on Linux than sunxi? I don't care about the Raspberry Pi
> and I equally don't care about sunxi. I don't own a Raspberry Pi and I
> don't own any Allwinner hardware. What I do care about is Linux and I
> want it to work well for all SoCs equally.
>
> Perhaps if you could put aside your crusade against the Raspberry Pi for
> just a second you'll realize that we're all on the same team. This isn't
> a competition and I'm not trying to put a spoke in your wheel. On the
> contrary, I'm actually trying to help you.
We've been over this already, and I'll tell you again that you're
getting this wrong.
No platform is more entitled to get merged than another one. I do care
about the Allwinner SoCs, and I care just as much about the broader
Linux support for all the other SoCs, be it from nvidia, samsung or
whatever vendor you can come up with.
But you can't hide the fact that the bcm2835 still has a very limited
clock support, and I really don't know about its clock tree, but I
guess that if the times come when they add a more complete clock
support, they will face the same issue.
If the driver would have been developped initially to create a
framebuffer on the Allwinner SoCs, at a time when we didn't have any
clock support too, calling it only usable on sunxi wouldn't have
shocked me tbh.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930075258.GH4081@lukather>
[-- Attachment #1: Type: text/plain, Size: 3828 bytes --]
On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
[...]
> > > > What happened in the Snow example is that regulators that were
> > > > previously on would all of a sudden be automatically disabled on boot
> > > > because there was now a driver that registered them with a generic
> > > > framework.
> > > >
> > > > The same thing is going to happen with simplefb for your device. If you
> > > > later realize that you need a regulator to keep the panel going, you'll
> > > > have to add code to your firmware to populate the corresponding
> > > > properties, otherwise the regulator will end up unused and will be
> > > > automatically disabled. At the same time you're going to break upstream
> > > > for all users of your old firmware because it doesn't add that property
> > > > yet.
> > > >
> > > > And the same will continue to happen for every new type of resource
> > > > you're going to add.
> > >
> > > Sure, we can add any resources we will need. Regulators, reset lines,
> > > pm domains, allocated memory, but I'm not really sure this is what you
> > > want, right?
> >
> > No it's not what I want. *You* want to add resource management to this
> > driver. What I'm saying is that if we start adding clocks then we can no
> > longer say no to resets or regulators or power domains either.
>
> Yes, because resource management can be more than just "keep the thing
> enabled". It might also be about not modifying anything, just like we
> saw for the clocks, but that might also apply to regulators voltage.
We've already determined that simplefb can't do anything meaningful with
the resources other than keep them in the status quo. It simply doesn't
have enough knowledge to do so. It doesn't know the exact pixel clock or
what voltage the attached panel needs.
> And the only way I can think of to deal with that properly is to have
> resources management in the driver.
My point is that if we had a proper way to tell the kernel not to do
anything with resources owned by firmware, then the driver wouldn't
have to do anything with the resources.
> > > I really start to consider adding a sunxi-uboot-fb, that would just
> > > duplicate the source code of simplefb but also taking the
> > > clocks. Somehow, I feel like it will be easier (and definitely less of
> > > a hack than using the generic common clock API).
> >
> > You're not getting it are you? What makes you think sunxi-uboot-fb is
> > going to be any different? This isn't about a name.
>
> At least, we would get to do any binding and resource management we
> want. And that's a big win.
So instead of trying to understand the concerns that I've expressed and
constructively contribute to finding a solution that works for everybody
you'd rather go and write a driver from scratch. Way to go.
I've already said that I'm not saying strictly no to these patches, but
what I want to see happen is some constructive discussion about whether
we can find better ways to do it. If we can't then I'm all for merging
these patches. Fortunately other (sub)threads have been somewhat more
constructive and actually come up with alternatives that should make
everyone happier.
If you're going to do SoC-specific bindings and resource management you
are in fact implementing what Grant suggested in a subthread. You're
implementing a dummy driver only for resource management, which isn't
really a bad thing. It can serve as a placeholder for now until you add
the real driver. And you can also use the simplefb driver to provide
the framebuffer.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-09-30 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930085429.GB4059@ulmo>
On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
>> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
>> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
>> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> [...]
>> > > > What happened in the Snow example is that regulators that were
>> > > > previously on would all of a sudden be automatically disabled on boot
>> > > > because there was now a driver that registered them with a generic
>> > > > framework.
>> > > >
>> > > > The same thing is going to happen with simplefb for your device. If you
>> > > > later realize that you need a regulator to keep the panel going, you'll
>> > > > have to add code to your firmware to populate the corresponding
>> > > > properties, otherwise the regulator will end up unused and will be
>> > > > automatically disabled. At the same time you're going to break upstream
>> > > > for all users of your old firmware because it doesn't add that property
>> > > > yet.
>> > > >
>> > > > And the same will continue to happen for every new type of resource
>> > > > you're going to add.
>> > >
>> > > Sure, we can add any resources we will need. Regulators, reset lines,
>> > > pm domains, allocated memory, but I'm not really sure this is what you
>> > > want, right?
>> >
>> > No it's not what I want. *You* want to add resource management to this
>> > driver. What I'm saying is that if we start adding clocks then we can no
>> > longer say no to resets or regulators or power domains either.
>>
>> Yes, because resource management can be more than just "keep the thing
>> enabled". It might also be about not modifying anything, just like we
>> saw for the clocks, but that might also apply to regulators voltage.
>
> We've already determined that simplefb can't do anything meaningful with
> the resources other than keep them in the status quo. It simply doesn't
> have enough knowledge to do so. It doesn't know the exact pixel clock or
> what voltage the attached panel needs.
>
>> And the only way I can think of to deal with that properly is to have
>> resources management in the driver.
>
> My point is that if we had a proper way to tell the kernel not to do
> anything with resources owned by firmware, then the driver wouldn't
> have to do anything with the resources.
The firmware on sunxi does not own any resources whatsoever. It ceases
running once it executes the kernel. This is different from ACPI and
UEFI where you have pieces of the firmware lingering indefinitely and
potentially getting invoked by user pressing some button or some other
hardware event. It is also different from rpi where the Linux kernel
effectively runs in a virtual environment created by the firmware
hypervisor.
So on sunxi and many other ARM machines the Linux kernel is the sole
owner of any resources that might happen to be available on the
machine. There is no firmware owning them when the Linux kernel is
running, ever.
And we do have a proper way to tell to the kernel what these resources
are used for - inserting description of them into the simplefb DT
node. Sure the simplefb cannot manage the resources in any way and but
it does own them. When it is running they are in use, when it stops
they are free to be reclaimed by the platform driver.
But you refuse to marge the kernel change for this proper management to happen.
The alternate proposal to stop the kernel from managing resources
while simplefb is running and refernce count drivers that cannot
manage resources is indeed a workable, general alternative.
It does however switch the kernel into special mode when resources are
not managed and will needlessly hinder eg. development and testing of
powermanagement and hotplug for drivers unrelated to kms in parallel
to kms.
But let's look at this special mode closer.
Under normal boot sequence when the built-in drivers are initialized
or around that time the kernel does a pass in which it disables unused
clocks and possibly reclaims other resources.
The boot has finished for the kernel and now it hands over to
userspace and it is up to the userspace to load any more drivers (such
as kms) or not. If at that point a driver like simplefb exists it
should have called that stop_managing_resources() which should replace
clk_ignore_unused() so that other resources are properly handled
without the driver ever having to know about them.
For clocks this should be simple. At least on sunxi the clock driver
can tell if the clock is gated and can potentially be in use. It can
even mark clocks that are used at this point to know not to ever
disable them. Also it should refuse to ever change a clock frequency
on these clocks since if one of them was used for simplefb it should
break. It does not matter it's a mmc bus clock completely unrelated to
display. If you assume no knowledge you cannot change the mmc clock
when a different card is inserted or when a card is inserted into an
empty slot. If the firmware probed the slot the clock might be running
anyway.
Other resources might be more difficult to tackle. Is it possible to
change voltage regulators? Even the fact that it is set to 0 does not
necessarily mean that it is unused and then other driver claiming it
later can change the voltage. So no changing the regulators, ever.
And there goes gpio. You cannot assume that you can change any pin
muxing or any pin level whatsoever. The pin might be connected to the
display. So no loading of drivers that need any pin assignment
changes.
I guess by now it's clear that if this mode that assumes no knowledge
of the underlying hardware and requires the kernel to not manage
resources in order to keep dumb drivers running completely paralyzes
the kernel and will among other things prevent loading of the proper
kms driver.
>
>> > > I really start to consider adding a sunxi-uboot-fb, that would just
>> > > duplicate the source code of simplefb but also taking the
>> > > clocks. Somehow, I feel like it will be easier (and definitely less of
>> > > a hack than using the generic common clock API).
>> >
>> > You're not getting it are you? What makes you think sunxi-uboot-fb is
>> > going to be any different? This isn't about a name.
>>
>> At least, we would get to do any binding and resource management we
>> want. And that's a big win.
>
> So instead of trying to understand the concerns that I've expressed and
> constructively contribute to finding a solution that works for everybody
> you'd rather go and write a driver from scratch. Way to go.
It's the constructive thing to do when the existing driver cannot be
extended to work for everybody.
>
> I've already said that I'm not saying strictly no to these patches, but
> what I want to see happen is some constructive discussion about whether
> we can find better ways to do it. If we can't then I'm all for merging
> these patches. Fortunately other (sub)threads have been somewhat more
> constructive and actually come up with alternatives that should make
> everyone happier.
What are those alternatives?
>
> If you're going to do SoC-specific bindings and resource management you
> are in fact implementing what Grant suggested in a subthread. You're
> implementing a dummy driver only for resource management, which isn't
> really a bad thing. It can serve as a placeholder for now until you add
> the real driver. And you can also use the simplefb driver to provide
> the framebuffer.
Oh, so back to the proposal to make a driver that claims the required
resources and then instantiates an unextended simplefb that is
oblivious to the resources to be kept simple?
Did I not propose that way back?
Was it not already rejected?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] fonts: Add 6x10 font
From: Tomi Valkeinen @ 2014-09-30 9:41 UTC (permalink / raw)
To: Maarten ter Huurne, Jean-Christophe Plagniol-Villard,
Geert Uytterhoeven
Cc: Apelete Seketeli, linux-fbdev, linux-kernel
In-Reply-To: <1410263188-10532-1-git-send-email-maarten@treewalker.org>
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
Hi,
On 09/09/14 14:46, Maarten ter Huurne wrote:
> This font is suitable for framebuffer consoles on devices with a
> 320x240 screen, to get a reasonable number of characters (53x24) that
> are still at a readable size.
>
> The font is derived from the existing 6x11 font, but gets 3 extra
> lines without sacrificing readability. Also I redesigned a some glyhps
> so they are more distinct and better fill the available space.
>
> Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
> ---
> include/linux/font.h | 4 +-
> lib/fonts/Kconfig | 9 +
> lib/fonts/Makefile | 1 +
> lib/fonts/font_6x10.c | 3086 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/fonts/fonts.c | 4 +
> 5 files changed, 3103 insertions(+), 1 deletion(-)
> create mode 100644 lib/fonts/font_6x10.c
I think the patch is fine, but there hasn't been any comments or acks
for this. I'm a bit reluctant to add this much data to the kernel if
there are no users (except you) for this.
So I'd like to hear at least one "yeah, I could maybe use that font"
from someone =).
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: valkyriefb: Fix unused variable warning in set_valkyrie_clock()
From: Tomi Valkeinen @ 2014-09-30 9:43 UTC (permalink / raw)
To: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-m68k, linuxppc-dev
In-Reply-To: <1410688110-11042-1-git-send-email-geert@linux-m68k.org>
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
On 14/09/14 12:48, Geert Uytterhoeven wrote:
> If CONFIG_ADB_CUDA=n:
>
> drivers/video/fbdev/valkyriefb.c: In function ‘set_valkyrie_clock’:
> drivers/video/fbdev/valkyriefb.c:267: warning: unused variable ‘i’
> drivers/video/fbdev/valkyriefb.c:266: warning: unused variable ‘req’
>
> Move the variable declarations inside the existing #ifdef section to fix
> this.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> drivers/video/fbdev/valkyriefb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
> index 97cb9bd1d1dd..b6ed09f16355 100644
> --- a/drivers/video/fbdev/valkyriefb.c
> +++ b/drivers/video/fbdev/valkyriefb.c
> @@ -263,10 +263,10 @@ static inline int valkyrie_vram_reqd(int video_mode, int color_mode)
>
> static void set_valkyrie_clock(unsigned char *params)
> {
> +#ifdef CONFIG_ADB_CUDA
> struct adb_request req;
> int i;
>
> -#ifdef CONFIG_ADB_CUDA
> for (i = 0; i < 3; ++i) {
> cuda_request(&req, NULL, 5, CUDA_PACKET, CUDA_GET_SET_IIC,
> 0x50, i + 1, params[i]);
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: sis_main.c: Cleaning up missing null-terminate in conjunction with s
From: Tomi Valkeinen @ 2014-09-30 10:01 UTC (permalink / raw)
To: Rickard Strandqvist, Thomas Winischhofer,
Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <1410716148-16243-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
On 14/09/14 20:35, Rickard Strandqvist wrote:
> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/fbdev/sis/sis_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
> index 3f12a2d..1f4c216 100644
> --- a/drivers/video/fbdev/sis/sis_main.c
> +++ b/drivers/video/fbdev/sis/sis_main.c
> @@ -5830,7 +5830,7 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> ivideo->cardnumber++;
> }
>
> - strncpy(ivideo->myid, chipinfo->chip_name, 30);
> + strlcpy(ivideo->myid, chipinfo->chip_name, sizeof(ivideo->myid));
>
> ivideo->warncount = 0;
> ivideo->chip_id = pdev->device;
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] video: fbdev: stifb.c: use container_of to resolve stifb_info from fb_info
From: Tomi Valkeinen @ 2014-09-30 10:08 UTC (permalink / raw)
To: Fabian Frederick, linux-kernel
Cc: daniel.vetter, James E.J. Bottomley, Helge Deller,
Jean-Christophe Plagniol-Villard, linux-parisc, linux-fbdev
In-Reply-To: <1410980419-5326-2-git-send-email-fabf@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Hi,
On 17/09/14 22:00, Fabian Frederick wrote:
> Use container_of instead of casting first structure member.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> PARISC cross-compiled but untested.
>
> drivers/video/fbdev/stifb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
> index cfe8a2f..86621fa 100644
> --- a/drivers/video/fbdev/stifb.c
> +++ b/drivers/video/fbdev/stifb.c
> @@ -918,7 +918,7 @@ static int
> stifb_setcolreg(u_int regno, u_int red, u_int green,
> u_int blue, u_int transp, struct fb_info *info)
> {
> - struct stifb_info *fb = (struct stifb_info *) info;
> + struct stifb_info *fb = container_of(info, struct stifb_info, info);
> u32 color;
>
> if (regno >= NR_PALETTE)
> @@ -978,7 +978,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
> static int
> stifb_blank(int blank_mode, struct fb_info *info)
> {
> - struct stifb_info *fb = (struct stifb_info *) info;
> + struct stifb_info *fb = container_of(info, struct stifb_info, info);
> int enable = (blank_mode == 0) ? ENABLE : DISABLE;
>
> switch (fb->id) {
>
Thanks, queuing the series for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM
From: Tomi Valkeinen @ 2014-09-30 10:15 UTC (permalink / raw)
To: Behan Webster, plagnioj
Cc: daniel.vetter, geert, laurent.pinchart, linux-fbdev, linux-kernel,
linux-omap, robdclark
In-Reply-To: <1411780253-4492-1-git-send-email-behanw@converseincode.com>
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
On 27/09/14 04:10, Behan Webster wrote:
> Replace the use of nested functions where a normal function will suffice.
>
> Nested functions are not liked by upstream kernel developers in general. Their
> use breaks the use of clang as a compiler, and doesn't make the code any
> better.
>
> This code now works for both gcc and clang.
>
> The LLVMLinux project aims to fully build the Linux kernel using both gcc and
> clang (the C front end for the LLVM compiler infrastructure project).
>
> Behan Webster (2):
> arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
> arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb
>
> drivers/video/fbdev/omap2/dss/dispc-compat.c | 9 +++++----
> drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +++++++++-------
> drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++++++-------
> 3 files changed, 21 insertions(+), 18 deletions(-)
>
I have to say I do like small helper funcs as nested functions, as they
are restricted inside the parent function's scope. But, of course,
nested funcs have their issues.
So looks fine to me, queuing for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Tomi Valkeinen @ 2014-09-30 10:24 UTC (permalink / raw)
To: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson
Cc: Guennadi Liakhovetski, linux-fbdev, linux-pm, linux-sh
In-Reply-To: <1411474918-2955-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]
Hi Geert, Ulf,
On 23/09/14 15:21, Geert Uytterhoeven wrote:
> When the PM domain containing the HDMI hardware block is powered down,
> the HDMI register values (incl. interrupt polarity settings) are lost.
> During resume, after powering up the PM domain, interrupts are
> re-enabled, and an interrupt storm happens due to incorrect interrupt
> polarity settings:
>
> irq 163: nobody cared (try booting with the "irqpoll" option)
> ...
> Disabling IRQ #163
>
> To fix this, re-initialize the interrupt polarity settings, and the
> htop1 register block (if present), during resume.
>
> As the .suspend_noirq() and .resume_noirq() callbacks are not called
> when using the generic PM domain, the normal .resume() callback is used,
> and the device interrupt needs to be disabled/enabled manually.
>
> This fixes resume from s2ram with power down of the A4MP PM domain on
> r8a7740/Armadillo.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
> callbacks are not called when using genpd, unlike .suspend(),
> .suspend_late(), .resume_early(), and .resume()?
> ---
> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
Should I queue this version for 3.18, or is there some modifications you
think needs to be done first?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] framebuffer: fix border color
From: Tomi Valkeinen @ 2014-09-30 10:41 UTC (permalink / raw)
To: Mikulas Patocka, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1409161238560.3150@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
On 16/09/14 19:40, Mikulas Patocka wrote:
> The framebuffer code uses the current background color to fill the border
> when switching consoles, however, this results in inconsistent behavior.
> For example:
> - start Midnigh Commander
> - the border is black
> - switch to another console and switch back
> - the border is cyan
> - type something into the command line in mc
> - the border is cyan
> - switch to another console and switch back
> - the border is black
> - press F9 to go to menu
> - the border is black
> - switch to another console and switch back
> - the border is dark blue
>
> When switching to a console with Midnight Commander, the border is random
> color that was left selected by the slang subsystem.
>
> This patch fixes this inconsistency by always using black as the
> background color when switching consoles.
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Geert Uytterhoeven @ 2014-09-30 10:41 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson, Guennadi Liakhovetski,
Linux Fbdev development list, Linux PM list, Linux-sh list
In-Reply-To: <542A84F7.5020005@ti.com>
Hi Tomi,
On Tue, Sep 30, 2014 at 12:24 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>> When the PM domain containing the HDMI hardware block is powered down,
>> the HDMI register values (incl. interrupt polarity settings) are lost.
>> During resume, after powering up the PM domain, interrupts are
>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>> polarity settings:
>>
>> irq 163: nobody cared (try booting with the "irqpoll" option)
>> ...
>> Disabling IRQ #163
>>
>> To fix this, re-initialize the interrupt polarity settings, and the
>> htop1 register block (if present), during resume.
>>
>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>> when using the generic PM domain, the normal .resume() callback is used,
>> and the device interrupt needs to be disabled/enabled manually.
>>
>> This fixes resume from s2ram with power down of the A4MP PM domain on
>> r8a7740/Armadillo.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>> callbacks are not called when using genpd, unlike .suspend(),
>> .suspend_late(), .resume_early(), and .resume()?
>> ---
>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> Should I queue this version for 3.18, or is there some modifications you
> think needs to be done first?
Given the lack of documentation, I cannot do more improvements.
Please apply this version, so the interrupt storm on resume is fixed.
Thanks for applying!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] framebuffer: fix screen corruption when copying
From: Tomi Valkeinen @ 2014-09-30 10:41 UTC (permalink / raw)
To: Mikulas Patocka, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1409161236550.3150@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
On 16/09/14 19:38, Mikulas Patocka wrote:
> The function bitcpy_rev has a bug that may result in screen corruption.
> The bug happens under these conditions:
> * the end of the destination area of a copy operation is aligned on a long
> word boundary
> * the end of the source area is not aligned on a long word boundary
> * we are copying more than one long word
>
> In this case, the variable shift is non-zero and the variable first is
> zero. The statements FB_WRITEL(comp(d0, FB_READL(dst), first), dst) reads
> the last long word of the destination and writes it back unchanged
> (because first is zero). Correctly, we should write the variable d0 to the
> last word of the destination in this case.
>
> This patch fixes the bug by introducing and extra test if first is zero.
>
> The patch also removes the references to fb_memmove in the code that is
> commented out because fb_memmove was removed from framebuffer subsystem.
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Tomi Valkeinen @ 2014-09-30 10:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson, Guennadi Liakhovetski,
Linux Fbdev development list, Linux PM list, Linux-sh list
In-Reply-To: <CAMuHMdV-PLgG6yC=e9=qJNKbyYbUcVS-QeOaAMtf53AUOvXBvA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]
On 30/09/14 13:41, Geert Uytterhoeven wrote:
> Hi Tomi,
>
> On Tue, Sep 30, 2014 at 12:24 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>>> When the PM domain containing the HDMI hardware block is powered down,
>>> the HDMI register values (incl. interrupt polarity settings) are lost.
>>> During resume, after powering up the PM domain, interrupts are
>>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>>> polarity settings:
>>>
>>> irq 163: nobody cared (try booting with the "irqpoll" option)
>>> ...
>>> Disabling IRQ #163
>>>
>>> To fix this, re-initialize the interrupt polarity settings, and the
>>> htop1 register block (if present), during resume.
>>>
>>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>>> when using the generic PM domain, the normal .resume() callback is used,
>>> and the device interrupt needs to be disabled/enabled manually.
>>>
>>> This fixes resume from s2ram with power down of the A4MP PM domain on
>>> r8a7740/Armadillo.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>>> callbacks are not called when using genpd, unlike .suspend(),
>>> .suspend_late(), .resume_early(), and .resume()?
>>> ---
>>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> Should I queue this version for 3.18, or is there some modifications you
>> think needs to be done first?
>
> Given the lack of documentation, I cannot do more improvements.
>
> Please apply this version, so the interrupt storm on resume is fixed.
Ok, queued. Thanks!
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Ulf Hansson @ 2014-09-30 10:43 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Guennadi Liakhovetski,
Linux Fbdev development list, linux-pm@vger.kernel.org,
Linux-sh list
In-Reply-To: <542A84F7.5020005@ti.com>
On 30 September 2014 12:24, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi Geert, Ulf,
>
> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>> When the PM domain containing the HDMI hardware block is powered down,
>> the HDMI register values (incl. interrupt polarity settings) are lost.
>> During resume, after powering up the PM domain, interrupts are
>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>> polarity settings:
>>
>> irq 163: nobody cared (try booting with the "irqpoll" option)
>> ...
>> Disabling IRQ #163
>>
>> To fix this, re-initialize the interrupt polarity settings, and the
>> htop1 register block (if present), during resume.
>>
>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>> when using the generic PM domain, the normal .resume() callback is used,
>> and the device interrupt needs to be disabled/enabled manually.
>>
>> This fixes resume from s2ram with power down of the A4MP PM domain on
>> r8a7740/Armadillo.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>> callbacks are not called when using genpd, unlike .suspend(),
>> .suspend_late(), .resume_early(), and .resume()?
>> ---
>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> Should I queue this version for 3.18, or is there some modifications you
> think needs to be done first?
I have no objections for this patch. Please go ahead.
The reason for my comments was to try to figure out if there were any
impact to the generic PM domain.
Kind regards
Uffe
^ permalink raw reply
* Re: [RFC] fbdev: mxsfb: Add support for mx6sl and mx6sx
From: Tomi Valkeinen @ 2014-09-30 10:54 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]
On 19/08/14 21:23, Fabio Estevam wrote:
> mx6sl and mx6sx share the same LCD controller as mx23 and mx28.
>
> Add support for it.
>
> The basic difference is the number of clocks that are required:
>
> - mx23/mx28: only one clock
> - mx6sl: two clocks
> - mx6sx: three clocks
>
> Tested on a mx28evk and mx6slevk.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> I am sending it as RFC because the scrolling is showing some artifacts when
> I boot with console=tty0. The kernel messages are displayed correctly, but when
> it scrolls I see some garbage.
Is the garbage on the framebuffer memory? If so, I don't think this
patch can cause it.
> Appreciate any comment/suggestion to improve this.
>
> drivers/video/fbdev/Kconfig | 2 +-
> drivers/video/fbdev/mxsfb.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index e911b9c..fd28987 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -2420,7 +2420,7 @@ config FB_JZ4740
>
> config FB_MXS
> tristate "MXS LCD framebuffer support"
> - depends on FB && ARCH_MXS
> + depends on FB && (ARCH_MXS || ARCH_MXC)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> index accf48a2..a567cc6 100644
> --- a/drivers/video/fbdev/mxsfb.c
> +++ b/drivers/video/fbdev/mxsfb.c
> @@ -172,6 +172,10 @@ struct mxsfb_info {
> struct fb_info fb_info;
> struct platform_device *pdev;
> struct clk *clk;
> + struct clk *clk_axi;
> + struct clk *clk_disp_axi;
> + bool clk_axi_enabled;
> + bool clk_disp_axi_enabled;
> void __iomem *base; /* registers */
> unsigned allocated_size;
> int enabled;
> @@ -185,6 +189,42 @@ struct mxsfb_info {
> #define mxsfb_is_v3(host) (host->devdata->ipversion == 3)
> #define mxsfb_is_v4(host) (host->devdata->ipversion == 4)
>
> +/* Enable lcdif axi clock */
> +static inline void clk_enable_axi(struct mxsfb_info *host)
> +{
> + if (!host->clk_axi_enabled && (!IS_ERR(host->clk_axi))) {
> + clk_prepare_enable(host->clk_axi);
> + host->clk_axi_enabled = true;
> + }
> +}
> +
> +/* Disable lcdif axi clock */
> +static inline void clk_disable_axi(struct mxsfb_info *host)
> +{
> + if (host->clk_axi_enabled && (!IS_ERR(host->clk_axi))) {
> + clk_disable_unprepare(host->clk_axi);
> + host->clk_axi_enabled = false;
> + }
I think you could do the IS_ERR(clk_axi) check when getting the clock,
and if there's an error, do host->clk_axi = NULL. Then above you can do
just:
if (host->clk_axi)
clk_disable_unprepare(host->clk_axi);
And no need to clk_axi_enabled. And probably the helper functions can be
left out and have the code inline.
And similarly for clk_disp_axi, of course.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930080354.GI4081@lukather>
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
On Tue, Sep 30, 2014 at 10:03:54AM +0200, Maxime Ripard wrote:
> On Tue, Sep 30, 2014 at 07:39:02AM +0200, Thierry Reding wrote:
> > You keep bringing up the Raspberry Pi for some reason and suggest that
> > it is somehow inferior to sunxi. What makes you think it's less entitled
> > to be supported on Linux than sunxi? I don't care about the Raspberry Pi
> > and I equally don't care about sunxi. I don't own a Raspberry Pi and I
> > don't own any Allwinner hardware. What I do care about is Linux and I
> > want it to work well for all SoCs equally.
> >
> > Perhaps if you could put aside your crusade against the Raspberry Pi for
> > just a second you'll realize that we're all on the same team. This isn't
> > a competition and I'm not trying to put a spoke in your wheel. On the
> > contrary, I'm actually trying to help you.
>
> We've been over this already, and I'll tell you again that you're
> getting this wrong.
>
> No platform is more entitled to get merged than another one. I do care
> about the Allwinner SoCs, and I care just as much about the broader
> Linux support for all the other SoCs, be it from nvidia, samsung or
> whatever vendor you can come up with.
Okay, I'm glad our goals aren't that far apart then. It would be helpful
to stop dragging the Raspberry Pi into this, though, since it isn't at
all relevant.
> But you can't hide the fact that the bcm2835 still has a very limited
> clock support, and I really don't know about its clock tree, but I
> guess that if the times come when they add a more complete clock
> support, they will face the same issue.
This isn't at all relevant. And that's exactly why I think it's good to
hide all the resource management behind firmware. That way it becomes
easy to support any SoC with any firmware.
> If the driver would have been developped initially to create a
> framebuffer on the Allwinner SoCs, at a time when we didn't have any
> clock support too, calling it only usable on sunxi wouldn't have
> shocked me tbh.
The functionality that it provides is still very generic. And the
firmware interface is generic too. It is this abstraction that allows
it to be generic. You on the other hand seem to be arguing that by
making it abstract we've made it less generic.
Abstraction is about hiding details to capture commonality.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMqctRM1YZBR+p=0R39BSX_GT6XagPcb4HBG1BqW1OqyB48zw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10430 bytes --]
On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote:
> On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> >> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> >> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> >> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> > [...]
> >> > > > What happened in the Snow example is that regulators that were
> >> > > > previously on would all of a sudden be automatically disabled on boot
> >> > > > because there was now a driver that registered them with a generic
> >> > > > framework.
> >> > > >
> >> > > > The same thing is going to happen with simplefb for your device. If you
> >> > > > later realize that you need a regulator to keep the panel going, you'll
> >> > > > have to add code to your firmware to populate the corresponding
> >> > > > properties, otherwise the regulator will end up unused and will be
> >> > > > automatically disabled. At the same time you're going to break upstream
> >> > > > for all users of your old firmware because it doesn't add that property
> >> > > > yet.
> >> > > >
> >> > > > And the same will continue to happen for every new type of resource
> >> > > > you're going to add.
> >> > >
> >> > > Sure, we can add any resources we will need. Regulators, reset lines,
> >> > > pm domains, allocated memory, but I'm not really sure this is what you
> >> > > want, right?
> >> >
> >> > No it's not what I want. *You* want to add resource management to this
> >> > driver. What I'm saying is that if we start adding clocks then we can no
> >> > longer say no to resets or regulators or power domains either.
> >>
> >> Yes, because resource management can be more than just "keep the thing
> >> enabled". It might also be about not modifying anything, just like we
> >> saw for the clocks, but that might also apply to regulators voltage.
> >
> > We've already determined that simplefb can't do anything meaningful with
> > the resources other than keep them in the status quo. It simply doesn't
> > have enough knowledge to do so. It doesn't know the exact pixel clock or
> > what voltage the attached panel needs.
> >
> >> And the only way I can think of to deal with that properly is to have
> >> resources management in the driver.
> >
> > My point is that if we had a proper way to tell the kernel not to do
> > anything with resources owned by firmware, then the driver wouldn't
> > have to do anything with the resources.
>
> The firmware on sunxi does not own any resources whatsoever. It ceases
> running once it executes the kernel. This is different from ACPI and
> UEFI where you have pieces of the firmware lingering indefinitely and
> potentially getting invoked by user pressing some button or some other
> hardware event. It is also different from rpi where the Linux kernel
> effectively runs in a virtual environment created by the firmware
> hypervisor.
You know all that because you of course wrote every single firmware
implementation that does and will ever exist for sunxi. There's nothing
keeping anyone from running UEFI on a sunxi SoC.
> So on sunxi and many other ARM machines the Linux kernel is the sole
> owner of any resources that might happen to be available on the
> machine. There is no firmware owning them when the Linux kernel is
> running, ever.
Of course this is part of the abstraction. The idea is that the device
is a virtual one created by firmware. Therefore firmware owns the
resources until the virtual device has been handed over to the kernel.
If you're into splitting hairs, then the simplefb device shouldn't exist
in the first place.
> And we do have a proper way to tell to the kernel what these resources
> are used for - inserting description of them into the simplefb DT
> node. Sure the simplefb cannot manage the resources in any way and but
> it does own them. When it is running they are in use, when it stops
> they are free to be reclaimed by the platform driver.
Yes. And again I'm not saying anything different. What I'm saying is
that we shouldn't need to know about the resources and instead hide that
within the firmware, for the same reason that we're already hiding the
register programming in hardware, namely to create an abstraction that
works irrespective of the underlying hardware.
> But you refuse to marge the kernel change for this proper management
> to happen.
I have no authority to merge these changes nor have I any way of vetoing
them. All I'm saying is that I think it's a bad idea. If nobody else
thinks it is then it will eventually get merged irrespective of what I'm
saying. And when that happens I'll shut up and live happily ever after.
But it doesn't magically convince me that it's a good idea.
> The alternate proposal to stop the kernel from managing resources
> while simplefb is running and refernce count drivers that cannot
> manage resources is indeed a workable, general alternative.
>
> It does however switch the kernel into special mode when resources are
> not managed and will needlessly hinder eg. development and testing of
> powermanagement and hotplug for drivers unrelated to kms in parallel
> to kms.
>
> But let's look at this special mode closer.
>
> Under normal boot sequence when the built-in drivers are initialized
> or around that time the kernel does a pass in which it disables unused
> clocks and possibly reclaims other resources.
>
> The boot has finished for the kernel and now it hands over to
> userspace and it is up to the userspace to load any more drivers (such
> as kms) or not. If at that point a driver like simplefb exists it
> should have called that stop_managing_resources() which should replace
> clk_ignore_unused() so that other resources are properly handled
> without the driver ever having to know about them.
>
> For clocks this should be simple. At least on sunxi the clock driver
> can tell if the clock is gated and can potentially be in use. It can
> even mark clocks that are used at this point to know not to ever
> disable them. Also it should refuse to ever change a clock frequency
> on these clocks since if one of them was used for simplefb it should
> break. It does not matter it's a mmc bus clock completely unrelated to
> display. If you assume no knowledge you cannot change the mmc clock
> when a different card is inserted or when a card is inserted into an
> empty slot. If the firmware probed the slot the clock might be running
> anyway.
>
> Other resources might be more difficult to tackle. Is it possible to
> change voltage regulators? Even the fact that it is set to 0 does not
> necessarily mean that it is unused and then other driver claiming it
> later can change the voltage. So no changing the regulators, ever.
>
> And there goes gpio. You cannot assume that you can change any pin
> muxing or any pin level whatsoever. The pin might be connected to the
> display. So no loading of drivers that need any pin assignment
> changes.
The original patches don't tackle any of these problems. And in addition
to what's been mentioned elsewhere you're adding two new types of
resources that need to be potentially claimed here. Fortunately for GPIO
and pinmux the default is to not touch them at all unless explicitly
claimed and controlled. At least as far as I know.
> I guess by now it's clear that if this mode that assumes no knowledge
> of the underlying hardware and requires the kernel to not manage
> resources in order to keep dumb drivers running completely paralyzes
> the kernel and will among other things prevent loading of the proper
> kms driver.
Yes, Geert already made that argument and I admit that it exposes a flaw
in the solution that I proposed.
> >> > > I really start to consider adding a sunxi-uboot-fb, that would just
> >> > > duplicate the source code of simplefb but also taking the
> >> > > clocks. Somehow, I feel like it will be easier (and definitely less of
> >> > > a hack than using the generic common clock API).
> >> >
> >> > You're not getting it are you? What makes you think sunxi-uboot-fb is
> >> > going to be any different? This isn't about a name.
> >>
> >> At least, we would get to do any binding and resource management we
> >> want. And that's a big win.
> >
> > So instead of trying to understand the concerns that I've expressed and
> > constructively contribute to finding a solution that works for everybody
> > you'd rather go and write a driver from scratch. Way to go.
>
> It's the constructive thing to do when the existing driver cannot be
> extended to work for everybody.
No, it isn't. If a generic driver doesn't work for everybody then it
isn't generic and we should fix it. Not duplicate it and add platform
specific quirks.
> > I've already said that I'm not saying strictly no to these patches, but
> > what I want to see happen is some constructive discussion about whether
> > we can find better ways to do it. If we can't then I'm all for merging
> > these patches. Fortunately other (sub)threads have been somewhat more
> > constructive and actually come up with alternatives that should make
> > everyone happier.
>
> What are those alternatives?
Sorry, you've got to do some of the work yourself. They've been
mentioned in this thread and the one that Maxime pointed to the other
day.
> > If you're going to do SoC-specific bindings and resource management you
> > are in fact implementing what Grant suggested in a subthread. You're
> > implementing a dummy driver only for resource management, which isn't
> > really a bad thing. It can serve as a placeholder for now until you add
> > the real driver. And you can also use the simplefb driver to provide
> > the framebuffer.
>
> Oh, so back to the proposal to make a driver that claims the required
> resources and then instantiates an unextended simplefb that is
> oblivious to the resources to be kept simple?
Pretty much, yes.
> Did I not propose that way back?
Yes, I think you did.
> Was it not already rejected?
No, I don't think it was. In fact I don't think anything was really
rejected yet, we're still in the middle of a discussion.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
From: Tomi Valkeinen @ 2014-09-30 11:40 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
linux-fbdev
In-Reply-To: <1407329562-10832-1-git-send-email-daniel.vetter@ffwll.ch>
[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]
Hi,
On 06/08/14 15:52, Daniel Vetter wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
>
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.
>
> Whiel at it fix the checkpatch fail in this code.
>
> v2: Also move fb_mode_option. Spotted by the kbuild.
>
> v3: Review from Geert:
> - Keep the old copyright notice from fb_mem.c, although I have no
> idea what exactly applies.
> - Only compile this when needed.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> --
>
> I prefer if we can merge this through drm-next since we'll use it
> there in follow-up patches.
> -Daniel
> ---
> drivers/video/fbdev/Kconfig | 4 ++
> drivers/video/fbdev/core/Makefile | 1 +
> drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
> drivers/video/fbdev/core/fbmem.c | 92 ----------------------------
> drivers/video/fbdev/core/modedb.c | 3 -
> 5 files changed, 115 insertions(+), 95 deletions(-)
> create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
Sorry for late response.
Looks fine for me, and I'm fine merging it via drm-next.
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-09-30 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930045957.GA29874@ulmo>
Hi,
On 09/30/2014 06:59 AM, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
<snip>
>> But sure, you can still try to point new issues, get an obvious and
>> robust solution, and then discard the issue when the solution doesn't
>> go your way...
>
> And you've already proven that you're completely unwilling to even
> consider any other solution than what was originally proposed, so I
> really don't see how discussing this further with you is going to be
> productive.
That is not true, we have seriously considered various other alternatives,
as you know since you've participated in the discussion about them.
And we've found them all lacking, mostly because they are 10 times as
complicated.
You've made your point that you don't like this solution quite loudly
already, and we've all heard you. However you seem to be mostly alone in
this. Even the clk maintainer has said that what we want to do is
exactly how clocks are supposed to be used in dt.
If you don't like this no-one is forcing you to use the clocks property
in your own code. If it is not there, simplefb will behave exactly as
before.
Now since you're the only one very vocally against this, and a lot
of people are in favor of this and have a need for this, can we
please just get this merged and get this over with ?
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <542A9771.2030401@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]
On Tue, Sep 30, 2014 at 01:43:45PM +0200, Hans de Goede wrote:
> Hi,
>
> On 09/30/2014 06:59 AM, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
>
> <snip>
>
> >> But sure, you can still try to point new issues, get an obvious and
> >> robust solution, and then discard the issue when the solution doesn't
> >> go your way...
> >
> > And you've already proven that you're completely unwilling to even
> > consider any other solution than what was originally proposed, so I
> > really don't see how discussing this further with you is going to be
> > productive.
>
> That is not true, we have seriously considered various other alternatives,
> as you know since you've participated in the discussion about them.
>
> And we've found them all lacking, mostly because they are 10 times as
> complicated.
>
> You've made your point that you don't like this solution quite loudly
> already, and we've all heard you. However you seem to be mostly alone in
> this. Even the clk maintainer has said that what we want to do is
> exactly how clocks are supposed to be used in dt.
>
> If you don't like this no-one is forcing you to use the clocks property
> in your own code. If it is not there, simplefb will behave exactly as
> before.
>
> Now since you're the only one very vocally against this, and a lot
> of people are in favor of this and have a need for this, can we
> please just get this merged and get this over with ?
Whatever. I no longer care.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox