* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-02-22 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160222175929.GA23899@ulmo>
Thierry,
On Mon, Feb 22, 2016 at 9:59 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
>> This is because only some drivers would be able to read the hardware
>> state? I'm not sure how we can get away from that. In all proposals
>> we've talked about (including what you propose below, right?) the PWM
>> regulator will need a PWM driver that can read hardware state. Only
>> PWM drivers that have been upgraded to support reading hardware state
>> can use the PWM regulator (or at least only those drivers will be able
>> to use the PWM regulator glitch-free).
>
> Yes, the key here is glitch-free. There's no reason whatsoever that the
> rugaltor-pwm driver should be limited to usage with a hardware readout-
> capable PWM driver. If you don't care about glitches, likely because no
> critical components depend on the regulator, you can simply force what
> state you choose on boot.
>
> As a matter of fact, I think that's how regulators work already. If the
> current output voltage doesn't match the specified constraints, then a
> valid value will be forced by the regulator core. If the voltage lies
> within the constraints the core won't touch the regulator. Is this not
> going to "just work" with the PWM regulator?
>
> The problem is somewhat simplified if that's the case. An implementation
> could then fail the regulator_get_voltage() if hardware readout is not
> supported and return the current voltage when readout is possible.
Based on looking at the current code, I believe it just returns 0V
until you call regulator_set_voltage() today. I don't think that was
always the case. Back before it was made continuous I think it
returned the voltage that matched with 0% duty cycle.
I haven't dug into what the regulator framework does in the current
system nor what happens if regulator_get_voltage() returns an error.
Perhaps Boris can dig / comment?
>> When we add a new feature then it's expected that only updated drivers
>> will support that feature.
>>
>> We need to make sure that we don't regress / negatively change the
>> behavior for anyone running non-updated drivers. ...and we should
>> strive to require as few changes to drivers as possible. ...but if
>> the best we can do requires changes to the PWM driver API then we will
>> certainly have differences depending on the PWM driver.
>
> How so? Drivers should behave consistently, irrespective of the API. Of
> course if you need to change behaviour of the user driver depending on
> the availability of a certain feature, that's perfectly fine.
>
> Furthermore it's out of the question that changes to the API will be
> required. That's precisely the reason why the atomic PWM proposal came
> about. It's an attempt to solve the shortcomings of the current API for
> cases such as Rockchip.
I _think_ we're on the same page here. If there are shortcomings with
the current API that make it impossible to implement a feature, we've
got to change and/or add to the existing API. ...but we don't want to
break existing users / drivers.
Note that historically I remember that Linus Torvalds has stated that
there is no stable API within the Linux kernel and that forcing the
in-kernel API to never change was bad for software development. I
tracked down my memory and found
<http://lwn.net/1999/0211/a/lt-binary.html>. Linus is rabid about not
breaking userspace, but in general there's no strong requirement to
never change the driver API inside the kernel. That being said,
changing the driver API causes a lot of churn, so presumably changing
it in a backward compatible way (like adding to the API instead of
changing it) will make things happier.
>> That means that if you call pwm_get_period() right away at boot time
>> you're not getting the current period of the hardware but the period
>> that was specified in the device tree.
>
> Yes.
>
>> So all we need is a new API call that lets you read the hardware
>> values and make sure that the PWM regulator calls that before anyone
>> calls pwm_config(). That's roughly B) above.
>
> Yes. I'm thinking that we should have a pwm_get_state() which retrieves
> the current state of the PWM. For drivers that support hardware readout
> this state should match the hardware state. For other drivers it should
> reflect whatever was specified in DT; essentially what pwm_get_period()
> and friends return today.
Excellent, so pwm_get_period() gets the period as specified in the
device tree (or other board config) and pwm_get_state() returns the
hardware state. SGTM.
> That way if you want to get the current voltage in the regulator-pwm
> driver you'd simply do a pwm_get_state() and compute the voltage from
> the period and duty cycle. If the PWM driver that you happen to use
> doesn't support hardware readout, you'll get an initial output voltage
> of 0, which is as good as any, really.
Sounds fine to me. PWM regulator is in charge of calling
pwm_get_state(), which can return 0 (or an error?) if driver (or
underlying hardware) doesn't support hardware readout. PWM regulator
is in charge of using the resulting period / duty cycle to calculate a
percentage.
>> Sure. ...but you agree that somehow you need a new API call for this,
>> right? Somehow the PWM regulator needs to be able to say that it
>> wants the hardware state, not the initial state as specified in the
>> device tree.
>
> The atomic PWM API is this new API. I'm not arguing that we don't need
> new API. What's being discussed is what the API needs to look like to
> support this new use-case and at the same time be maximally compatible
> with existing users.
>
> I think perhaps one question that needs answering to do that is whether
> or not the regulator-pwm driver can guess the correct period. The issue
> that was initially being discussed is what to do with hardware state vs
> initial state (i.e. what's defined in DT). Mark commented in another
> subthread that DT should simply leave out data that doesn't make sense
> to be specified.
>
> However I don't think there's any particularly reasonable period for the
> regulator-pwm use-case, so specifying the period within DT would still
> be necessary. What works for one board may not be the correct (or
> optimal) value for another. Similarly one board may need to invert the
> PWM signal to generate the proper voltage, or require other parameters
> that we haven't even defined yet.
>
> There's also the issue of a voltage table that you need to define in the
> DT for the regulator-pwm device. Does that consist of only duty-cycle
> values, or does it have corresponding period values as well. I'd guess
> that it really only needs the duty cycle given any period. So something
> like this is what I'd expect:
>
> regulator {
> compatible = "regulator-pwm";
>
> pwms = <&pwm0 5000>;
>
> voltages = <0 0>, <1000000 1000>, ..., <5000000 5000>;
> };
>
> I think the pwm-regulator binding defines the duty cycle in percent. I
> guess that would work equally well.
>
> And now we've come full circle because, again, we need to differentiate
> between the current hardware state and the initial state. Or, as was
> also discussed previously, alternatively, ignore the period specified in
> DT and just go with what hardware readout defined. In case hardware
> readout isn't supported, the "hardware state" will simply be the
> "initial state".
>
> The objection to the latter alternative was that we shouldn't trust the
> firmware to have set up the regulator correctly. But if it didn't, how
> can we trust that the duty cycle to period ratio is correct? Or the
> other way around: if we trust the duty cycle to period ratio to have
> been setup correctly, why don't we trust the period?
To answer:
* No, PWM regulator can't guess the correct period.
* PWM regulator API is already fine as is. Period specified in PWM
specifier and table is specified in percentages.
* Firmware may have voltage setup correctly but may not have the
"ideal" period. Often firmware configures things in a way that works
but is sub-optimal. Basically the kernel should be able to tell what
voltage the firmware set things up at (so we know not to go lower on
accident), but we shouldn't assume that the firmware values are
perfect. Said another way: obviously the firmware made values that
were good enough to boot us to where we are (or we wouldn't be even
executing code), but we might want to configure things to reduce noise
on the lines (make HDMI work better?) or optimize power consumption.
--
I _think_ the end result of all this is just:
1. Introduce pwm_get_state() that gets hardware state. Up for debate
if this returns 0 or ERROR if a driver doesn't implement this.
2. PWM regulator calls pwm_get_state at probe time to get hardware
state, calculates a percentage (and voltage) with this.
3. PWM regulator does nothing else until it is asked to set the
voltage, but uses the voltage calculated from #2 to satisfy any "get
voltage" calls.
4. When asked to set the voltage, PWM regulator uses pwm_get_period()
and calculates a duty cycle based on that, just like it does today.
This uses pwm_config() which includes a duty cycle and period and is
thus "atomic".
Said another way: the only action item from this point of view is just
that we introduce a new pwm_get_state().
Boris: I think that's everything you need, right?
--
Historically there was also a necessity that we were very careful with
the PWM clock because the clock would end up getting disabled
temporarily at bootup (after the PWM probe time but before the PWM
regulator finished probing). Presumably that's either been solved
already or can be debated totally separately.
-Doug
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Thierry Reding @ 2016-02-22 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=U_x1cxx6RrHuVgVV8GbOwm9DXXJcyiArbbQySUOaxrvQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 14152 bytes --]
On Wed, Feb 03, 2016 at 11:04:20AM -0800, Doug Anderson wrote:
> Thierry
>
> On Wed, Feb 3, 2016 at 6:53 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> A) The software state here is the period and flags (AKA "inverted),
> >> right? It does seem possible that you could apply the period and
> >> flags while keeping the calculated bootup duty cycle percentage
> >> (presuming that the PWM was actually enabled at probe time and there
> >> was a bootup duty cycle at all). That would basically say that
> >> whenever you set the period of a PWM then the duty cycle of the PWM
> >> should remain the same percentage. That actually seems quite sane
> >> IMHO. It seems much saner than trying to keep the duty cycle "ns"
> >> when the period changes or resetting the PWM to some default when the
> >> period changes.
> >
> > That really depends on the use-case. If you're interested in the output
> > power of the PWM then, yes, this is sane. But it might not be the right
> > answer in other cases.
>
> Ah, I see. You're envisioning a device where active time in "ns" is
> more important than the percentage of active time. Perhaps an LED
> where it's more important to have it on for no more than .1 seconds
> (so we don't drive it too long and burn it out?). If we slowed down
> the duty cycle and adjusted the period to match, we could get into a
> bad state.
Yes. Granted, the vast majority of users is not in this category, but
it's something that has been brought to my attention in the past and it
works fine with the current API.
> >> B) Alternatively, I'd also say that setting a period without a duty
> >> cycle doesn't make a lot of sense. ...so you could just apply the
> >> period at the same time that you apply the duty cycle the first time.
> >> Presumably you'd want to "lie" to the callers of the PWM subsystem and
> >> tell them that you already changed the period even though the change
> >> won't really take effect until they actually set the duty cycle. If
> >> anyone cared to find out the true hardware period we could add a new
> >> pwm_get_hw_period(). ...or since the only reason you'd want to know
> >> the hardware period would be if you're trying to read the current duty
> >> cycle percentage, you could instead add "pwm_get_hw_state()" and have
> >> that return both the hardware period ns and duty cycle ns (which is
> >> the most accurate way to return the "percentage" without using fix or
> >> floating point math).
> >
> > But then you get into a situation where behaviour is dependent on the
> > PWM driver, whereas this is really very specific to one specific use-
> > case.
>
> This is because only some drivers would be able to read the hardware
> state? I'm not sure how we can get away from that. In all proposals
> we've talked about (including what you propose below, right?) the PWM
> regulator will need a PWM driver that can read hardware state. Only
> PWM drivers that have been upgraded to support reading hardware state
> can use the PWM regulator (or at least only those drivers will be able
> to use the PWM regulator glitch-free).
Yes, the key here is glitch-free. There's no reason whatsoever that the
rugaltor-pwm driver should be limited to usage with a hardware readout-
capable PWM driver. If you don't care about glitches, likely because no
critical components depend on the regulator, you can simply force what
state you choose on boot.
As a matter of fact, I think that's how regulators work already. If the
current output voltage doesn't match the specified constraints, then a
valid value will be forced by the regulator core. If the voltage lies
within the constraints the core won't touch the regulator. Is this not
going to "just work" with the PWM regulator?
The problem is somewhat simplified if that's the case. An implementation
could then fail the regulator_get_voltage() if hardware readout is not
supported and return the current voltage when readout is possible.
> When we add a new feature then it's expected that only updated drivers
> will support that feature.
>
> We need to make sure that we don't regress / negatively change the
> behavior for anyone running non-updated drivers. ...and we should
> strive to require as few changes to drivers as possible. ...but if
> the best we can do requires changes to the PWM driver API then we will
> certainly have differences depending on the PWM driver.
How so? Drivers should behave consistently, irrespective of the API. Of
course if you need to change behaviour of the user driver depending on
the availability of a certain feature, that's perfectly fine.
Furthermore it's out of the question that changes to the API will be
required. That's precisely the reason why the atomic PWM proposal came
about. It's an attempt to solve the shortcomings of the current API for
cases such as Rockchip.
> > In the end the PWM API is as low-level as it is because it needs to be
> > flexible enough to cope with other use-cases. In the general case the
> > simple truth is that it doesn't make sense to set a period without the
> > duty cycle and vice versa. That's why pwm_config() takes both as input
> > parameters. The atomic API is going to take that one step further in
> > that you need to specify the complete state of the PWM when applying.
>
> I guess this is still showing the strangeness of the device tree
> specifying a period without a duty cycle. You just said it doesn't
> make sense, but that's exactly what the device tree has.
The device tree specifies the period because there is no way for the
driver (such as pwm-backlight) to "guess" the right one. Furthermore
there is a certain degree of board-specificity to that parameter and
therefore any heuristic trying to come up with a sensible value will
inevitably fail eventually.
> I'd imagine that the only reason that the device tree specifies just
> the period is that it's intended to be there only for clients that
> don't care about the specific duty cycle in terms of seconds but
> _only_ care about the duty cycle in terms of percentage. Anyone who
> cared about the duty cycle in terms of seconds would presumably also
> care about specifying the period (in terms of seconds) in the same
> place they specify the duty cycle.
Yes, exactly.
> >> > That doesn't really get us closer, though. There is still the issue of
> >> > the user having to deal with two states: the current hardware state and
> >> > the software state as configured in DT or board files.
> >>
> >> I think the only users that need to deal with this are one that need a
> >> seamless transition from bootup settings. Adding a new API call to
> >> support a new feature like this doesn't seem insane, and anyone who
> >> doesn't want this new feature can just never call the new API.
> >>
> >> The only thing that would "change" from the point of view of old
> >> drivers is that the PWM period wouldn't change at bootup until the
> >> duty cycle was set. IMHO this is probably a bug fix. AKA, for a PWM
> >> backlight, imagine:
> >>
> >> 1. Firmware sets period to 20000 ns, duty cycle to 8000 ns (40%)
> >> 2. Linux boots up and sets period to 10000 ns. Brightness of
> >> backlight instantly goes to 80%.
> >> 3. Eventually something decides to set the backlight duty cycle and it
> >> goes to the proper rate.
> >>
> >> Skipping #2 seems like the right move. ...or did I misunderstand how
> >> something works?
> >
> > I'm not aware of any code in the PWM subsystem that would do this
> > automatically. If you don't call any of the pwm_*() functions the
> > hardware state should not be modified. The responsibility is with
> > the user drivers.
>
> Ah, OK. I must have gotten confused.
>
> Oh, I see. So pwm_get_period() is actually "lying" about the hardware
> today! So what you're saying is that at boot time we grab the period
> out of the device tree but we _don't_ apply it to the hardware, right?
> I was thinking it would get applied right away...
That's correct. The value retrieved by pwm_get_period() is the one
specified in DT (or a PWM lookup table). It should match the hardware
value after the first call to pwm_config(), though.
> That means that if you call pwm_get_period() right away at boot time
> you're not getting the current period of the hardware but the period
> that was specified in the device tree.
Yes.
> So all we need is a new API call that lets you read the hardware
> values and make sure that the PWM regulator calls that before anyone
> calls pwm_config(). That's roughly B) above.
Yes. I'm thinking that we should have a pwm_get_state() which retrieves
the current state of the PWM. For drivers that support hardware readout
this state should match the hardware state. For other drivers it should
reflect whatever was specified in DT; essentially what pwm_get_period()
and friends return today.
That way if you want to get the current voltage in the regulator-pwm
driver you'd simply do a pwm_get_state() and compute the voltage from
the period and duty cycle. If the PWM driver that you happen to use
doesn't support hardware readout, you'll get an initial output voltage
of 0, which is as good as any, really.
> > That is, it is up to the regulator or backlight driver to apply any new
> > configuration to a PWM channel on boot, or leave it as is. For
> > backlight it's probably fine to simply apply some default, since having
> > the brightness change on boot isn't going to be terribly irritating.
> >
> > With the atomic API it would be possible to avoid even this and have the
> > backlight driver simply read out the current hardware state and apply an
> > equivalent brightness even if the period changed.
> >
> > For the regulator case you'd need to read out the current state and then
> > recompute the values that will yield the same output power given data
> > specified in DT. I think that much is already implemented in Boris'
> > series, and it's really only the details that are being debated.
> >
> > The problematic issue is still that we might have a disparity between
> > the current hardware state and the state initially specified by DT. In
> > the general case the DT will specify the period and polarity of the PWM
> > signal and leave it up to the user driver to determine what a correct
> > duty cycle would be. With the atomic API we'll essentially have two
> > states: the current (hardware) state and the "initial" state, which is
> > what an OS will see as the state to apply. The problem now is that once
> > you have applied the initial state with a duty cycle you've determined,
> > there is no longer a need to keep it around. But there's also no way to
> > know when this is the case. So the controversial part about all this is
> > when to start using the current state rather that the initial state.
> >
> > The most straightforward way to solve this would be to apply the initial
> > configuration on driver probe. That is, when the pwm-regulator driver
> > gets probed it would retrieve the current and initial states, then
> > adjust the current state such that it matches the initial state but with
> > a duty cycle that yields the same output power as the current state, and
> > finally apply the new state. After that, every regulator_set_voltage()
> > call could simply operate on the current state and adjust the duty cycle
> > exclusively.
> >
> > Does that sound reasonable?
>
> Sure. ...but you agree that somehow you need a new API call for this,
> right? Somehow the PWM regulator needs to be able to say that it
> wants the hardware state, not the initial state as specified in the
> device tree.
The atomic PWM API is this new API. I'm not arguing that we don't need
new API. What's being discussed is what the API needs to look like to
support this new use-case and at the same time be maximally compatible
with existing users.
I think perhaps one question that needs answering to do that is whether
or not the regulator-pwm driver can guess the correct period. The issue
that was initially being discussed is what to do with hardware state vs
initial state (i.e. what's defined in DT). Mark commented in another
subthread that DT should simply leave out data that doesn't make sense
to be specified.
However I don't think there's any particularly reasonable period for the
regulator-pwm use-case, so specifying the period within DT would still
be necessary. What works for one board may not be the correct (or
optimal) value for another. Similarly one board may need to invert the
PWM signal to generate the proper voltage, or require other parameters
that we haven't even defined yet.
There's also the issue of a voltage table that you need to define in the
DT for the regulator-pwm device. Does that consist of only duty-cycle
values, or does it have corresponding period values as well. I'd guess
that it really only needs the duty cycle given any period. So something
like this is what I'd expect:
regulator {
compatible = "regulator-pwm";
pwms = <&pwm0 5000>;
voltages = <0 0>, <1000000 1000>, ..., <5000000 5000>;
};
I think the pwm-regulator binding defines the duty cycle in percent. I
guess that would work equally well.
And now we've come full circle because, again, we need to differentiate
between the current hardware state and the initial state. Or, as was
also discussed previously, alternatively, ignore the period specified in
DT and just go with what hardware readout defined. In case hardware
readout isn't supported, the "hardware state" will simply be the
"initial state".
The objection to the latter alternative was that we shouldn't trust the
firmware to have set up the regulator correctly. But if it didn't, how
can we trust that the duty cycle to period ratio is correct? Or the
other way around: if we trust the duty cycle to period ratio to have
been setup correctly, why don't we trust the period?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-02-22 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=U_x1cxx6RrHuVgVV8GbOwm9DXXJcyiArbbQySUOaxrvQ@mail.gmail.com>
Thierry,
On Wed, Feb 3, 2016 at 11:04 AM, Doug Anderson <dianders@google.com> wrote:
> Thierry
>
> On Wed, Feb 3, 2016 at 6:53 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
>>> A) The software state here is the period and flags (AKA "inverted),
>>> right? It does seem possible that you could apply the period and
>>> flags while keeping the calculated bootup duty cycle percentage
>>> (presuming that the PWM was actually enabled at probe time and there
>>> was a bootup duty cycle at all). That would basically say that
>>> whenever you set the period of a PWM then the duty cycle of the PWM
>>> should remain the same percentage. That actually seems quite sane
>>> IMHO. It seems much saner than trying to keep the duty cycle "ns"
>>> when the period changes or resetting the PWM to some default when the
>>> period changes.
>>
>> That really depends on the use-case. If you're interested in the output
>> power of the PWM then, yes, this is sane. But it might not be the right
>> answer in other cases.
>
> Ah, I see. You're envisioning a device where active time in "ns" is
> more important than the percentage of active time. Perhaps an LED
> where it's more important to have it on for no more than .1 seconds
> (so we don't drive it too long and burn it out?). If we slowed down
> the duty cycle and adjusted the period to match, we could get into a
> bad state.
>
>
>>> B) Alternatively, I'd also say that setting a period without a duty
>>> cycle doesn't make a lot of sense. ...so you could just apply the
>>> period at the same time that you apply the duty cycle the first time.
>>> Presumably you'd want to "lie" to the callers of the PWM subsystem and
>>> tell them that you already changed the period even though the change
>>> won't really take effect until they actually set the duty cycle. If
>>> anyone cared to find out the true hardware period we could add a new
>>> pwm_get_hw_period(). ...or since the only reason you'd want to know
>>> the hardware period would be if you're trying to read the current duty
>>> cycle percentage, you could instead add "pwm_get_hw_state()" and have
>>> that return both the hardware period ns and duty cycle ns (which is
>>> the most accurate way to return the "percentage" without using fix or
>>> floating point math).
>>
>> But then you get into a situation where behaviour is dependent on the
>> PWM driver, whereas this is really very specific to one specific use-
>> case.
>
> This is because only some drivers would be able to read the hardware
> state? I'm not sure how we can get away from that. In all proposals
> we've talked about (including what you propose below, right?) the PWM
> regulator will need a PWM driver that can read hardware state. Only
> PWM drivers that have been upgraded to support reading hardware state
> can use the PWM regulator (or at least only those drivers will be able
> to use the PWM regulator glitch-free).
>
> When we add a new feature then it's expected that only updated drivers
> will support that feature.
>
> We need to make sure that we don't regress / negatively change the
> behavior for anyone running non-updated drivers. ...and we should
> strive to require as few changes to drivers as possible. ...but if
> the best we can do requires changes to the PWM driver API then we will
> certainly have differences depending on the PWM driver.
>
>
>> In the end the PWM API is as low-level as it is because it needs to be
>> flexible enough to cope with other use-cases. In the general case the
>> simple truth is that it doesn't make sense to set a period without the
>> duty cycle and vice versa. That's why pwm_config() takes both as input
>> parameters. The atomic API is going to take that one step further in
>> that you need to specify the complete state of the PWM when applying.
>
> I guess this is still showing the strangeness of the device tree
> specifying a period without a duty cycle. You just said it doesn't
> make sense, but that's exactly what the device tree has.
>
> I'd imagine that the only reason that the device tree specifies just
> the period is that it's intended to be there only for clients that
> don't care about the specific duty cycle in terms of seconds but
> _only_ care about the duty cycle in terms of percentage. Anyone who
> cared about the duty cycle in terms of seconds would presumably also
> care about specifying the period (in terms of seconds) in the same
> place they specify the duty cycle.
>
>
>>> I think this is like my suggestion B), right? AKA the PWM regulator
>>> would be the sole caller of pwm_get_hw_state() and it would use this
>>> to figure out the existing duty cycle percentage. Then it would
>>> translate that into "ns" and would set the duty cycle. Upon the first
>>> set of the duty cycle both the period and duty cycle would be applied
>>> at the same time.
>>
>> Yes and no. I think the PWM regulator would need to get the current
>> hardware state and derive the output power from duty cycle and period.
>> It would then need to rescale to whatever new period it wants to use
>> to ensure the same output power is used.
>
> Right. We all agree that somehow this needs to happen, it's just a
> question of the implementation.
>
>
>>> > That doesn't really get us closer, though. There is still the issue of
>>> > the user having to deal with two states: the current hardware state and
>>> > the software state as configured in DT or board files.
>>>
>>> I think the only users that need to deal with this are one that need a
>>> seamless transition from bootup settings. Adding a new API call to
>>> support a new feature like this doesn't seem insane, and anyone who
>>> doesn't want this new feature can just never call the new API.
>>>
>>> The only thing that would "change" from the point of view of old
>>> drivers is that the PWM period wouldn't change at bootup until the
>>> duty cycle was set. IMHO this is probably a bug fix. AKA, for a PWM
>>> backlight, imagine:
>>>
>>> 1. Firmware sets period to 20000 ns, duty cycle to 8000 ns (40%)
>>> 2. Linux boots up and sets period to 10000 ns. Brightness of
>>> backlight instantly goes to 80%.
>>> 3. Eventually something decides to set the backlight duty cycle and it
>>> goes to the proper rate.
>>>
>>> Skipping #2 seems like the right move. ...or did I misunderstand how
>>> something works?
>>
>> I'm not aware of any code in the PWM subsystem that would do this
>> automatically. If you don't call any of the pwm_*() functions the
>> hardware state should not be modified. The responsibility is with
>> the user drivers.
>
> Ah, OK. I must have gotten confused.
>
> Oh, I see. So pwm_get_period() is actually "lying" about the hardware
> today! So what you're saying is that at boot time we grab the period
> out of the device tree but we _don't_ apply it to the hardware, right?
> I was thinking it would get applied right away...
>
> That means that if you call pwm_get_period() right away at boot time
> you're not getting the current period of the hardware but the period
> that was specified in the device tree.
>
> So all we need is a new API call that lets you read the hardware
> values and make sure that the PWM regulator calls that before anyone
> calls pwm_config(). That's roughly B) above.
>
>
>> That is, it is up to the regulator or backlight driver to apply any new
>> configuration to a PWM channel on boot, or leave it as is. For
>> backlight it's probably fine to simply apply some default, since having
>> the brightness change on boot isn't going to be terribly irritating.
>>
>> With the atomic API it would be possible to avoid even this and have the
>> backlight driver simply read out the current hardware state and apply an
>> equivalent brightness even if the period changed.
>>
>> For the regulator case you'd need to read out the current state and then
>> recompute the values that will yield the same output power given data
>> specified in DT. I think that much is already implemented in Boris'
>> series, and it's really only the details that are being debated.
>>
>> The problematic issue is still that we might have a disparity between
>> the current hardware state and the state initially specified by DT. In
>> the general case the DT will specify the period and polarity of the PWM
>> signal and leave it up to the user driver to determine what a correct
>> duty cycle would be. With the atomic API we'll essentially have two
>> states: the current (hardware) state and the "initial" state, which is
>> what an OS will see as the state to apply. The problem now is that once
>> you have applied the initial state with a duty cycle you've determined,
>> there is no longer a need to keep it around. But there's also no way to
>> know when this is the case. So the controversial part about all this is
>> when to start using the current state rather that the initial state.
>>
>> The most straightforward way to solve this would be to apply the initial
>> configuration on driver probe. That is, when the pwm-regulator driver
>> gets probed it would retrieve the current and initial states, then
>> adjust the current state such that it matches the initial state but with
>> a duty cycle that yields the same output power as the current state, and
>> finally apply the new state. After that, every regulator_set_voltage()
>> call could simply operate on the current state and adjust the duty cycle
>> exclusively.
>>
>> Does that sound reasonable?
>
> Sure. ...but you agree that somehow you need a new API call for this,
> right? Somehow the PWM regulator needs to be able to say that it
> wants the hardware state, not the initial state as specified in the
> device tree.
I'm hoping we can get your latest thoughts on this topic so that Boris
can make forward progress. It would be nice to get to some resolution
somehow. The unfortunate part about having only one back-and-forth
exchange every few weeks is that I'm worried that we'll forget what we
already talked about and have the same discussions over and over
again. :(
-Doug
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Linus Walleij @ 2016-02-22 15:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <56CB2C31.5040703@ti.com>
On Mon, Feb 22, 2016 at 4:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> After thinking this a bit and discussing it with Laurent P., generally
> speaking I still think that the only sane option is that the bootloader
> does any detection needed and provides the kernel a .dtb that contains
> the HW that is connected. No board specific drivers are needed on the
> kernel side.
>
> In some cases userspace loaded DT overlays may be fine, if the userspace
> can do the detection and the device in question is not somehow critical
> to operation. But I think displays are critical, and afaik in Versatile
> case the userspace can't even do the detection (?).
>
> The third option is to have board specific display handling code and the
> display HW data in the kernel, as you've done in the patches.
Yeah correct...
> But, of course, which option should be used for which board is not
> always clear...
>
> What bootloader is used on Versatile?
It's U-boot indeed. Not that I've tried to compile or use it, I got it
as binary from ARM.
> If it's some proprietary loader
> which can't be changed, then the bootloader option is out, and I guess
> it points to the third option, i.e. either the version in this patch or
> the earlier version. If it's u-boot, I would suggest going for the
> bootloader option.
>
> Afaik u-boot doesn't support combining DT fragments yet. But (also
> afaik) the u-boot maintainer is ok with the idea. And I know there are
> others (for example TI) interested in the same functionality.
Hm OK.... so the bootloader need to be better at augmenting device
trees than the kernel. Well... The problem is that there are a bunch
of deployed systems out there and they all need to have their boot loader
updated then, which may be OK since it's ARM development boards
but I don't know.
> Now, adding that support might take some time, and in the meantime it'd
> be good to get the HW working with kernel with a temporary solution. To
> do that, my suggestion is basically "any solution which requires no
> (temporary) changes to .dts".
>
> While I don't like too much the solution in the patch here, it's all
> inside kernel code and can be dropped easily, right? If we would merge
> the the multi-endpoint solution you had in the earlier patch, you would
> have to support that .dts in the future too.
To go with this solution I need to extend the drivers/of library to be
able to update properties properly instead of this hack, and that is
non-reversible if we start to use it.
It is not really an overlay because the DT stuff is dynamically
augmented by the kernel, not taken from somewhere else.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Tomi Valkeinen @ 2016-02-22 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdaXFUCR5=5mS28_4Dx_LfzqV13zwT=vVeJwuOzm_rGRBQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]
On 22/02/16 00:39, Linus Walleij wrote:
> On Thu, Feb 18, 2016 at 12:52 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> For panels we need DT fragments. The question is where these fragments
>> are and, possibly, who who loads them.
>
> I hacked something up that augments the device tree from the kernel,
> given you have a node with all the props you want to augment, tell me
> what you think of this and whether I should continue in this direction...
> also the DT people need to be involved:
What you have there is almost like a legacy board file, isn't it? It's
just passing DT data forward, instead of device platform data. In fact,
if the driver in question supports platform data too, you could as well
generate platform data for it (but I'm not saying that's a better option).
After thinking this a bit and discussing it with Laurent P., generally
speaking I still think that the only sane option is that the bootloader
does any detection needed and provides the kernel a .dtb that contains
the HW that is connected. No board specific drivers are needed on the
kernel side.
In some cases userspace loaded DT overlays may be fine, if the userspace
can do the detection and the device in question is not somehow critical
to operation. But I think displays are critical, and afaik in Versatile
case the userspace can't even do the detection (?).
The third option is to have board specific display handling code and the
display HW data in the kernel, as you've done in the patches.
But, of course, which option should be used for which board is not
always clear...
What bootloader is used on Versatile? If it's some proprietary loader
which can't be changed, then the bootloader option is out, and I guess
it points to the third option, i.e. either the version in this patch or
the earlier version. If it's u-boot, I would suggest going for the
bootloader option.
Afaik u-boot doesn't support combining DT fragments yet. But (also
afaik) the u-boot maintainer is ok with the idea. And I know there are
others (for example TI) interested in the same functionality.
Now, adding that support might take some time, and in the meantime it'd
be good to get the HW working with kernel with a temporary solution. To
do that, my suggestion is basically "any solution which requires no
(temporary) changes to .dts".
While I don't like too much the solution in the patch here, it's all
inside kernel code and can be dropped easily, right? If we would merge
the the multi-endpoint solution you had in the earlier patch, you would
have to support that .dts in the future too.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Laurent Pinchart @ 2016-02-22 13:05 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
Hi Geert,
On Monday 22 February 2016 13:39:37 Geert Uytterhoeven wrote:
> On Mon, Feb 22, 2016 at 1:24 PM, Laurent Pinchart wrote:
> > On Monday 22 February 2016 10:59:51 Simon Horman wrote:
> >> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
> >>
> >> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> >> ARCH_RENESAS the motivation for which being that RENESAS seems to be a
> >> more appropriate name than SHMOBILE for the majority of Renesas ARM based
> >> SoCs.
> >>
> >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >
> > Wouldn't it make sense to drop the driver instead ? We have a DRM driver
> > that replaces it.
>
> Does the DRM driver work on all hardware supported by the fbdev driver?
> It's not only used on r8a7740/armadillo (through staging/board due to lack
> of DT support), but also on many SH boards.
It's supposed to be a replacement (lacking support for SYS panels though), but
has obviously not been tested on SH boards.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Geert Uytterhoeven @ 2016-02-22 12:39 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
Hi Laurent,
On Mon, Feb 22, 2016 at 1:24 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 22 February 2016 10:59:51 Simon Horman wrote:
>> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
>>
>> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
>> ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
>> appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.
>>
>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>
> Wouldn't it make sense to drop the driver instead ? We have a DRM driver that
> replaces it.
Does the DRM driver work on all hardware supported by the fbdev driver?
It's not only used on r8a7740/armadillo (through staging/board due to lack of
DT support), but also on many SH boards.
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] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Laurent Pinchart @ 2016-02-22 12:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
Hi Simon,
Thank you for the patch.
On Monday 22 February 2016 10:59:51 Simon Horman wrote:
> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
>
> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
> appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Wouldn't it make sense to drop the driver instead ? We have a DRM driver that
replaces it.
> ---
> drivers/video/fbdev/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Based on v4.5-rc1
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 8ea45a5cd806..936ebd4bcf73 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1985,7 +1985,7 @@ config FB_W100
>
> config FB_SH_MOBILE_LCDC
> tristate "SuperH Mobile LCDC framebuffer support"
> - depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> + depends on FB && (SUPERH || ARCH_RENESAS) && HAVE_CLK
> depends on FB_SH_MOBILE_MERAM || !FB_SH_MOBILE_MERAM
> select FB_SYS_FILLRECT
> select FB_SYS_COPYAREA
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Geert Uytterhoeven @ 2016-02-22 9:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1456106391-10353-1-git-send-email-horms+renesas@verge.net.au>
On Mon, Feb 22, 2016 at 2:59 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
>
> This is part of an ongoing process to migrate from ARCH_SHMOBILE to
> ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
> appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
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
* [PATCH 3/3] drivers/video: make fbdev/sunxvr2500.c explicitly non-modular
From: Paul Gortmaker @ 2016-02-22 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
David S. Miller, sparclinux, linux-fbdev
In-Reply-To: <1456110792-21771-1-git-send-email-paul.gortmaker@windriver.com>
The Kconfig currently controlling compilation of this code is:
config FB_XVR2500
bool "Sun XVR-2500 3DLABS Wildcat support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.
We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, and it allows us to drop the ".remove"
code for non-modular drivers.
We don't replace module.h with init.h since the file already has that.
We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/video/fbdev/sunxvr2500.c | 39 +++++++--------------------------------
1 file changed, 7 insertions(+), 32 deletions(-)
diff --git a/drivers/video/fbdev/sunxvr2500.c b/drivers/video/fbdev/sunxvr2500.c
index 843b6bab0483..1a053292f2eb 100644
--- a/drivers/video/fbdev/sunxvr2500.c
+++ b/drivers/video/fbdev/sunxvr2500.c
@@ -1,9 +1,10 @@
-/* s3d.c: Sun 3DLABS XVR-2500 et al. driver for sparc64 systems
+/* sunxvr2500.c: Sun 3DLABS XVR-2500 et al. fb driver for sparc64 systems
+ *
+ * License: GPL
*
* Copyright (C) 2007 David S. Miller (davem@davemloft.net)
*/
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fb.h>
#include <linux/pci.h>
@@ -219,22 +220,6 @@ err_out:
return err;
}
-static void s3d_pci_unregister(struct pci_dev *pdev)
-{
- struct fb_info *info = pci_get_drvdata(pdev);
- struct s3d_info *sp = info->par;
-
- unregister_framebuffer(info);
-
- iounmap(sp->fb_base);
-
- pci_release_region(pdev, 1);
-
- framebuffer_release(info);
-
- pci_disable_device(pdev);
-}
-
static struct pci_device_id s3d_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_3DLABS, 0x002c), },
{ PCI_DEVICE(PCI_VENDOR_ID_3DLABS, 0x002d), },
@@ -248,10 +233,12 @@ static struct pci_device_id s3d_pci_table[] = {
};
static struct pci_driver s3d_driver = {
+ .driver = {
+ .suppress_bind_attrs = true,
+ },
.name = "s3d",
.id_table = s3d_pci_table,
.probe = s3d_pci_register,
- .remove = s3d_pci_unregister,
};
static int __init s3d_init(void)
@@ -261,16 +248,4 @@ static int __init s3d_init(void)
return pci_register_driver(&s3d_driver);
}
-
-static void __exit s3d_exit(void)
-{
- pci_unregister_driver(&s3d_driver);
-}
-
-module_init(s3d_init);
-module_exit(s3d_exit);
-
-MODULE_DESCRIPTION("framebuffer driver for Sun XVR-2500 graphics");
-MODULE_AUTHOR("David S. Miller <davem@davemloft.net>");
-MODULE_VERSION("1.0");
-MODULE_LICENSE("GPL");
+device_initcall(s3d_init);
--
2.6.1
^ permalink raw reply related
* [PATCH 2/3] drivers/video: make fbdev/sunxvr1000.c explicitly non-modular
From: Paul Gortmaker @ 2016-02-22 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
David S. Miller, sparclinux, linux-fbdev
In-Reply-To: <1456110792-21771-1-git-send-email-paul.gortmaker@windriver.com>
The Kconfig currently controlling compilation of this code is:
config FB_XVR1000
bool "Sun XVR-1000 support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.
We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, and it allows us to drop the ".remove"
code for non-modular drivers.
We don't replace module.h with init.h since the file already has that.
Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/video/fbdev/sunxvr1000.c | 42 +++++++---------------------------------
1 file changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/video/fbdev/sunxvr1000.c b/drivers/video/fbdev/sunxvr1000.c
index 08879bdfad35..fb37f6e05391 100644
--- a/drivers/video/fbdev/sunxvr1000.c
+++ b/drivers/video/fbdev/sunxvr1000.c
@@ -1,9 +1,10 @@
-/* sunxvr1000.c: Sun XVR-1000 driver for sparc64 systems
+/* sunxvr1000.c: Sun XVR-1000 fb driver for sparc64 systems
+ *
+ * License: GPL
*
* Copyright (C) 2010 David S. Miller (davem@davemloft.net)
*/
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fb.h>
#include <linux/init.h>
@@ -173,36 +174,19 @@ err_out:
return err;
}
-static int gfb_remove(struct platform_device *op)
-{
- struct fb_info *info = dev_get_drvdata(&op->dev);
- struct gfb_info *gp = info->par;
-
- unregister_framebuffer(info);
-
- iounmap(gp->fb_base);
-
- of_iounmap(&op->resource[6], gp->fb_base, gp->fb_size);
-
- framebuffer_release(info);
-
- return 0;
-}
-
static const struct of_device_id gfb_match[] = {
{
.name = "SUNW,gfb",
},
{},
};
-MODULE_DEVICE_TABLE(of, ffb_match);
static struct platform_driver gfb_driver = {
.probe = gfb_probe,
- .remove = gfb_remove,
.driver = {
- .name = "gfb",
- .of_match_table = gfb_match,
+ .name = "gfb",
+ .of_match_table = gfb_match,
+ .suppress_bind_attrs = true,
},
};
@@ -213,16 +197,4 @@ static int __init gfb_init(void)
return platform_driver_register(&gfb_driver);
}
-
-static void __exit gfb_exit(void)
-{
- platform_driver_unregister(&gfb_driver);
-}
-
-module_init(gfb_init);
-module_exit(gfb_exit);
-
-MODULE_DESCRIPTION("framebuffer driver for Sun XVR-1000 graphics");
-MODULE_AUTHOR("David S. Miller <davem@davemloft.net>");
-MODULE_VERSION("1.0");
-MODULE_LICENSE("GPL");
+device_initcall(gfb_init);
--
2.6.1
^ permalink raw reply related
* [PATCH 1/3] drivers/video: make fbdev/sunxvr500.c explicitly non-modular
From: Paul Gortmaker @ 2016-02-22 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
David S. Miller, sparclinux, linux-fbdev
In-Reply-To: <1456110792-21771-1-git-send-email-paul.gortmaker@windriver.com>
The Kconfig currently controlling compilation of this code is:
config FB_XVR500
bool "Sun XVR-500 3DLABS Wildcat support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, and it allows us to drop the ".remove"
code for non-modular drivers.
Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.
We don't replace module.h with init.h since the file already has that.
We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/video/fbdev/sunxvr500.c | 42 +++++++----------------------------------
1 file changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/video/fbdev/sunxvr500.c b/drivers/video/fbdev/sunxvr500.c
index 387350d004df..dc0d886e4e7e 100644
--- a/drivers/video/fbdev/sunxvr500.c
+++ b/drivers/video/fbdev/sunxvr500.c
@@ -1,9 +1,10 @@
-/* sunxvr500.c: Sun 3DLABS XVR-500 Expert3D driver for sparc64 systems
+/* sunxvr500.c: Sun 3DLABS XVR-500 Expert3D fb driver for sparc64 systems
+ *
+ * License: GPL
*
* Copyright (C) 2007 David S. Miller (davem@davemloft.net)
*/
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fb.h>
#include <linux/pci.h>
@@ -392,25 +393,6 @@ err_out:
return err;
}
-static void e3d_pci_unregister(struct pci_dev *pdev)
-{
- struct fb_info *info = pci_get_drvdata(pdev);
- struct e3d_info *ep = info->par;
-
- unregister_framebuffer(info);
-
- iounmap(ep->ramdac);
- iounmap(ep->fb_base);
-
- pci_release_region(pdev, 0);
- pci_release_region(pdev, 1);
-
- fb_dealloc_cmap(&info->cmap);
- framebuffer_release(info);
-
- pci_disable_device(pdev);
-}
-
static struct pci_device_id e3d_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_3DLABS, 0x7a0), },
{ PCI_DEVICE(0x1091, 0x7a0), },
@@ -434,10 +416,12 @@ static struct pci_device_id e3d_pci_table[] = {
};
static struct pci_driver e3d_driver = {
+ .driver = {
+ .suppress_bind_attrs = true,
+ },
.name = "e3d",
.id_table = e3d_pci_table,
.probe = e3d_pci_register,
- .remove = e3d_pci_unregister,
};
static int __init e3d_init(void)
@@ -447,16 +431,4 @@ static int __init e3d_init(void)
return pci_register_driver(&e3d_driver);
}
-
-static void __exit e3d_exit(void)
-{
- pci_unregister_driver(&e3d_driver);
-}
-
-module_init(e3d_init);
-module_exit(e3d_exit);
-
-MODULE_DESCRIPTION("framebuffer driver for Sun XVR-500 graphics");
-MODULE_AUTHOR("David S. Miller <davem@davemloft.net>");
-MODULE_VERSION("1.0");
-MODULE_LICENSE("GPL");
+device_initcall(e3d_init);
--
2.6.1
^ permalink raw reply related
* [PATCH 0/3] video/fbdev: avoid module usage in non-modular sparc code
From: Paul Gortmaker @ 2016-02-22 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: Paul Gortmaker, David S. Miller, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, sparclinux
This series of commits is a part of a larger project to ensure
people don't reference modular support functions in non-modular
code. Overall there was roughly 5k lines of dead code in the
kernel due to this. So far we've fixed several areas, like tty,
x86, net, ... and we continue to work on other areas.
There are several reasons to not use module support for code that
can never be built as a module, but the big ones are:
(1) it is easy to accidentally write unused module_exit and remove code
(2) it can be misleading when reading the source, thinking it can be
modular when the Makefile and/or Kconfig prohibit it
(3) it requires the include of the module.h header file which in turn
includes nearly everything else, thus adding to CPP overhead.
(4) it gets copied/replicated into other code and spreads like weeds.
For the video fb subsystem, I have about 40 commits in my personal
testing queue, but rather than overwhelm maintainers with a large
patch series, I'm looking for subsets within that to keep the Cc on
the series and the 00/N boiler-plate to a sane amount.
To that end, the sparc specific fb drivers is a nice small subset
that can be used to test the waters in the video subsystem, to ensure
the overall goals here are OK with everyone maintaining fb code.
The disallow of unbind might seem like overkill, but for drivers that
had a ".remove" function, it is the only way we can block people from
abusing existing infrastructure in order to continue getting at that
otherwise dead function call that we want to remove.
We toss out a net amount of about 80 lines of dead code here in just
three commits; not groundbreaking, but a step in the right direction.
Build tested for sparc64, allmodconfig; but with CONFIG_FB=y as the
drivers here all depend on this specific value to get enabled.
Paul.
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Paul Gortmaker (3):
drivers/video: make fbdev/sunxvr500.c explicitly non-modular
drivers/video: make fbdev/sunxvr1000.c explicitly non-modular
drivers/video: make fbdev/sunxvr2500.c explicitly non-modular
drivers/video/fbdev/sunxvr1000.c | 42 +++++++---------------------------------
drivers/video/fbdev/sunxvr2500.c | 39 +++++++------------------------------
drivers/video/fbdev/sunxvr500.c | 42 +++++++---------------------------------
3 files changed, 21 insertions(+), 102 deletions(-)
--
2.6.1
^ permalink raw reply
* [PATCH] fbdev: sh_mobile_lcdc: Use ARCH_RENESAS
From: Simon Horman @ 2016-02-22 1:59 UTC (permalink / raw)
To: linux-fbdev
Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.
This is part of an ongoing process to migrate from ARCH_SHMOBILE to
ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Based on v4.5-rc1
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 8ea45a5cd806..936ebd4bcf73 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1985,7 +1985,7 @@ config FB_W100
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
- depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
+ depends on FB && (SUPERH || ARCH_RENESAS) && HAVE_CLK
depends on FB_SH_MOBILE_MERAM || !FB_SH_MOBILE_MERAM
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
--
2.1.4
^ permalink raw reply related
* [PATCH 7/7] video: fbdev: bt431: Correct cursor format control macro
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
The Bt431 cursor generator supports simultaneous generation of a 64 x 64
and a cross hair cursor in which the cursor format control bit (bit D4)
of the command register "specifies whether the contents of the cursor
RAM are to be logically exclusive-ORed (logical zero) or ORed (logical
one) with the cross hair cursor". Rename the relevant macro accordingly.
References:
[1] "Bt431 Monolithic CMOS 64 x 64 Pixel Cursor Generator", Brooktree
Corporation, Document Number: L431001, Rev. J
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-bt431-cmd-xor.diff
Index: linux-20160211-4maxp64/drivers/video/fbdev/bt431.h
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/bt431.h
+++ linux-20160211-4maxp64/drivers/video/fbdev/bt431.h
@@ -63,7 +63,7 @@ static inline u8 bt431_get_value(u16 val
#define BT431_CMD_CURS_ENABLE 0x40
#define BT431_CMD_XHAIR_ENABLE 0x20
#define BT431_CMD_OR_CURSORS 0x10
-#define BT431_CMD_AND_CURSORS 0x00
+#define BT431_CMD_XOR_CURSORS 0x00
#define BT431_CMD_1_1_MUX 0x00
#define BT431_CMD_4_1_MUX 0x04
#define BT431_CMD_5_1_MUX 0x08
^ permalink raw reply
* [PATCH 6/7] video: fbdev: pmag-ba-fb: Optimize Bt455 colormap addressing
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
Use the address autoincrement feature when accessing successive palette
entries and also skip loading a palette address in overlay register
assesses which do not use that address. Provide a red/green/blue
register sequencer reset helper for use in overlay register assesses
where the state of the sequencer is not known.
References:
[1] "Bt454 Bt455 170 MHz Monolithic CMOS 16 Color Palette RAMDAC",
Brooktree Corporation, Document Number: L454001, Rev. I
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-bt455-cmap-addr.diff
Index: linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/bt455.h
+++ linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
@@ -30,13 +30,17 @@ static inline void bt455_select_reg(stru
regs->addr_cmap = ir & 0x0f;
}
+static inline void bt455_reset_reg(struct bt455_regs *regs)
+{
+ mb();
+ regs->addr_clr = 0;
+}
+
/*
* Read/write to a Bt455 color map register.
*/
-static inline void bt455_read_cmap_entry(struct bt455_regs *regs,
- int cr, u8 *grey)
+static inline void bt455_read_cmap_next(struct bt455_regs *regs, u8 *grey)
{
- bt455_select_reg(regs, cr);
mb();
regs->addr_cmap_data;
rmb();
@@ -45,10 +49,8 @@ static inline void bt455_read_cmap_entry
regs->addr_cmap_data;
}
-static inline void bt455_write_cmap_entry(struct bt455_regs *regs,
- int cr, u8 grey)
+static inline void bt455_write_cmap_next(struct bt455_regs *regs, u8 grey)
{
- bt455_select_reg(regs, cr);
wmb();
regs->addr_cmap_data = 0x0;
wmb();
@@ -57,10 +59,8 @@ static inline void bt455_write_cmap_entr
regs->addr_cmap_data = 0x0;
}
-static inline void bt455_write_ovly_entry(struct bt455_regs *regs,
- int cr, u8 grey)
+static inline void bt455_write_ovly_next(struct bt455_regs *regs, u8 grey)
{
- bt455_select_reg(regs, cr);
wmb();
regs->addr_ovly = 0x0;
wmb();
@@ -68,3 +68,23 @@ static inline void bt455_write_ovly_entr
wmb();
regs->addr_ovly = 0x0;
}
+
+static inline void bt455_read_cmap_entry(struct bt455_regs *regs,
+ int cr, u8 *grey)
+{
+ bt455_select_reg(regs, cr);
+ bt455_read_cmap_next(regs, grey);
+}
+
+static inline void bt455_write_cmap_entry(struct bt455_regs *regs,
+ int cr, u8 grey)
+{
+ bt455_select_reg(regs, cr);
+ bt455_write_cmap_next(regs, grey);
+}
+
+static inline void bt455_write_ovly_entry(struct bt455_regs *regs, u8 grey)
+{
+ bt455_reset_reg(regs);
+ bt455_write_ovly_next(regs, grey);
+}
Index: linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/pmag-aa-fb.c
+++ linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
@@ -122,8 +122,8 @@ static int aafb_cursor(struct fb_info *i
u8 bg = cursor->image.bg_color ? 0xf : 0x0;
bt455_write_cmap_entry(par->bt455, 8, bg);
- bt455_write_cmap_entry(par->bt455, 9, bg);
- bt455_write_ovly_entry(par->bt455, 0, fg);
+ bt455_write_cmap_next(par->bt455, bg);
+ bt455_write_ovly_next(par->bt455, fg);
}
if (cursor->set & (FB_CUR_SETSIZE | FB_CUR_SETSHAPE | FB_CUR_SETIMAGE))
bt431_set_cursor(par->bt431,
@@ -212,7 +212,7 @@ static int pmagaafb_probe(struct device
/* Init colormap. */
bt455_write_cmap_entry(par->bt455, 0, 0x0);
- bt455_write_cmap_entry(par->bt455, 1, 0xf);
+ bt455_write_cmap_next(par->bt455, 0xf);
/* Init hardware cursor. */
bt431_erase_cursor(par->bt431);
^ permalink raw reply
* [PATCH 5/7] video: fbdev: pmag-ba-fb: Fix and rework Bt455 colormap handling
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
The Bt455 is a greyscale RAMDAC, using the green color palette entries
only while still providing registers for the red and blue components,
all the three of which have to be loaded on palette updates. Chip
documentation [1] mandates that the unused red and blue registers are
written with 0.
Therefore update code to follow this requirement and given that it makes
the red and blue components unusable remove them from internal API calls
altogether.
References:
[1] "Bt454 Bt455 170 MHz Monolithic CMOS 16 Color Palette RAMDAC",
Brooktree Corporation, Document Number: L454001, Rev. I
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-bt455-cmap-grey.diff
Index: linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/bt455.h
+++ linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
@@ -2,6 +2,7 @@
* linux/drivers/video/bt455.h
*
* Copyright 2003 Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
+ * Copyright 2016 Maciej W. Rozycki <macro@linux-mips.org>
*
* This file is subject to the terms and conditions of the GNU General
* Public License. See the file COPYING in the main directory of this
@@ -32,38 +33,38 @@ static inline void bt455_select_reg(stru
/*
* Read/write to a Bt455 color map register.
*/
-static inline void bt455_read_cmap_entry(struct bt455_regs *regs, int cr,
- u8* red, u8* green, u8* blue)
+static inline void bt455_read_cmap_entry(struct bt455_regs *regs,
+ int cr, u8 *grey)
{
bt455_select_reg(regs, cr);
mb();
- *red = regs->addr_cmap_data & 0x0f;
+ regs->addr_cmap_data;
rmb();
- *green = regs->addr_cmap_data & 0x0f;
+ *grey = regs->addr_cmap_data & 0xf;
rmb();
- *blue = regs->addr_cmap_data & 0x0f;
+ regs->addr_cmap_data;
}
-static inline void bt455_write_cmap_entry(struct bt455_regs *regs, int cr,
- u8 red, u8 green, u8 blue)
+static inline void bt455_write_cmap_entry(struct bt455_regs *regs,
+ int cr, u8 grey)
{
bt455_select_reg(regs, cr);
wmb();
- regs->addr_cmap_data = red & 0x0f;
+ regs->addr_cmap_data = 0x0;
wmb();
- regs->addr_cmap_data = green & 0x0f;
+ regs->addr_cmap_data = grey & 0xf;
wmb();
- regs->addr_cmap_data = blue & 0x0f;
+ regs->addr_cmap_data = 0x0;
}
-static inline void bt455_write_ovly_entry(struct bt455_regs *regs, int cr,
- u8 red, u8 green, u8 blue)
+static inline void bt455_write_ovly_entry(struct bt455_regs *regs,
+ int cr, u8 grey)
{
bt455_select_reg(regs, cr);
wmb();
- regs->addr_ovly = red & 0x0f;
+ regs->addr_ovly = 0x0;
wmb();
- regs->addr_ovly = green & 0x0f;
+ regs->addr_ovly = grey & 0xf;
wmb();
- regs->addr_ovly = blue & 0x0f;
+ regs->addr_ovly = 0x0;
}
Index: linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/pmag-aa-fb.c
+++ linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
@@ -121,9 +121,9 @@ static int aafb_cursor(struct fb_info *i
u8 fg = cursor->image.fg_color ? 0xf : 0x0;
u8 bg = cursor->image.bg_color ? 0xf : 0x0;
- bt455_write_cmap_entry(par->bt455, 8, 0, bg, 0);
- bt455_write_cmap_entry(par->bt455, 9, 0, bg, 0);
- bt455_write_ovly_entry(par->bt455, 0, 0, fg, 0);
+ bt455_write_cmap_entry(par->bt455, 8, bg);
+ bt455_write_cmap_entry(par->bt455, 9, bg);
+ bt455_write_ovly_entry(par->bt455, 0, fg);
}
if (cursor->set & (FB_CUR_SETSIZE | FB_CUR_SETSHAPE | FB_CUR_SETIMAGE))
bt431_set_cursor(par->bt431,
@@ -143,7 +143,7 @@ static int aafb_blank(int blank, struct
struct aafb_par *par = info->par;
u8 val = blank ? 0x00 : 0x0f;
- bt455_write_cmap_entry(par->bt455, 1, val, val, val);
+ bt455_write_cmap_entry(par->bt455, 1, val);
return 0;
}
@@ -211,8 +211,8 @@ static int pmagaafb_probe(struct device
info->screen_size = info->fix.smem_len;
/* Init colormap. */
- bt455_write_cmap_entry(par->bt455, 0, 0x00, 0x00, 0x00);
- bt455_write_cmap_entry(par->bt455, 1, 0x0f, 0x0f, 0x0f);
+ bt455_write_cmap_entry(par->bt455, 0, 0x0);
+ bt455_write_cmap_entry(par->bt455, 1, 0xf);
/* Init hardware cursor. */
bt431_erase_cursor(par->bt431);
^ permalink raw reply
* [PATCH 4/7] video: fbdev: bt455: Remove unneeded colormap helpers for cursor support
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-bt455-cursor.diff
Index: linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/bt455.h
+++ linux-20160211-4maxp64/drivers/video/fbdev/bt455.h
@@ -67,28 +67,3 @@ static inline void bt455_write_ovly_entr
wmb();
regs->addr_ovly = blue & 0x0f;
}
-
-static inline void bt455_set_cursor(struct bt455_regs *regs)
-{
- mb();
- regs->addr_ovly = 0x0f;
- wmb();
- regs->addr_ovly = 0x0f;
- wmb();
- regs->addr_ovly = 0x0f;
-}
-
-static inline void bt455_erase_cursor(struct bt455_regs *regs)
-{
- /* bt455_write_cmap_entry(regs, 8, 0x00, 0x00, 0x00); */
- /* bt455_write_cmap_entry(regs, 9, 0x00, 0x00, 0x00); */
- bt455_write_ovly_entry(regs, 8, 0x03, 0x03, 0x03);
- bt455_write_ovly_entry(regs, 9, 0x07, 0x07, 0x07);
-
- wmb();
- regs->addr_ovly = 0x09;
- wmb();
- regs->addr_ovly = 0x09;
- wmb();
- regs->addr_ovly = 0x09;
-}
^ permalink raw reply
* [PATCH 3/7] video: fbdev: pmag-aa-fb: Report video timings
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
The board uses hardwired timings compatible with 72Hz DEC VR319-DA and
VRM17-AA monitors, according to the board owner's manual[1]. These
timings are accordingly taken from the VR319 manual[2].
References:
[1] "The Monochrome Frame Buffer TURBOchannel Module", Digital Equipment
Corporation, Order Number: EK-MFBOM-TC-001, December 1991
[2] "Installing and Using the VR319 Monochrome Monitor", Digital
Equipment Corporation, Order Number: EK-VR319-IN-001, First Edition,
January 1990, Table 6-1 "Video Timing--1280 x 1024 Resolution"
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-pmag-aa-timing.patch
Index: linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/pmag-aa-fb.c
+++ linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
@@ -79,6 +79,13 @@ static struct fb_var_screeninfo aafb_def
.blue.length = 0,
.activate = FB_ACTIVATE_NOW,
.accel_flags = FB_ACCEL_NONE,
+ .pixclock = 7645,
+ .left_margin = 224,
+ .right_margin = 32,
+ .upper_margin = 33,
+ .lower_margin = 3,
+ .hsync_len = 160,
+ .vsync_len = 3,
.sync = FB_SYNC_ON_GREEN,
.vmode = FB_VMODE_NONINTERLACED,
};
^ permalink raw reply
* [PATCH 2/7] video: fbdev: pmag-aa-fb: Enable building as a module
From: Maciej W. Rozycki @ 2016-02-22 1:55 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
With the current TURBOchannel API support is automagical.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-pmag-aa-module.patch
Index: linux-20160211-4maxp64/drivers/video/fbdev/Kconfig
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/Kconfig
+++ linux-20160211-4maxp64/drivers/video/fbdev/Kconfig
@@ -1808,8 +1808,8 @@ config FB_HIT
frame buffer card.
config FB_PMAG_AA
- bool "PMAG-AA TURBOchannel framebuffer support"
- depends on (FB = y) && TC
+ tristate "PMAG-AA TURBOchannel framebuffer support"
+ depends on FB && TC
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
^ permalink raw reply
* [PATCH 1/7] video: fbdev: pmag-aa-fb: Adapt to current APIs
From: Maciej W. Rozycki @ 2016-02-22 1:54 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1602220110330.7431@eddie.linux-mips.org>
Rework the driver to use the current frambuffer and TURBOchannel APIs,
including proper resource management and using the new framework for
hardware cursor support.
NB two Bt431 cursor generators are included onboard, both responding at
the same TURBOchannel bus addresses and with their host data buses wired
to byte lanes #0 and #1 respectively of the 32-bit bus. Therefore both
can be accessed simultaneously with 16-bit data transfers. Cursor
outputs of the chip wired to lane #0 drive the respective overlay select
inputs of the Bt455 RAMDAC, whereas cursor outputs of the chip wired to
lane #1 drive the respective P3 pixel select inputs of the RAMDAC.
So 5 (out of 17) Bt455 color registers are usable with this board:
palette entries #0 and #1 for frame buffer pixel data driven while
neither cursor generator is active, palette entries #8 and #9 for frame
buffer pixel data driven while cursor generator #1 is active only and
the overlay entry while cursor generator #0 is active.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-pmag-aa-fb.patch
Index: linux-20160211-4maxp64/drivers/video/fbdev/bt431.h
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/bt431.h
+++ linux-20160211-4maxp64/drivers/video/fbdev/bt431.h
@@ -2,6 +2,7 @@
* linux/drivers/video/bt431.h
*
* Copyright 2003 Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
+ * Copyright 2016 Maciej W. Rozycki <macro@linux-mips.org>
*
* This file is subject to the terms and conditions of the GNU General
* Public License. See the file COPYING in the main directory of this
@@ -9,6 +10,8 @@
*/
#include <linux/types.h>
+#define BT431_CURSOR_SIZE 64
+
/*
* Bt431 cursor generator registers, 32-bit aligned.
* Two twin Bt431 are used on the DECstation's PMAG-AA.
@@ -196,28 +199,30 @@ static inline void bt431_position_cursor
bt431_write_reg_inc(regs, (y >> 8) & 0x0f); /* BT431_REG_CYHI */
}
-static inline void bt431_set_font(struct bt431_regs *regs, u8 fgc,
- u16 width, u16 height)
+static inline void bt431_set_cursor(struct bt431_regs *regs,
+ const char *data, const char *mask,
+ u16 rop, u16 width, u16 height)
{
+ u16 x, y;
int i;
- u16 fgp = fgc ? 0xffff : 0x0000;
- u16 bgp = fgc ? 0x0000 : 0xffff;
+ i = 0;
+ width = DIV_ROUND_UP(width, 8);
bt431_select_reg(regs, BT431_REG_CRAM_BASE);
- for (i = BT431_REG_CRAM_BASE; i <= BT431_REG_CRAM_END; i++) {
- u16 value;
-
- if (height << 6 <= i << 3)
- value = bgp;
- else if (width <= i % 8 << 3)
- value = bgp;
- else if (((width >> 3) & 0xffff) > i % 8)
- value = fgp;
- else
- value = fgp & ~(bgp << (width % 8 << 1));
+ for (y = 0; y < BT431_CURSOR_SIZE; y++)
+ for (x = 0; x < BT431_CURSOR_SIZE / 8; x++) {
+ u16 val = 0;
- bt431_write_cmap_inc(regs, value);
- }
+ if (y < height && x < width) {
+ val = mask[i];
+ if (rop = ROP_XOR)
+ val = (val << 8) | (val ^ data[i]);
+ else
+ val = (val << 8) | (val & data[i]);
+ i++;
+ }
+ bt431_write_cmap_inc(regs, val);
+ }
}
static inline void bt431_init_cursor(struct bt431_regs *regs)
Index: linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
=================================--- linux-20160211-4maxp64.orig/drivers/video/fbdev/pmag-aa-fb.c
+++ linux-20160211-4maxp64/drivers/video/fbdev/pmag-aa-fb.c
@@ -8,6 +8,7 @@
* and Harald Koerfgen <hkoerfg@web.de>, which itself is derived from
* "HP300 Topcat framebuffer support (derived from macfb of all things)
* Phil Blundell <philb@gnu.org> 1998"
+ * Copyright (c) 2016 Maciej W. Rozycki
*
* This file is subject to the terms and conditions of the GNU General
* Public License. See the file COPYING in the main directory of this
@@ -21,37 +22,29 @@
*
* 2003-09-21 Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
* Hardware cursor support.
+ *
+ * 2016-02-21 Maciej W. Rozycki <macro@linux-mips.org>
+ * Version 0.03: Rewritten for the new FB and TC APIs.
*/
-#include <linux/module.h>
-#include <linux/kernel.h>
+
+#include <linux/compiler.h>
#include <linux/errno.h>
-#include <linux/string.h>
-#include <linux/timer.h>
-#include <linux/mm.h>
-#include <linux/delay.h>
-#include <linux/init.h>
#include <linux/fb.h>
-#include <linux/console.h>
-
-#include <asm/bootinfo.h>
-#include <asm/dec/machtype.h>
-#include <asm/dec/tc.h>
-
-#include <video/fbcon.h>
-#include <video/fbcon-cfb8.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/tc.h>
+#include <linux/timer.h>
#include "bt455.h"
#include "bt431.h"
/* Version information */
-#define DRIVER_VERSION "0.02"
+#define DRIVER_VERSION "0.03"
#define DRIVER_AUTHOR "Karsten Merker <merker@linuxtag.org>"
#define DRIVER_DESCRIPTION "PMAG-AA Framebuffer Driver"
-/* Prototypes */
-static int aafb_set_var(struct fb_var_screeninfo *var, int con,
- struct fb_info *info);
-
/*
* Bt455 RAM DAC register base offset (rel. to TC slot base address).
*/
@@ -68,443 +61,239 @@ static int aafb_set_var(struct fb_var_sc
*/
#define PMAG_AA_ONBOARD_FBMEM_OFFSET 0x200000
-struct aafb_cursor {
- struct timer_list timer;
- int enable;
- int on;
- int vbl_cnt;
- int blink_rate;
- u16 x, y, width, height;
+struct aafb_par {
+ void __iomem *mmio;
+ struct bt455_regs __iomem *bt455;
+ struct bt431_regs __iomem *bt431;
};
-#define CURSOR_TIMER_FREQ (HZ / 50)
-#define CURSOR_BLINK_RATE (20)
-#define CURSOR_DRAW_DELAY (2)
-
-struct aafb_info {
- struct fb_info info;
- struct display disp;
- struct aafb_cursor cursor;
- struct bt455_regs *bt455;
- struct bt431_regs *bt431;
- unsigned long fb_start;
- unsigned long fb_size;
- unsigned long fb_line_length;
+static struct fb_var_screeninfo aafb_defined = {
+ .xres = 1280,
+ .yres = 1024,
+ .xres_virtual = 2048,
+ .yres_virtual = 1024,
+ .bits_per_pixel = 8,
+ .grayscale = 1,
+ .red.length = 0,
+ .green.length = 1,
+ .blue.length = 0,
+ .activate = FB_ACTIVATE_NOW,
+ .accel_flags = FB_ACCEL_NONE,
+ .sync = FB_SYNC_ON_GREEN,
+ .vmode = FB_VMODE_NONINTERLACED,
};
-/*
- * Max 3 TURBOchannel slots -> max 3 PMAG-AA.
- */
-static struct aafb_info my_fb_info[3];
-
-static struct aafb_par {
-} current_par;
-
-static int currcon = -1;
-
-static void aafb_set_cursor(struct aafb_info *info, int on)
-{
- struct aafb_cursor *c = &info->cursor;
-
- if (on) {
- bt431_position_cursor(info->bt431, c->x, c->y);
- bt431_enable_cursor(info->bt431);
- } else
- bt431_erase_cursor(info->bt431);
-}
-
-static void aafbcon_cursor(struct display *disp, int mode, int x, int y)
-{
- struct aafb_info *info = (struct aafb_info *)disp->fb_info;
- struct aafb_cursor *c = &info->cursor;
-
- x *= fontwidth(disp);
- y *= fontheight(disp);
-
- if (c->x = x && c->y = y && (mode = CM_ERASE) = !c->enable)
- return;
-
- c->enable = 0;
- if (c->on)
- aafb_set_cursor(info, 0);
- c->x = x - disp->var.xoffset;
- c->y = y - disp->var.yoffset;
-
- switch (mode) {
- case CM_ERASE:
- c->on = 0;
- break;
- case CM_DRAW:
- case CM_MOVE:
- if (c->on)
- aafb_set_cursor(info, c->on);
- else
- c->vbl_cnt = CURSOR_DRAW_DELAY;
- c->enable = 1;
- break;
- }
-}
+static struct fb_fix_screeninfo aafb_fix = {
+ .id = "PMAG-AA",
+ .smem_len = (2048 * 1024),
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO10,
+ .ypanstep = 1,
+ .ywrapstep = 1,
+ .line_length = 2048,
+ .mmio_len = PMAG_AA_ONBOARD_FBMEM_OFFSET - PMAG_AA_BT455_OFFSET,
+};
-static int aafbcon_set_font(struct display *disp, int width, int height)
+static int aafb_cursor(struct fb_info *info, struct fb_cursor *cursor)
{
- struct aafb_info *info = (struct aafb_info *)disp->fb_info;
- struct aafb_cursor *c = &info->cursor;
- u8 fgc = ~attr_bgcol_ec(disp, disp->conp, &info->info);
+ struct aafb_par *par = info->par;
- if (width > 64 || height > 64 || width < 0 || height < 0)
+ if (cursor->image.height > BT431_CURSOR_SIZE ||
+ cursor->image.width > BT431_CURSOR_SIZE) {
+ bt431_erase_cursor(par->bt431);
return -EINVAL;
-
- c->height = height;
- c->width = width;
-
- bt431_set_font(info->bt431, fgc, width, height);
-
- return 1;
-}
-
-static void aafb_cursor_timer_handler(unsigned long data)
-{
- struct aafb_info *info = (struct aafb_info *)data;
- struct aafb_cursor *c = &info->cursor;
-
- if (!c->enable)
- goto out;
-
- if (c->vbl_cnt && --c->vbl_cnt = 0) {
- c->on ^= 1;
- aafb_set_cursor(info, c->on);
- c->vbl_cnt = c->blink_rate;
}
-out:
- c->timer.expires = jiffies + CURSOR_TIMER_FREQ;
- add_timer(&c->timer);
-}
-
-static void __init aafb_cursor_init(struct aafb_info *info)
-{
- struct aafb_cursor *c = &info->cursor;
-
- c->enable = 1;
- c->on = 1;
- c->x = c->y = 0;
- c->width = c->height = 0;
- c->vbl_cnt = CURSOR_DRAW_DELAY;
- c->blink_rate = CURSOR_BLINK_RATE;
-
- init_timer(&c->timer);
- c->timer.data = (unsigned long)info;
- c->timer.function = aafb_cursor_timer_handler;
- mod_timer(&c->timer, jiffies + CURSOR_TIMER_FREQ);
-}
-
-static void __exit aafb_cursor_exit(struct aafb_info *info)
-{
- struct aafb_cursor *c = &info->cursor;
-
- del_timer_sync(&c->timer);
-}
-
-static struct display_switch aafb_switch8 = {
- .setup = fbcon_cfb8_setup,
- .bmove = fbcon_cfb8_bmove,
- .clear = fbcon_cfb8_clear,
- .putc = fbcon_cfb8_putc,
- .putcs = fbcon_cfb8_putcs,
- .revc = fbcon_cfb8_revc,
- .cursor = aafbcon_cursor,
- .set_font = aafbcon_set_font,
- .clear_margins = fbcon_cfb8_clear_margins,
- .fontwidthmask = FONTWIDTH(4)|FONTWIDTH(8)|FONTWIDTH(12)|FONTWIDTH(16)
-};
+ if (!cursor->enable)
+ bt431_erase_cursor(par->bt431);
-static void aafb_get_par(struct aafb_par *par)
-{
- *par = current_par;
-}
+ if (cursor->set & FB_CUR_SETPOS)
+ bt431_position_cursor(par->bt431,
+ cursor->image.dx, cursor->image.dy);
+ if (cursor->set & FB_CUR_SETCMAP) {
+ u8 fg = cursor->image.fg_color ? 0xf : 0x0;
+ u8 bg = cursor->image.bg_color ? 0xf : 0x0;
-static int aafb_get_fix(struct fb_fix_screeninfo *fix, int con,
- struct fb_info *info)
-{
- struct aafb_info *ip = (struct aafb_info *)info;
+ bt455_write_cmap_entry(par->bt455, 8, 0, bg, 0);
+ bt455_write_cmap_entry(par->bt455, 9, 0, bg, 0);
+ bt455_write_ovly_entry(par->bt455, 0, 0, fg, 0);
+ }
+ if (cursor->set & (FB_CUR_SETSIZE | FB_CUR_SETSHAPE | FB_CUR_SETIMAGE))
+ bt431_set_cursor(par->bt431,
+ cursor->image.data, cursor->mask, cursor->rop,
+ cursor->image.width, cursor->image.height);
- memset(fix, 0, sizeof(struct fb_fix_screeninfo));
- strcpy(fix->id, "PMAG-AA");
- fix->smem_start = ip->fb_start;
- fix->smem_len = ip->fb_size;
- fix->type = FB_TYPE_PACKED_PIXELS;
- fix->ypanstep = 1;
- fix->ywrapstep = 1;
- fix->visual = FB_VISUAL_MONO10;
- fix->line_length = 1280;
- fix->accel = FB_ACCEL_NONE;
+ if (cursor->enable)
+ bt431_enable_cursor(par->bt431);
return 0;
}
-static void aafb_set_disp(struct display *disp, int con,
- struct aafb_info *info)
-{
- struct fb_fix_screeninfo fix;
-
- disp->fb_info = &info->info;
- aafb_set_var(&disp->var, con, &info->info);
- if (disp->conp && disp->conp->vc_sw && disp->conp->vc_sw->con_cursor)
- disp->conp->vc_sw->con_cursor(disp->conp, CM_ERASE);
- disp->dispsw = &aafb_switch8;
- disp->dispsw_data = 0;
-
- aafb_get_fix(&fix, con, &info->info);
- disp->screen_base = (u8 *) fix.smem_start;
- disp->visual = fix.visual;
- disp->type = fix.type;
- disp->type_aux = fix.type_aux;
- disp->ypanstep = fix.ypanstep;
- disp->ywrapstep = fix.ywrapstep;
- disp->line_length = fix.line_length;
- disp->next_line = 2048;
- disp->can_soft_blank = 1;
- disp->inverse = 0;
- disp->scrollmode = SCROLL_YREDRAW;
-
- aafbcon_set_font(disp, fontwidth(disp), fontheight(disp));
-}
+/* 0 unblanks, any other blanks. */
-static int aafb_get_cmap(struct fb_cmap *cmap, int kspc, int con,
- struct fb_info *info)
+static int aafb_blank(int blank, struct fb_info *info)
{
- static u16 color[2] = {0x0000, 0x000f};
- static struct fb_cmap aafb_cmap = {0, 2, color, color, color, NULL};
+ struct aafb_par *par = info->par;
+ u8 val = blank ? 0x00 : 0x0f;
- fb_copy_cmap(&aafb_cmap, cmap, kspc ? 0 : 2);
+ bt455_write_cmap_entry(par->bt455, 1, val, val, val);
return 0;
}
-static int aafb_set_cmap(struct fb_cmap *cmap, int kspc, int con,
- struct fb_info *info)
-{
- u16 color[2] = {0x0000, 0x000f};
-
- if (cmap->start = 0
- && cmap->len = 2
- && memcmp(cmap->red, color, sizeof(color)) = 0
- && memcmp(cmap->green, color, sizeof(color)) = 0
- && memcmp(cmap->blue, color, sizeof(color)) = 0
- && cmap->transp = NULL)
- return 0;
- else
- return -EINVAL;
-}
-
-static int aafb_ioctl(struct fb_info *info, u32 cmd, unsigned long arg)
-{
- /* TODO: Not yet implemented */
- return -ENOIOCTLCMD;
-}
+static struct fb_ops aafb_ops = {
+ .owner = THIS_MODULE,
+ .fb_blank = aafb_blank,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
+ .fb_cursor = aafb_cursor,
+};
-static int aafb_switch(int con, struct fb_info *info)
+static int pmagaafb_probe(struct device *dev)
{
- struct aafb_info *ip = (struct aafb_info *)info;
- struct display *old = (currcon < 0) ? &ip->disp : (fb_display + currcon);
- struct display *new = (con < 0) ? &ip->disp : (fb_display + con);
-
- if (old->conp && old->conp->vc_sw && old->conp->vc_sw->con_cursor)
- old->conp->vc_sw->con_cursor(old->conp, CM_ERASE);
+ struct tc_dev *tdev = to_tc_dev(dev);
+ resource_size_t start, len;
+ struct fb_info *info;
+ struct aafb_par *par;
+ int err;
- /* Set the current console. */
- currcon = con;
- aafb_set_disp(new, con, ip);
+ info = framebuffer_alloc(sizeof(struct aafb_par), dev);
+ if (!info) {
+ printk(KERN_ERR "%s: Cannot allocate memory\n", dev_name(dev));
+ return -ENOMEM;
+ }
- return 0;
-}
+ par = info->par;
+ dev_set_drvdata(dev, info);
-static void aafb_encode_var(struct fb_var_screeninfo *var,
- struct aafb_par *par)
-{
- var->xres = 1280;
- var->yres = 1024;
- var->xres_virtual = 2048;
- var->yres_virtual = 1024;
- var->xoffset = 0;
- var->yoffset = 0;
- var->bits_per_pixel = 8;
- var->grayscale = 1;
- var->red.offset = 0;
- var->red.length = 0;
- var->red.msb_right = 0;
- var->green.offset = 0;
- var->green.length = 1;
- var->green.msb_right = 0;
- var->blue.offset = 0;
- var->blue.length = 0;
- var->blue.msb_right = 0;
- var->transp.offset = 0;
- var->transp.length = 0;
- var->transp.msb_right = 0;
- var->nonstd = 0;
- var->activate &= ~FB_ACTIVATE_MASK & FB_ACTIVATE_NOW;
- var->accel_flags = 0;
- var->sync = FB_SYNC_ON_GREEN;
- var->vmode &= ~FB_VMODE_MASK & FB_VMODE_NONINTERLACED;
-}
+ info->fbops = &aafb_ops;
+ info->fix = aafb_fix;
+ info->var = aafb_defined;
+ info->flags = FBINFO_DEFAULT;
-static int aafb_get_var(struct fb_var_screeninfo *var, int con,
- struct fb_info *info)
-{
- if (con < 0) {
- struct aafb_par par;
+ /* Request the I/O MEM resource. */
+ start = tdev->resource.start;
+ len = tdev->resource.end - start + 1;
+ if (!request_mem_region(start, len, dev_name(dev))) {
+ printk(KERN_ERR "%s: Cannot reserve FB region\n",
+ dev_name(dev));
+ err = -EBUSY;
+ goto err_alloc;
+ }
- memset(var, 0, sizeof(struct fb_var_screeninfo));
- aafb_get_par(&par);
- aafb_encode_var(var, &par);
- } else
- *var = info->var;
+ /* MMIO mapping setup. */
+ info->fix.mmio_start = start + PMAG_AA_BT455_OFFSET;
+ par->mmio = ioremap_nocache(info->fix.mmio_start, info->fix.mmio_len);
+ if (!par->mmio) {
+ printk(KERN_ERR "%s: Cannot map MMIO\n", dev_name(dev));
+ err = -ENOMEM;
+ goto err_resource;
+ }
+ par->bt455 = par->mmio - PMAG_AA_BT455_OFFSET + PMAG_AA_BT455_OFFSET;
+ par->bt431 = par->mmio - PMAG_AA_BT455_OFFSET + PMAG_AA_BT431_OFFSET;
- return 0;
-}
+ /* Frame buffer mapping setup. */
+ info->fix.smem_start = start + PMAG_AA_ONBOARD_FBMEM_OFFSET;
+ info->screen_base = ioremap_nocache(info->fix.smem_start,
+ info->fix.smem_len);
+ if (!info->screen_base) {
+ printk(KERN_ERR "%s: Cannot map FB\n", dev_name(dev));
+ err = -ENOMEM;
+ goto err_mmio_map;
+ }
+ info->screen_size = info->fix.smem_len;
-static int aafb_set_var(struct fb_var_screeninfo *var, int con,
- struct fb_info *info)
-{
- struct aafb_par par;
+ /* Init colormap. */
+ bt455_write_cmap_entry(par->bt455, 0, 0x00, 0x00, 0x00);
+ bt455_write_cmap_entry(par->bt455, 1, 0x0f, 0x0f, 0x0f);
- aafb_get_par(&par);
- aafb_encode_var(var, &par);
- info->var = *var;
+ /* Init hardware cursor. */
+ bt431_erase_cursor(par->bt431);
+ bt431_init_cursor(par->bt431);
- return 0;
-}
+ err = register_framebuffer(info);
+ if (err < 0) {
+ printk(KERN_ERR "%s: Cannot register framebuffer\n",
+ dev_name(dev));
+ goto err_smem_map;
+ }
-static int aafb_update_var(int con, struct fb_info *info)
-{
- struct aafb_info *ip = (struct aafb_info *)info;
- struct display *disp = (con < 0) ? &ip->disp : (fb_display + con);
+ get_device(dev);
- if (con = currcon)
- aafbcon_cursor(disp, CM_ERASE, ip->cursor.x, ip->cursor.y);
+ pr_info("fb%d: %s frame buffer device at %s\n",
+ info->node, info->fix.id, dev_name(dev));
return 0;
-}
-
-/* 0 unblanks, any other blanks. */
-static void aafb_blank(int blank, struct fb_info *info)
-{
- struct aafb_info *ip = (struct aafb_info *)info;
- u8 val = blank ? 0x00 : 0x0f;
- bt455_write_cmap_entry(ip->bt455, 1, val, val, val);
- aafbcon_cursor(&ip->disp, CM_ERASE, ip->cursor.x, ip->cursor.y);
-}
-
-static struct fb_ops aafb_ops = {
- .owner = THIS_MODULE,
- .fb_get_fix = aafb_get_fix,
- .fb_get_var = aafb_get_var,
- .fb_set_var = aafb_set_var,
- .fb_get_cmap = aafb_get_cmap,
- .fb_set_cmap = aafb_set_cmap,
- .fb_ioctl = aafb_ioctl
-};
-
-static int __init init_one(int slot)
-{
- unsigned long base_addr = CKSEG1ADDR(get_tc_base_addr(slot));
- struct aafb_info *ip = &my_fb_info[slot];
-
- memset(ip, 0, sizeof(struct aafb_info));
-
- /*
- * Framebuffer display memory base address and friends.
- */
- ip->bt455 = (struct bt455_regs *) (base_addr + PMAG_AA_BT455_OFFSET);
- ip->bt431 = (struct bt431_regs *) (base_addr + PMAG_AA_BT431_OFFSET);
- ip->fb_start = base_addr + PMAG_AA_ONBOARD_FBMEM_OFFSET;
- ip->fb_size = 2048 * 1024; /* fb_fix_screeninfo.smem_length
- seems to be physical */
- ip->fb_line_length = 2048;
-
- /*
- * Let there be consoles..
- */
- strcpy(ip->info.modename, "PMAG-AA");
- ip->info.node = -1;
- ip->info.flags = FBINFO_FLAG_DEFAULT;
- ip->info.fbops = &aafb_ops;
- ip->info.disp = &ip->disp;
- ip->info.changevar = NULL;
- ip->info.switch_con = &aafb_switch;
- ip->info.updatevar = &aafb_update_var;
- ip->info.blank = &aafb_blank;
-
- aafb_set_disp(&ip->disp, currcon, ip);
-
- /*
- * Configure the RAM DACs.
- */
- bt455_erase_cursor(ip->bt455);
-
- /* Init colormap. */
- bt455_write_cmap_entry(ip->bt455, 0, 0x00, 0x00, 0x00);
- bt455_write_cmap_entry(ip->bt455, 1, 0x0f, 0x0f, 0x0f);
-
- /* Init hardware cursor. */
- bt431_init_cursor(ip->bt431);
- aafb_cursor_init(ip);
-
- /* Clear the screen. */
- memset ((void *)ip->fb_start, 0, ip->fb_size);
+err_smem_map:
+ iounmap(info->screen_base);
- if (register_framebuffer(&ip->info) < 0)
- return -EINVAL;
+err_mmio_map:
+ iounmap(par->mmio);
- printk(KERN_INFO "fb%d: %s frame buffer in TC slot %d\n",
- GET_FB_IDX(ip->info.node), ip->info.modename, slot);
+err_resource:
+ release_mem_region(start, len);
- return 0;
+err_alloc:
+ framebuffer_release(info);
+ return err;
}
-static int __exit exit_one(int slot)
+static int __exit pmagaafb_remove(struct device *dev)
{
- struct aafb_info *ip = &my_fb_info[slot];
-
- if (unregister_framebuffer(&ip->info) < 0)
- return -EINVAL;
+ struct tc_dev *tdev = to_tc_dev(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
+ struct aafb_par *par = info->par;
+ resource_size_t start, len;
+ put_device(dev);
+ unregister_framebuffer(info);
+ iounmap(info->screen_base);
+ iounmap(par->mmio);
+ start = tdev->resource.start;
+ len = tdev->resource.end - start + 1;
+ release_mem_region(start, len);
+ framebuffer_release(info);
return 0;
}
/*
* Initialise the framebuffer.
*/
-int __init pmagaafb_init(void)
-{
- int sid;
- int found = 0;
+static const struct tc_device_id pmagaafb_tc_table[] = {
+ { "DEC ", "PMAG-AA " },
+ { }
+};
+MODULE_DEVICE_TABLE(tc, pmagaafb_tc_table);
- while ((sid = search_tc_card("PMAG-AA")) >= 0) {
- found = 1;
- claim_tc_card(sid);
- init_one(sid);
- }
+static struct tc_driver pmagaafb_driver = {
+ .id_table = pmagaafb_tc_table,
+ .driver = {
+ .name = "pmagaafb",
+ .bus = &tc_bus_type,
+ .probe = pmagaafb_probe,
+ .remove = __exit_p(pmagaafb_remove),
+ },
+};
- return found ? 0 : -ENXIO;
+static int __init pmagaafb_init(void)
+{
+#ifndef MODULE
+ if (fb_get_options("pmagaafb", NULL))
+ return -ENXIO;
+#endif
+ return tc_register_driver(&pmagaafb_driver);
}
static void __exit pmagaafb_exit(void)
{
- int sid;
-
- while ((sid = search_tc_card("PMAG-AA")) >= 0) {
- exit_one(sid);
- release_tc_card(sid);
- }
+ tc_unregister_driver(&pmagaafb_driver);
}
+module_init(pmagaafb_init);
+module_exit(pmagaafb_exit);
+
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
MODULE_LICENSE("GPL");
-#ifdef MODULE
-module_init(pmagaafb_init);
-module_exit(pmagaafb_exit);
-#endif
^ permalink raw reply
* [PATCH 0/7] PMAG-AA frame buffer driver rework
From: Maciej W. Rozycki @ 2016-02-22 1:54 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Geert Uytterhoeven
Cc: linux-fbdev, linux-kernel
Hi,
This is an update to the PMAG-AA frame buffer driver to adapt it to our
current APIs, which I promised a while ago (too much of a while, which I
apologise for).
This series starts with a change that makes the driver build and retains
its previous functionality. A bunch of further clean-ups follow, each of
which is explained individually. There's a potential in this code for
further clean-ups and improvements, but the weekend is running out quick
now, so I had to stop somewhere. Next time then.
A couple of checkpatch.pl warnings are noted.
First on using `printk(KERN_ERR ...' rather tha `pr_err'. I could
probably fix it, but I think it's not worth it by itself, the interface is
not wrong and the issue is mainly cosmetical. I think I'll just go across
all the pmag* drivers at once and bulk convert these calls sometime.
Second on using `mb()' (without a comment). This is not a new problem as
the driver has lots of these calls throughout. Fixing them is blocked on
the unavailability of generic cross-platform MMIO synchronisation and
completion barriers. See the discussion at:
<http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=alpine.LFD.2.11.1404280048540.11598%40eddie.linux-mips.org>
for details. Most TURBOchannel drivers suffer from this problem due to
the complexities around many MIPS processors being weakly ordered,
including in particular those used in TURBOchannel systems. Once the
generic issue of MMIO barriers has been sorted out the hacks to use memory
barriers (which just happen to DTRT on the MIPS platform, because there's
just a single underlying mechanism for all kinds of barriers in the MIPS
architecture) in their place can be removed.
All of these pieces have been visually tested with a live specimen. VT
only however at this point, no X11.
Please apply,
Maciej
^ permalink raw reply
* [PATCH] video: fbdev: pmag-ba-fb: Fix the lower margin size
From: Maciej W. Rozycki @ 2016-02-22 1:54 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: linux-fbdev, linux-kernel
According to the board specification[1] the width of the vertical sync
front porch is 12 pixels or the same as the width of the horizontal sync
front porch. This in turn means the size of the lower margin is 0,
because the vertical sync starts as soon as the start of the horizontal
sync terminates the last line.
References:
[1] "PMAG-BA TURBOchannel Color Frame Buffer Functional Specification",
Revision 1.2, Workstation Systems Engineering, Digital Equipment
Corporation, August 27, 1990, Table 3-5: "Video Timing"
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
linux-pmag-ba-lower.patch
Index: linux-20150524-4maxp64/drivers/video/fbdev/pmag-ba-fb.c
=================================--- linux-20150524-4maxp64.orig/drivers/video/fbdev/pmag-ba-fb.c
+++ linux-20150524-4maxp64/drivers/video/fbdev/pmag-ba-fb.c
@@ -60,7 +60,7 @@ static struct fb_var_screeninfo pmagbafb
.left_margin = 116,
.right_margin = 12,
.upper_margin = 34,
- .lower_margin = 12,
+ .lower_margin = 0,
.hsync_len = 128,
.vsync_len = 3,
.sync = FB_SYNC_ON_GREEN,
^ permalink raw reply
* Re: [PATCH 11/11] ARM: versatile: move CLCD configuration to device tree
From: Linus Walleij @ 2016-02-21 22:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <56C5B080.9090007@ti.com>
On Thu, Feb 18, 2016 at 12:52 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> For panels we need DT fragments. The question is where these fragments
> are and, possibly, who who loads them.
I hacked something up that augments the device tree from the kernel,
given you have a node with all the props you want to augment, tell me
what you think of this and whether I should continue in this direction...
also the DT people need to be involved:
#include "../../drivers/of/of_private.h"
/* Obviously make the property duplication function public instead,
it's a test */
struct versatile_panel {
u32 id;
char *compatible;
u32 clock_frequency;
u32 pixelclk_active;
u32 hsync_active;
u32 vsync_active;
u32 de_active;
u32 hactive;
u32 hback_porch;
u32 hfront_porch;
u32 hsync_len;
u32 vactive;
u32 vback_porch;
u32 vfront_porch;
u32 vsync_len;
bool ib2;
};
static const struct versatile_panel versatile_panels[] = {
{
.id = SYS_CLCD_ID_VGA,
.compatible = "VGA",
.clock_frequency = 25175000,
.pixelclk_active = 0,
.hsync_active = 1,
.vsync_active = 1,
.de_active = 1,
.hactive = 640,
.hback_porch = 48,
.hfront_porch = 16,
.hsync_len = 96,
.vactive = 480,
.vback_porch = 33,
.vfront_porch = 10,
.vsync_len = 2,
},
{
.id = SYS_CLCD_ID_SANYO_3_8,
.compatible = "sanyo,tm38qv67a02a",
.clock_frequency = 10000000,
.pixelclk_active = 1,
.hsync_active = 1,
.vsync_active = 1,
.de_active = 1,
.hactive = 320,
.hback_porch = 6,
.hfront_porch = 6,
.hsync_len = 6,
.vactive = 240,
.vback_porch = 6,
.vfront_porch = 6,
.vsync_len = 0,
},
{
.id = SYS_CLCD_ID_SHARP_8_4,
.compatible = "sharp,lq084v1dg21",
.clock_frequency = 25175000,
.pixelclk_active = 0,
.hsync_active = 1,
.vsync_active = 1,
.de_active = 1,
.hactive = 640,
.hback_porch = 48,
.hfront_porch = 16,
.hsync_len = 96,
.vactive = 480,
.vback_porch = 33,
.vfront_porch = 10,
.vsync_len = 2,
},
{
.id = SYS_CLCD_ID_EPSON_2_2,
.compatible = "epson,l2f50113t00",
.clock_frequency = 16000000,
.pixelclk_active = 0,
.hsync_active = 1,
.vsync_active = 1,
.de_active = 1,
.hactive = 176,
.hback_porch = 3,
.hfront_porch = 2,
.hsync_len = 3,
.vactive = 220,
.vback_porch = 1,
.vfront_porch = 0,
.vsync_len = 2,
},
{
.id = SYS_CLCD_ID_SANYO_2_5,
.compatible = "sanyo,alr252rgt",
.ib2 = true,
.clock_frequency = 5440000,
.pixelclk_active = 0,
.hsync_active = 0,
.vsync_active = 0,
.de_active = 1,
.hactive = 240,
.hback_porch = 20,
.hfront_porch = 10,
.hsync_len = 10,
.vactive = 320,
.vback_porch = 2,
.vfront_porch = 2,
.vsync_len = 2,
},
};
static void update_timings_prop(struct device *dev,
struct of_changeset *cset,
struct device_node *timings,
const char *propname,
u32 val)
{
struct property *prop, *new;
__be32 *dt_val;
prop = of_find_property(timings, propname, NULL);
if (!prop) {
dev_err(dev, "could not find property \"%s\" - skipping\n",
propname);
return;
}
new = __of_prop_dup(prop, GFP_KERNEL);
if (!new) {
dev_err(dev, "could not copy property \"%s\" - skipping\n",
propname);
return;
}
dt_val = new->value;
*dt_val = cpu_to_be32(val);
of_changeset_update_property(cset, timings, new);
}
static int versatile_overwrite_of_panel(struct device *dev,
struct device_node *panel,
const struct versatile_panel *vpanel)
{
struct of_changeset cset;
struct device_node *timings;
int ret;
dev_info(dev, "CLCD: overwriting device tree\n");
of_changeset_init(&cset);
/* Find the timings node */
timings = of_get_child_by_name(panel, "panel-timing");
if (!timings) {
dev_err(dev, "could not find panel timing node\n");
goto err_destroy_cs;
}
update_timings_prop(dev, &cset, timings, "clock-frequency",
vpanel->clock_frequency);
update_timings_prop(dev, &cset, timings, "pixelclk-active",
vpanel->pixelclk_active);
update_timings_prop(dev, &cset, timings, "hsync-active",
vpanel->hsync_active);
update_timings_prop(dev, &cset, timings, "vsync-active",
vpanel->vsync_active);
update_timings_prop(dev, &cset, timings, "de-active",
vpanel->de_active);
update_timings_prop(dev, &cset, timings, "hactive",
vpanel->hactive);
update_timings_prop(dev, &cset, timings, "hback-porch",
vpanel->hback_porch);
update_timings_prop(dev, &cset, timings, "hsync-len",
vpanel->hsync_len);
update_timings_prop(dev, &cset, timings, "vactive",
vpanel->vactive);
update_timings_prop(dev, &cset, timings, "vback-porch",
vpanel->vback_porch);
update_timings_prop(dev, &cset, timings, "vfront-porch",
vpanel->vfront_porch);
update_timings_prop(dev, &cset, timings, "vsync-len",
vpanel->hsync_len);
ret = of_changeset_apply(&cset);
if (ret) {
dev_err(dev, "could not apply device tree changeset\n");
goto err_destroy_cs;
}
return ret;
err_destroy_cs:
of_changeset_destroy(&cset);
return ret;
}
static void versatile_panel_probe(struct device *dev,
struct device_node *endpoint)
{
struct versatile_panel const *vpanel = NULL;
struct device_node *panel = NULL;
u32 val;
int ret;
int i;
/*
* The Versatile CLCD has a panel auto-detection mechanism.
* We use this and look for the compatible panel in the
* device tree.
*/
ret = regmap_read(versatile_syscon_map, SYS_CLCD, &val);
if (ret) {
dev_err(dev, "cannot read CLCD syscon register\n");
return;
}
val &= SYS_CLCD_CLCDID_MASK;
dev_info(dev, "SYS_CLCD=%08x\n", val);
/* First find corresponding panel information */
for (i = 0; i < ARRAY_SIZE(versatile_panels); i++) {
vpanel = &versatile_panels[i];
if (val = vpanel->id) {
dev_err(dev, "autodetected panel \"%s\"\n",
vpanel->compatible);
break;
}
}
if (i = ARRAY_SIZE(versatile_panels)) {
dev_err(dev, "could not auto-detect panel\n");
return;
}
/* This is the default panel node in the device tree */
panel = of_graph_get_remote_port_parent(endpoint);
if (!panel) {
dev_err(dev, "could not locate remote port for panel\n");
return;
}
/*
* So now we dynamically update the display properties of the
* panel in accordance to what was detected..
*/
ret = versatile_overwrite_of_panel(dev, panel, vpanel);
if (ret) {
dev_err(dev, "cannot overwrite devicetree panel\n");
return;
}
/*
* If we have a Sanyo 2.5" port
* that we're running on an IB2 and proceed to look for the
* IB2 syscon regmap.
*/
if (!vpanel->ib2)
return;
versatile_ib2_map = syscon_regmap_lookup_by_compatible(
"arm,versatile-ib2-syscon");
if (IS_ERR(versatile_ib2_map)) {
dev_err(dev, "could not locate IB2 control register\n");
versatile_ib2_map = NULL;
return;
}
}
This actually works. But would need some public device tree
property augmentation API instead of the hacks. Should I pursue
it?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [02/11] video: ARM CLCD: support DT signal inversion flags
From: Linus Walleij @ 2016-02-20 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ac0739e5-4339-7fc5-1179-73af722beb08@broadcom.com>
On Sat, Feb 20, 2016 at 2:23 AM, Ray Jui <ray.jui@broadcom.com> wrote:
>> Do you have a system that needs this?
>
> Yah, we have a system (Cygnus) that can either use the HCLK, or an external
> clock generated from a PLL. To get certain pixel clock frequencies
> (depending on the LCD panel we use), we need to use the external clock
> source with the PLL configured to a particular VCO frequency.
>
> We have not enabled full LCD/video support on that platform in the mainline
> kernel, but we'll eventually get to it.
OK I hope to finish these patches soon so you have some nice stuff
to build upon. I guess it will need the same per-vendor plug-in that
I supplied for the ARMs and the Nomadik.
Yours,
Linus Walleij
^ 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