* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-07 20:25 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1438901893.3109.72.camel@hp.com>
On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
>> On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
>> > > > > > On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
>> > > > There are two usages on MTRRs:
>> > > > 1) MTRR entries set by firmware
>> > > > 2) MTRR entries set by OS drivers
>> > > >
>> > > > We can obsolete 2), but we have no control over 1). As UEFI
>> > > > firmwares
>> > > > also set this up, this usage will continue to stay. So, we should
>> > > > not
>> > > > get rid of the MTRR code that looks up the MTRR entries, while we
>> > > > have
>> > > > no need to modify them.
>> > > >
>> > > > Such MTRR entries provide safe guard to /dev/mem, which allows
>> > > > privileged user to access a range that may require UC mapping while
>> > > > the /dev/mem driver blindly maps it with WB. MTRRs converts WB to
>> > > > UC in
>> > > > such a case.
>> > >
>> > > But it wouldn't be impossible to simply read the MTRRs upon boot,
>> > > store the information, disable MTRRs, and correctly use PAT to
>> > > achieve the same effect (i.e. the "blindly maps" part of course
>> > > would need fixing).
>> >
>> > It could be done, but I do not see much benefit of doing it. One of the
>> > reasons platform vendors set MTRRs is so that a system won't hit a
>> > machine check when an OS bug leads an access with a wrong cache type.
>> >
>> > A machine check is hard to analyze and can be seen as a hardware issue
>> > by customers. Emulating MTRRs with PAT won't protect from such a bug.
>>
>> That's seems like a fair and valid concern. This could only happen if
>> the OS would have code that would use MTRR, in the case of Linux we'll
>> soon be able to vet that this cannot happen.
>
> No, there is no OS support necessary to use MTRR. After firmware sets it
> up, CPUs continue to use it without any OS support. I think the Linux
> change you are referring is to obsolete legacy interfaces that modify the
> MTRR setup. I agree that Linux should not modify MTRR.
Its a bit more than that though. Since you agree that the OS can live
without MTRR code I was hoping to then see if we can fold out PAT
Linux code from under the MTRR dependency on Linux and make PAT a
first class citizen, maybe at least for x86-64. Right now you can only
get PAT support on Linux if you have MTRR code, but I'd like to see if
instead we can rip MTRR code out completely under its own Kconfig and
let it start rotting away.
Code-wise the only issue I saw was that PAT code also relies on
mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
no other obvious issues.
Platform firmware and SMIs seems to be the only other possible issue.
More on this below.
>> For those type of OSes...
>> could it be possible to negotiate or hint to the platform through an
>> attribute somehow that the OS has such capability to not use MTRR?
>
> The OS can disable MTRR. However, this can also cause a problem in
> firmware, which may rely on MTRR.
Can you describe what type of issues we could expect ? I tend to care
more about this for 64-bit systems so if 32-bit platforms would be
more of the ones which could cause an issue would restricting
disabling MTRR only for 64-bit help?
>> Then, only if this bit is set, the platform could then avoid such MTRR
>> settings, and if we have issues you can throw rocks at us.
>
>> And if that's not possible how about a new platform setting that would
>> need to be set at the platform level to enable disabling this junk?
>> Then only folks who know what they are doing would enable it, and if
>> the customer set it, the issue would not be on the platform.
>
>> Could this also be used to prevent SMIs with MTRRs?
>
> ACPI _OSI could be used for firmware to implement some OS-specific features,
> but it may be too late for firmware to make major changes and is generally
> useless unless OS requirements are described in a spec backed by logo
> certification.
I see.. So there are no guarantees that platform firmware would not
expect OS MTRR support.
> SMIs are also used for platform management, such as fan
> speed control.
And its conceivable that some devices, or the platform itself, may
trigger SMIs to have the platform firmware poke with MTRRs?
> Is there any issue for Linux to use MTRR set by firmware?
Even though we don't have the Kconfig option right now to disable MTRR
cod explicitly I'll note that there are a few other cases that could
flip Linux to note use MTRR:
a) Some BIOSes could let MTRR get disabled
b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
disables MTRR on Linux
If these environments can exist it'd be good to understand possible
issues that could creep up as a result of the OS not having MTRR
enabled. If this is a reasonable thing for x86-64 I was hoping we
could just let users opt-in to a similar build configuration through
the OS by letting PAT not depend on MTRR.
Luis
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-08-07 21:56 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <CAB=NE6VnspTPfrn5+ZFSdgKb3uh_4g7LsuZVwe2FET=noijr5Q@mail.gmail.com>
On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
> On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
> > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
> > > > On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
> > > > > > > > On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
> > > > > > There are two usages on MTRRs:
> > > > > > 1) MTRR entries set by firmware
> > > > > > 2) MTRR entries set by OS drivers
> > > > > >
> > > > > > We can obsolete 2), but we have no control over 1). As UEFI
> > > > > > firmwares also set this up, this usage will continue to stay.
> > > > > > So, we should not get rid of the MTRR code that looks up the
> > > > > > MTRR entries, while we have no need to modify them.
> > > > > >
> > > > > > Such MTRR entries provide safe guard to /dev/mem, which allows
> > > > > > privileged user to access a range that may require UC mapping
> > > > > > while the /dev/mem driver blindly maps it with WB. MTRRs
> > > > > > converts WB to UC in such a case.
> > > > >
> > > > > But it wouldn't be impossible to simply read the MTRRs upon boot,
> > > > > store the information, disable MTRRs, and correctly use PAT to
> > > > > achieve the same effect (i.e. the "blindly maps" part of course
> > > > > would need fixing).
> > > >
> > > > It could be done, but I do not see much benefit of doing it. One of
> > > > the reasons platform vendors set MTRRs is so that a system won't hit
> > > > a machine check when an OS bug leads an access with a wrong cache
> > > > type.
> > > >
> > > > A machine check is hard to analyze and can be seen as a hardware
> > > > issue by customers. Emulating MTRRs with PAT won't protect from
> > > > such a bug.
> > >
> > > That's seems like a fair and valid concern. This could only happen if
> > > the OS would have code that would use MTRR, in the case of Linux we'll
> > > soon be able to vet that this cannot happen.
> >
> > No, there is no OS support necessary to use MTRR. After firmware sets
> > it up, CPUs continue to use it without any OS support. I think the
> > Linux change you are referring is to obsolete legacy interfaces that
> > modify the MTRR setup. I agree that Linux should not modify MTRR.
>
> Its a bit more than that though. Since you agree that the OS can live
> without MTRR code I was hoping to then see if we can fold out PAT
> Linux code from under the MTRR dependency on Linux and make PAT a
> first class citizen, maybe at least for x86-64. Right now you can only
> get PAT support on Linux if you have MTRR code, but I'd like to see if
> instead we can rip MTRR code out completely under its own Kconfig and
> let it start rotting away.
>
> Code-wise the only issue I saw was that PAT code also relies on
> mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
> no other obvious issues.
We can rip of the MTTR code that modifies the MTRR setup, but not
mtrr_type_lookup(). This function provides necessary checks per documented
in commit 7f0431e3dc89 as follows.
1) reserve_memtype() tracks an effective memory type in case
a request type is WB (ex. /dev/mem blindly uses WB). Missing
to track with its effective type causes a subsequent request
to map the same range with the effective type to fail.
2) pud_set_huge() and pmd_set_huge() check if a requested range
has any overlap with MTRRs. Missing to detect an overlap may
cause a performance penalty or undefined behavior.
mtrr_type_lookup() is still admittedly awkward, but I do not think we have
an immediate issue in PAT code calling it. I do not think it makes PAT code
a second class citizen.
> Platform firmware and SMIs seems to be the only other possible issue.
> More on this below.
>
> > > For those type of OSes...
> > > could it be possible to negotiate or hint to the platform through an
> > > attribute somehow that the OS has such capability to not use MTRR?
> >
> > The OS can disable MTRR. However, this can also cause a problem in
> > firmware, which may rely on MTRR.
>
> Can you describe what type of issues we could expect ? I tend to care
> more about this for 64-bit systems so if 32-bit platforms would be
> more of the ones which could cause an issue would restricting
> disabling MTRR only for 64-bit help?
The SMI handler runs in real-mode and relies on MTRR being effective to
provide right cache types. It does not matter if it is 64-bit or not.
> > > Then, only if this bit is set, the platform could then avoid such MTRR
> > > settings, and if we have issues you can throw rocks at us.
> >
> > > And if that's not possible how about a new platform setting that would
> > > need to be set at the platform level to enable disabling this junk?
> > > Then only folks who know what they are doing would enable it, and if
> > > the customer set it, the issue would not be on the platform.
> >
> > > Could this also be used to prevent SMIs with MTRRs?
> >
> > ACPI _OSI could be used for firmware to implement some OS-specific
> > features, but it may be too late for firmware to make major changes and
> > is generally useless unless OS requirements are described in a spec
> > backed by logo certification.
>
> I see.. So there are no guarantees that platform firmware would not
> expect OS MTRR support.
>
> > SMIs are also used for platform management, such as fan
> > speed control.
>
> And its conceivable that some devices, or the platform itself, may
> trigger SMIs to have the platform firmware poke with MTRRs?
SMIs are outside of OS control. SMI handler relies on MTRR being set. SMI
must be quick, so the handler should not be required to initialize MTRR or
page tables.
> > Is there any issue for Linux to use MTRR set by firmware?
>
> Even though we don't have the Kconfig option right now to disable MTRR
> cod explicitly I'll note that there are a few other cases that could
> flip Linux to note use MTRR:
>
> a) Some BIOSes could let MTRR get disabled
> b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
> disables MTRR on Linux
>
> If these environments can exist it'd be good to understand possible
> issues that could creep up as a result of the OS not having MTRR
> enabled. If this is a reasonable thing for x86-64 I was hoping we
> could just let users opt-in to a similar build configuration through
> the OS by letting PAT not depend on MTRR.
Case a) and b) do not cause any issue. They simply lead mtrr_type_lookup()
to return immediately with MTRR_TYPE_INVALID (i.e. MTRR disable), and the
callers handle this value properly. These cases are only problematic when
the OS tries to modify MTRR.
Thanks,
-Toshi
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-07 22:23 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1438984574.3109.151.camel@hp.com>
On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
>> On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
>> > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > > > On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
>> > > > > > > > On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
>> > > > > > There are two usages on MTRRs:
>> > > > > > 1) MTRR entries set by firmware
>> > > > > > 2) MTRR entries set by OS drivers
>> > > > > >
>> > > > > > We can obsolete 2), but we have no control over 1). As UEFI
>> > > > > > firmwares also set this up, this usage will continue to stay.
>> > > > > > So, we should not get rid of the MTRR code that looks up the
>> > > > > > MTRR entries, while we have no need to modify them.
>> > > > > >
>> > > > > > Such MTRR entries provide safe guard to /dev/mem, which allows
>> > > > > > privileged user to access a range that may require UC mapping
>> > > > > > while the /dev/mem driver blindly maps it with WB. MTRRs
>> > > > > > converts WB to UC in such a case.
>> > > > >
>> > > > > But it wouldn't be impossible to simply read the MTRRs upon boot,
>> > > > > store the information, disable MTRRs, and correctly use PAT to
>> > > > > achieve the same effect (i.e. the "blindly maps" part of course
>> > > > > would need fixing).
>> > > >
>> > > > It could be done, but I do not see much benefit of doing it. One of
>> > > > the reasons platform vendors set MTRRs is so that a system won't hit
>> > > > a machine check when an OS bug leads an access with a wrong cache
>> > > > type.
>> > > >
>> > > > A machine check is hard to analyze and can be seen as a hardware
>> > > > issue by customers. Emulating MTRRs with PAT won't protect from
>> > > > such a bug.
>> > >
>> > > That's seems like a fair and valid concern. This could only happen if
>> > > the OS would have code that would use MTRR, in the case of Linux we'll
>> > > soon be able to vet that this cannot happen.
>> >
>> > No, there is no OS support necessary to use MTRR. After firmware sets
>> > it up, CPUs continue to use it without any OS support. I think the
>> > Linux change you are referring is to obsolete legacy interfaces that
>> > modify the MTRR setup. I agree that Linux should not modify MTRR.
>>
>> Its a bit more than that though. Since you agree that the OS can live
>> without MTRR code I was hoping to then see if we can fold out PAT
>> Linux code from under the MTRR dependency on Linux and make PAT a
>> first class citizen, maybe at least for x86-64. Right now you can only
>> get PAT support on Linux if you have MTRR code, but I'd like to see if
>> instead we can rip MTRR code out completely under its own Kconfig and
>> let it start rotting away.
>>
>> Code-wise the only issue I saw was that PAT code also relies on
>> mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
>> no other obvious issues.
>
> We can rip of the MTTR code that modifies the MTRR setup, but not
> mtrr_type_lookup(). This function provides necessary checks per documented
> in commit 7f0431e3dc89 as follows.
>
> 1) reserve_memtype() tracks an effective memory type in case
> a request type is WB (ex. /dev/mem blindly uses WB). Missing
> to track with its effective type causes a subsequent request
> to map the same range with the effective type to fail.
>
> 2) pud_set_huge() and pmd_set_huge() check if a requested range
> has any overlap with MTRRs. Missing to detect an overlap may
> cause a performance penalty or undefined behavior.
>
> mtrr_type_lookup() is still admittedly awkward, but I do not think we have
> an immediate issue in PAT code calling it. I do not think it makes PAT code
> a second class citizen.
OK since we know that if MTRR set up code ends up disabled and would
return MTRR_TYPE_INVALID what if we just static inline this for the
no-MTRR Kconfig build option immediately, and only then have the full
blown implementation for the case where MTRR Kconfig option is
enabled?
>> Platform firmware and SMIs seems to be the only other possible issue.
>> More on this below.
>>
>> > > For those type of OSes...
>> > > could it be possible to negotiate or hint to the platform through an
>> > > attribute somehow that the OS has such capability to not use MTRR?
>> >
>> > The OS can disable MTRR. However, this can also cause a problem in
>> > firmware, which may rely on MTRR.
>>
>> Can you describe what type of issues we could expect ? I tend to care
>> more about this for 64-bit systems so if 32-bit platforms would be
>> more of the ones which could cause an issue would restricting
>> disabling MTRR only for 64-bit help?
>
> The SMI handler runs in real-mode and relies on MTRR being effective to
> provide right cache types. It does not matter if it is 64-bit or not.
I see... since I have no visibility to what goes under the hood, can
you provide one example use case where an SMI handler would require
getting a cache type through MTRR ? I realize this can vary, vendor by
vendor, but any example would do just to satisfy my curiosity.
>> > > Then, only if this bit is set, the platform could then avoid such MTRR
>> > > settings, and if we have issues you can throw rocks at us.
>> >
>> > > And if that's not possible how about a new platform setting that would
>> > > need to be set at the platform level to enable disabling this junk?
>> > > Then only folks who know what they are doing would enable it, and if
>> > > the customer set it, the issue would not be on the platform.
>> >
>> > > Could this also be used to prevent SMIs with MTRRs?
>> >
>> > ACPI _OSI could be used for firmware to implement some OS-specific
>> > features, but it may be too late for firmware to make major changes and
>> > is generally useless unless OS requirements are described in a spec
>> > backed by logo certification.
>>
>> I see.. So there are no guarantees that platform firmware would not
>> expect OS MTRR support.
>>
>> > SMIs are also used for platform management, such as fan
>> > speed control.
>>
>> And its conceivable that some devices, or the platform itself, may
>> trigger SMIs to have the platform firmware poke with MTRRs?
>
> SMIs are outside of OS control. SMI handler relies on MTRR being set. SMI
> must be quick, so the handler should not be required to initialize MTRR or
> page tables.
Right makes sense.
>> > Is there any issue for Linux to use MTRR set by firmware?
>>
>> Even though we don't have the Kconfig option right now to disable MTRR
>> cod explicitly I'll note that there are a few other cases that could
>> flip Linux to note use MTRR:
>>
>> a) Some BIOSes could let MTRR get disabled
>> b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
>> disables MTRR on Linux
>>
>> If these environments can exist it'd be good to understand possible
>> issues that could creep up as a result of the OS not having MTRR
>> enabled. If this is a reasonable thing for x86-64 I was hoping we
>> could just let users opt-in to a similar build configuration through
>> the OS by letting PAT not depend on MTRR.
>
> Case a) and b) do not cause any issue. They simply lead mtrr_type_lookup()
> to return immediately with MTRR_TYPE_INVALID (i.e. MTRR disable), and the
> callers handle this value properly. These cases are only problematic when
> the OS tries to modify MTRR.
OK if the OS returns MTRR_TYPE_INVALID, for folks who do not want MTRR
code on their kernel, we should be OK?
Luis
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-08-07 23:08 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <CAB=NE6V+dAq1u52c3tDhBOYWU1BNMBVL3KzDdM=C-zTkLEx4xA@mail.gmail.com>
On Fri, 2015-08-07 at 15:23 -0700, Luis R. Rodriguez wrote:
> On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
> > > On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > > > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
> > > > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com>
> > > > > wrote:
:
> > > >
> > > > No, there is no OS support necessary to use MTRR. After firmware
> > > > sets it up, CPUs continue to use it without any OS support. I think
> > > > the Linux change you are referring is to obsolete legacy interfaces
> > > > that modify the MTRR setup. I agree that Linux should not modify
> > > > MTRR.
> > >
> > > Its a bit more than that though. Since you agree that the OS can live
> > > without MTRR code I was hoping to then see if we can fold out PAT
> > > Linux code from under the MTRR dependency on Linux and make PAT a
> > > first class citizen, maybe at least for x86-64. Right now you can only
> > > get PAT support on Linux if you have MTRR code, but I'd like to see if
> > > instead we can rip MTRR code out completely under its own Kconfig and
> > > let it start rotting away.
> > >
> > > Code-wise the only issue I saw was that PAT code also relies on
> > > mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
> > > no other obvious issues.
> >
> > We can rip of the MTTR code that modifies the MTRR setup, but not
> > mtrr_type_lookup(). This function provides necessary checks per
> > documented
> > in commit 7f0431e3dc89 as follows.
> >
> > 1) reserve_memtype() tracks an effective memory type in case
> > a request type is WB (ex. /dev/mem blindly uses WB). Missing
> > to track with its effective type causes a subsequent request
> > to map the same range with the effective type to fail.
> >
> > 2) pud_set_huge() and pmd_set_huge() check if a requested range
> > has any overlap with MTRRs. Missing to detect an overlap may
> > cause a performance penalty or undefined behavior.
> >
> > mtrr_type_lookup() is still admittedly awkward, but I do not think we
> > have an immediate issue in PAT code calling it. I do not think it makes
> > PAT code a second class citizen.
>
> OK since we know that if MTRR set up code ends up disabled and would
> return MTRR_TYPE_INVALID what if we just static inline this for the
> no-MTRR Kconfig build option immediately, and only then have the full
> blown implementation for the case where MTRR Kconfig option is
> enabled?
Yes, the MTRR code could be disabled by Kconfig with such inline stubs as
long as the kernel is built specifically for a particular platform with MTRR
disabled, such as Xen guest kernel.
However, since MTRR is a CPU feature enabled on most of the systems, I am
not sure if it makes sense to be configurable with Kconfig, though.
> > > Platform firmware and SMIs seems to be the only other possible issue.
> > > More on this below.
> > >
> > > > > For those type of OSes...
> > > > > could it be possible to negotiate or hint to the platform through
> > > > > an attribute somehow that the OS has such capability to not use
> > > > > MTRR?
> > > >
> > > > The OS can disable MTRR. However, this can also cause a problem in
> > > > firmware, which may rely on MTRR.
> > >
> > > Can you describe what type of issues we could expect ? I tend to care
> > > more about this for 64-bit systems so if 32-bit platforms would be
> > > more of the ones which could cause an issue would restricting
> > > disabling MTRR only for 64-bit help?
> >
> > The SMI handler runs in real-mode and relies on MTRR being effective to
> > provide right cache types. It does not matter if it is 64-bit or not.
>
> I see... since I have no visibility to what goes under the hood, can
> you provide one example use case where an SMI handler would require
> getting a cache type through MTRR ? I realize this can vary, vendor by
> vendor, but any example would do just to satisfy my curiosity.
For fan control, it would need UC access to its registers.
> > > > > Then, only if this bit is set, the platform could then avoid such
> > > > > MTRR settings, and if we have issues you can throw rocks at us.
> > > >
> > > > > And if that's not possible how about a new platform setting that
> > > > > would need to be set at the platform level to enable disabling
> > > > > this junk?
> > > > > Then only folks who know what they are doing would enable it, and
> > > > > if the customer set it, the issue would not be on the platform.
> > > >
> > > > > Could this also be used to prevent SMIs with MTRRs?
> > > >
> > > > ACPI _OSI could be used for firmware to implement some OS-specific
> > > > features, but it may be too late for firmware to make major changes
> > > > and
> > > > is generally useless unless OS requirements are described in a spec
> > > > backed by logo certification.
> > >
> > > I see.. So there are no guarantees that platform firmware would not
> > > expect OS MTRR support.
> > >
> > > > SMIs are also used for platform management, such as fan
> > > > speed control.
> > >
> > > And its conceivable that some devices, or the platform itself, may
> > > trigger SMIs to have the platform firmware poke with MTRRs?
> >
> > SMIs are outside of OS control. SMI handler relies on MTRR being set.
> > SMI must be quick, so the handler should not be required to initialize
> > MTRR or page tables.
>
> Right makes sense.
>
> > > > Is there any issue for Linux to use MTRR set by firmware?
> > >
> > > Even though we don't have the Kconfig option right now to disable MTRR
> > > cod explicitly I'll note that there are a few other cases that could
> > > flip Linux to note use MTRR:
> > >
> > > a) Some BIOSes could let MTRR get disabled
> > > b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
> > > disables MTRR on Linux
> > >
> > > If these environments can exist it'd be good to understand possible
> > > issues that could creep up as a result of the OS not having MTRR
> > > enabled. If this is a reasonable thing for x86-64 I was hoping we
> > > could just let users opt-in to a similar build configuration through
> > > the OS by letting PAT not depend on MTRR.
> >
> > Case a) and b) do not cause any issue. They simply lead
> > mtrr_type_lookup() to return immediately with MTRR_TYPE_INVALID (i.e.
> > MTRR disable), and the callers handle this value properly. These cases
> > are only problematic when the OS tries to modify MTRR.
>
> OK if the OS returns MTRR_TYPE_INVALID, for folks who do not want MTRR
> code on their kernel, we should be OK?
Technically OK. Not sure if we want such a Kconfig option, though.
Thanks,
-Toshi
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-08-07 23:19 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1438988915.3109.175.camel@hp.com>
On Fri, 2015-08-07 at 17:08 -0600, Toshi Kani wrote:
> On Fri, 2015-08-07 at 15:23 -0700, Luis R. Rodriguez wrote:
> > On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > > On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
> > > > On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com>
> > > > wrote:
> > > > > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
> > > > > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com>
> > > > > > wrote:
> :
> > > > >
> > > > > No, there is no OS support necessary to use MTRR. After firmware
> > > > > sets it up, CPUs continue to use it without any OS support. I
> > > > > think the Linux change you are referring is to obsolete legacy
> > > > > interfaces that modify the MTRR setup. I agree that Linux should
> > > > > not modify MTRR.
> > > >
> > > > Its a bit more than that though. Since you agree that the OS can
> > > > live without MTRR code I was hoping to then see if we can fold out
> > > > PAT Linux code from under the MTRR dependency on Linux and make PAT
> > > > a first class citizen, maybe at least for x86-64. Right now you can
> > > > only get PAT support on Linux if you have MTRR code, but I'd like to
> > > > see if instead we can rip MTRR code out completely under its own
> > > > Kconfig and let it start rotting away.
> > > >
> > > > Code-wise the only issue I saw was that PAT code also relies on
> > > > mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I
> > > > found no other obvious issues.
> > >
> > > We can rip of the MTTR code that modifies the MTRR setup, but not
> > > mtrr_type_lookup(). This function provides necessary checks per
> > > documented in commit 7f0431e3dc89 as follows.
> > >
> > > 1) reserve_memtype() tracks an effective memory type in case
> > > a request type is WB (ex. /dev/mem blindly uses WB). Missing
> > > to track with its effective type causes a subsequent request
> > > to map the same range with the effective type to fail.
> > >
> > > 2) pud_set_huge() and pmd_set_huge() check if a requested range
> > > has any overlap with MTRRs. Missing to detect an overlap may
> > > cause a performance penalty or undefined behavior.
> > >
> > > mtrr_type_lookup() is still admittedly awkward, but I do not think we
> > > have an immediate issue in PAT code calling it. I do not think it
> > > makes
> > > PAT code a second class citizen.
> >
> > OK since we know that if MTRR set up code ends up disabled and would
> > return MTRR_TYPE_INVALID what if we just static inline this for the
> > no-MTRR Kconfig build option immediately, and only then have the full
> > blown implementation for the case where MTRR Kconfig option is
> > enabled?
>
> Yes, the MTRR code could be disabled by Kconfig with such inline stubs as
> long as the kernel is built specifically for a particular platform with
> MTRR disabled, such as Xen guest kernel.
Noticed that we do have CONFIG_MTRR and mtrr_type_lookup() inline stub
returns MTRR_INVALID.
-Toshi
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-07 23:26 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1438988915.3109.175.camel@hp.com>
On Fri, Aug 7, 2015 at 4:08 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Fri, 2015-08-07 at 15:23 -0700, Luis R. Rodriguez wrote:
>> On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
>> > > On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > > > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
>> > > > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com>
>> > > > > wrote:
> :
>> > > >
>> > > > No, there is no OS support necessary to use MTRR. After firmware
>> > > > sets it up, CPUs continue to use it without any OS support. I think
>> > > > the Linux change you are referring is to obsolete legacy interfaces
>> > > > that modify the MTRR setup. I agree that Linux should not modify
>> > > > MTRR.
>> > >
>> > > Its a bit more than that though. Since you agree that the OS can live
>> > > without MTRR code I was hoping to then see if we can fold out PAT
>> > > Linux code from under the MTRR dependency on Linux and make PAT a
>> > > first class citizen, maybe at least for x86-64. Right now you can only
>> > > get PAT support on Linux if you have MTRR code, but I'd like to see if
>> > > instead we can rip MTRR code out completely under its own Kconfig and
>> > > let it start rotting away.
>> > >
>> > > Code-wise the only issue I saw was that PAT code also relies on
>> > > mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
>> > > no other obvious issues.
>> >
>> > We can rip of the MTTR code that modifies the MTRR setup, but not
>> > mtrr_type_lookup(). This function provides necessary checks per
>> > documented
>> > in commit 7f0431e3dc89 as follows.
>> >
>> > 1) reserve_memtype() tracks an effective memory type in case
>> > a request type is WB (ex. /dev/mem blindly uses WB). Missing
>> > to track with its effective type causes a subsequent request
>> > to map the same range with the effective type to fail.
>> >
>> > 2) pud_set_huge() and pmd_set_huge() check if a requested range
>> > has any overlap with MTRRs. Missing to detect an overlap may
>> > cause a performance penalty or undefined behavior.
>> >
>> > mtrr_type_lookup() is still admittedly awkward, but I do not think we
>> > have an immediate issue in PAT code calling it. I do not think it makes
>> > PAT code a second class citizen.
>>
>> OK since we know that if MTRR set up code ends up disabled and would
>> return MTRR_TYPE_INVALID what if we just static inline this for the
>> no-MTRR Kconfig build option immediately, and only then have the full
>> blown implementation for the case where MTRR Kconfig option is
>> enabled?
>
> Yes, the MTRR code could be disabled by Kconfig with such inline stubs
OK thanks.
> as
> long as the kernel is built specifically for a particular platform with MTRR
> disabled, such as Xen guest kernel.
Sure.
> However, since MTRR is a CPU feature enabled on most of the systems, I am
> not sure if it makes sense to be configurable with Kconfig, though.
To me this is about making PAT a first class citizen in code though
and validating through Kconfig the option then to opt-out of MTRR from
OS code. Perhaps we can recommend to enable it but having the options
to split out PAT from MTRR is what I was aiming for.
>> > > Platform firmware and SMIs seems to be the only other possible issue.
>> > > More on this below.
>> > >
>> > > > > For those type of OSes...
>> > > > > could it be possible to negotiate or hint to the platform through
>> > > > > an attribute somehow that the OS has such capability to not use
>> > > > > MTRR?
>> > > >
>> > > > The OS can disable MTRR. However, this can also cause a problem in
>> > > > firmware, which may rely on MTRR.
>> > >
>> > > Can you describe what type of issues we could expect ? I tend to care
>> > > more about this for 64-bit systems so if 32-bit platforms would be
>> > > more of the ones which could cause an issue would restricting
>> > > disabling MTRR only for 64-bit help?
>> >
>> > The SMI handler runs in real-mode and relies on MTRR being effective to
>> > provide right cache types. It does not matter if it is 64-bit or not.
>>
>> I see... since I have no visibility to what goes under the hood, can
>> you provide one example use case where an SMI handler would require
>> getting a cache type through MTRR ? I realize this can vary, vendor by
>> vendor, but any example would do just to satisfy my curiosity.
>
> For fan control, it would need UC access to its registers.
OK thanks! To follow up with the example, since the platform firmware
would have set up the MTRRs anyway, the SMI should still work, even if
the OS didn't do anything, right?
>> > > > > Then, only if this bit is set, the platform could then avoid such
>> > > > > MTRR settings, and if we have issues you can throw rocks at us.
>> > > >
>> > > > > And if that's not possible how about a new platform setting that
>> > > > > would need to be set at the platform level to enable disabling
>> > > > > this junk?
>> > > > > Then only folks who know what they are doing would enable it, and
>> > > > > if the customer set it, the issue would not be on the platform.
>> > > >
>> > > > > Could this also be used to prevent SMIs with MTRRs?
>> > > >
>> > > > ACPI _OSI could be used for firmware to implement some OS-specific
>> > > > features, but it may be too late for firmware to make major changes
>> > > > and
>> > > > is generally useless unless OS requirements are described in a spec
>> > > > backed by logo certification.
>> > >
>> > > I see.. So there are no guarantees that platform firmware would not
>> > > expect OS MTRR support.
>> > >
>> > > > SMIs are also used for platform management, such as fan
>> > > > speed control.
>> > >
>> > > And its conceivable that some devices, or the platform itself, may
>> > > trigger SMIs to have the platform firmware poke with MTRRs?
>> >
>> > SMIs are outside of OS control. SMI handler relies on MTRR being set.
>> > SMI must be quick, so the handler should not be required to initialize
>> > MTRR or page tables.
>>
>> Right makes sense.
>>
>> > > > Is there any issue for Linux to use MTRR set by firmware?
>> > >
>> > > Even though we don't have the Kconfig option right now to disable MTRR
>> > > cod explicitly I'll note that there are a few other cases that could
>> > > flip Linux to note use MTRR:
>> > >
>> > > a) Some BIOSes could let MTRR get disabled
>> > > b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
>> > > disables MTRR on Linux
>> > >
>> > > If these environments can exist it'd be good to understand possible
>> > > issues that could creep up as a result of the OS not having MTRR
>> > > enabled. If this is a reasonable thing for x86-64 I was hoping we
>> > > could just let users opt-in to a similar build configuration through
>> > > the OS by letting PAT not depend on MTRR.
>> >
>> > Case a) and b) do not cause any issue. They simply lead
>> > mtrr_type_lookup() to return immediately with MTRR_TYPE_INVALID (i.e.
>> > MTRR disable), and the callers handle this value properly. These cases
>> > are only problematic when the OS tries to modify MTRR.
>>
>> OK if the OS returns MTRR_TYPE_INVALID, for folks who do not want MTRR
>> code on their kernel, we should be OK?
>
> Technically OK. Not sure if we want such a Kconfig option, though.
Its more of me wanting to get PAT out from under MTRR. Does that make sense?
Luis
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-08-07 23:48 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <CAB=NE6VmpBchJQ=Bfi1qbc+_aSMzcBe1xCqVvJpgbDw8gdrwwQ@mail.gmail.com>
On Fri, 2015-08-07 at 16:26 -0700, Luis R. Rodriguez wrote:
> On Fri, Aug 7, 2015 at 4:08 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Fri, 2015-08-07 at 15:23 -0700, Luis R. Rodriguez wrote:
> > > On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > > > On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote:
> > > > > On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com>
> > > > > wrote:
> > > > > > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
> > > > > > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com
> > > > > > > >
> > > > > > > wrote:
:
> > > > >
> > > > > Its a bit more than that though. Since you agree that the OS can
> > > > > live without MTRR code I was hoping to then see if we can fold out
> > > > > PAT Linux code from under the MTRR dependency on Linux and make
> > > > > PAT a first class citizen, maybe at least for x86-64. Right now
> > > > > you can only get PAT support on Linux if you have MTRR code, but
> > > > > I'd like to see if instead we can rip MTRR code out completely
> > > > > under its own Kconfig and let it start rotting away.
> > > > >
> > > > > Code-wise the only issue I saw was that PAT code also relies on
> > > > > mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I
> > > > > found no other obvious issues.
> > > >
> > > > We can rip of the MTTR code that modifies the MTRR setup, but not
> > > > mtrr_type_lookup(). This function provides necessary checks per
> > > > documented
> > > > in commit 7f0431e3dc89 as follows.
> > > >
> > > > 1) reserve_memtype() tracks an effective memory type in case
> > > > a request type is WB (ex. /dev/mem blindly uses WB). Missing
> > > > to track with its effective type causes a subsequent request
> > > > to map the same range with the effective type to fail.
> > > >
> > > > 2) pud_set_huge() and pmd_set_huge() check if a requested range
> > > > has any overlap with MTRRs. Missing to detect an overlap may
> > > > cause a performance penalty or undefined behavior.
> > > >
> > > > mtrr_type_lookup() is still admittedly awkward, but I do not think
> > > > we
> > > > have an immediate issue in PAT code calling it. I do not think it
> > > > makes
> > > > PAT code a second class citizen.
> > >
> > > OK since we know that if MTRR set up code ends up disabled and would
> > > return MTRR_TYPE_INVALID what if we just static inline this for the
> > > no-MTRR Kconfig build option immediately, and only then have the full
> > > blown implementation for the case where MTRR Kconfig option is
> > > enabled?
> >
> > Yes, the MTRR code could be disabled by Kconfig with such inline stubs
>
> OK thanks.
>
> > as
> > long as the kernel is built specifically for a particular platform with
> > MTRR disabled, such as Xen guest kernel.
>
> Sure.
>
> > However, since MTRR is a CPU feature enabled on most of the systems, I
> > am not sure if it makes sense to be configurable with Kconfig, though.
>
> To me this is about making PAT a first class citizen in code though
> and validating through Kconfig the option then to opt-out of MTRR from
> OS code. Perhaps we can recommend to enable it but having the options
> to split out PAT from MTRR is what I was aiming for.
Since we have CONFIG_MTRR already, we do not need to argue over this option.
:-) It makes sense since when MTRR code was introduced, there were CPUs
without this capability...
> > > > > Platform firmware and SMIs seems to be the only other possible
> > > > > issue. More on this below.
> > > > >
> > > > > > > For those type of OSes...
> > > > > > > could it be possible to negotiate or hint to the platform
> > > > > > > through an attribute somehow that the OS has such capability
> > > > > > > to not use MTRR?
> > > > > >
> > > > > > The OS can disable MTRR. However, this can also cause a problem
> > > > > > in firmware, which may rely on MTRR.
> > > > >
> > > > > Can you describe what type of issues we could expect ? I tend to
> > > > > care more about this for 64-bit systems so if 32-bit platforms
> > > > > would be more of the ones which could cause an issue would
> > > > > restricting disabling MTRR only for 64-bit help?
> > > >
> > > > The SMI handler runs in real-mode and relies on MTRR being effective
> > > > to provide right cache types. It does not matter if it is 64-bit or
> > > > not.
> > >
> > > I see... since I have no visibility to what goes under the hood, can
> > > you provide one example use case where an SMI handler would require
> > > getting a cache type through MTRR ? I realize this can vary, vendor by
> > > vendor, but any example would do just to satisfy my curiosity.
> >
> > For fan control, it would need UC access to its registers.
>
> OK thanks! To follow up with the example, since the platform firmware
> would have set up the MTRRs anyway, the SMI should still work, even if
> the OS didn't do anything, right?
Yes, MTRR works without the OS code. However, mtrr_type_lookup() is
necessary to make sure that OS mapping requests are aligned with with the
MTRR setup.
> > > > > > Is there any issue for Linux to use MTRR set by firmware?
> > > > >
> > > > > Even though we don't have the Kconfig option right now to disable
> > > > > MTRR cod explicitly I'll note that there are a few other cases
> > > > > that could flip Linux to note use MTRR:
> > > > >
> > > > > a) Some BIOSes could let MTRR get disabled
> > > > > b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
> > > > > disables MTRR on Linux
> > > > >
> > > > > If these environments can exist it'd be good to understand
> > > > > possible issues that could creep up as a result of the OS not
> > > > > having MTRR enabled. If this is a reasonable thing for x86-64 I
> > > > > was hoping we could just let users opt-in to a similar build
> > > > > configuration through the OS by letting PAT not depend on MTRR.
> > > >
> > > > Case a) and b) do not cause any issue. They simply lead
> > > > mtrr_type_lookup() to return immediately with MTRR_TYPE_INVALID
> > > > (i.e. MTRR disable), and the callers handle this value properly.
> > > > These cases are only problematic when the OS tries to modify MTRR.
> > >
> > > OK if the OS returns MTRR_TYPE_INVALID, for folks who do not want MTRR
> > > code on their kernel, we should be OK?
> >
> > Technically OK. Not sure if we want such a Kconfig option, though.
>
> Its more of me wanting to get PAT out from under MTRR. Does that make
> sense?
It makes sense if you need to make the kernel size a bit smaller, and you
build kernels specific to Xen guests. Leaving the MTRR code enabled on Xen
guests does not cause you any issue, though.
Thanks,
-Toshi
^ permalink raw reply
* Re: [BUG, RFC] Different bitfields in documentation and driver
From: Robert Jarzmik @ 2015-08-08 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55C503A7.6020304@tul.cz>
Petr Cvek <petr.cvek@tul.cz> writes:
> Hello,
> It seems that PXA2xx framebuffer has switched bitfields in nonstd definition (when using overlay).
>
> This:
>
> http://lxr.free-electrons.com/source/drivers/video/fbdev/pxafb.h#L80
>
> versus this:
>
> http://lxr.free-electrons.com/source/Documentation/fb/pxafb.txt#L128
>
> Should be this changed in documentation or in driver? I was able to find only
> one use (in form of little outdated mplayer patch) and it fills both bitfields
> with zero (so potential change in driver will not break it).
I'd rather go for documentation. Not only the driver's way looks more pretty to
me, but it also guarantees compatibility from the kernel side.
> P.S. Lack of overlay2's use is probably caused by errata "E24: LCD: Enabling
> Overlay 2 for YUV420 hangs LCD controller.", which is not workarounded in this
> driver. It should be mentioned in documentation and/or code, because the only
> way to start the LCD again is a full PXA SoC reset.
Ok, do you know if this this true for all pxa variants and steppings or is this
for only a subset of them ?
Cheers.
--
Robert
^ permalink raw reply
* Re: [BUG, RFC] Different bitfields in documentation and driver
From: Petr Cvek @ 2015-08-08 17:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87zj21k9bx.fsf@belgarion.home>
On 8.8.2015 18:56, Robert Jarzmik wrote:
> Petr Cvek <petr.cvek@tul.cz> writes:
>
>> Hello,
>> It seems that PXA2xx framebuffer has switched bitfields in nonstd definition (when using overlay).
>>
>> This:
>>
>> http://lxr.free-electrons.com/source/drivers/video/fbdev/pxafb.h#L80
>>
>> versus this:
>>
>> http://lxr.free-electrons.com/source/Documentation/fb/pxafb.txt#L128
>>
>> Should be this changed in documentation or in driver? I was able to find only
>> one use (in form of little outdated mplayer patch) and it fills both bitfields
>> with zero (so potential change in driver will not break it).
> I'd rather go for documentation. Not only the driver's way looks more pretty to
> me, but it also guarantees compatibility from the kernel side.
Oh you are right, I forgot to look into SoC datasheet and related register has YX version
(driver side). Will send patch with errata warning.
>
>> P.S. Lack of overlay2's use is probably caused by errata "E24: LCD: Enabling
>> Overlay 2 for YUV420 hangs LCD controller.", which is not workarounded in this
>> driver. It should be mentioned in documentation and/or code, because the only
>> way to start the LCD again is a full PXA SoC reset.
> Ok, do you know if this this true for all pxa variants and steppings or is this
> for only a subset of them ?
According to this:
http://www.marvell.com/application-processors/pxa-family/assets/pxa_27x_spec_update.pdf
errata (page 11, errata 24), bug is only for C0 revision (newer are OK, but I would guess most of devices
will have C0). There is some workaround pseudocode, but it seems it does not work with me (only
few tries to implement it, but penalty is reboot ... workaround description is a little bit confusing)
P.S. LCD controller has other errors too (notably E15 (not fixed) and E16).
Petr
>
> Cheers.
>
^ permalink raw reply
* [PATCH v1, TRIVIAL] Fix pxafb nonstd field documentation and add errata info
From: Petr Cvek @ 2015-08-08 18:20 UTC (permalink / raw)
To: linux-arm-kernel
Fix pxafb nonstd field documentation and add warning comment to Kconfig
and pxafb.c (errata: YUV420 hangs LCD).
Signed-off-by: Petr Cvek <petr.cvek@tul.cz>
---
Documentation/fb/pxafb.txt | 8 ++++----
drivers/video/fbdev/Kconfig | 5 +++++
drivers/video/fbdev/pxafb.c | 4 ++++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/fb/pxafb.txt b/Documentation/fb/pxafb.txt
index d143a0a..dbbf5eb 100644
--- a/Documentation/fb/pxafb.txt
+++ b/Documentation/fb/pxafb.txt
@@ -123,12 +123,12 @@ Overlay Support for PXA27x and later LCD controllers
framebuffer framework, application has to take care of the offsets
and lengths of each component within the framebuffer.
- 4. var->nonstd is used to pass starting (x, y) position and color format,
+ 4. var->nonstd is used to pass starting (y, x) position and color format,
the detailed bit fields are shown below:
31 23 20 10 0
+-----------------+---+----------+----------+
- | ... unused ... |FOR| XPOS | YPOS |
+ | ... unused ... |FOR| YPOS | XPOS | (as in OVLxC2 register)
+-----------------+---+----------+----------+
FOR - color format, as defined by OVERLAY_FORMAT_* in pxafb.h
@@ -136,7 +136,7 @@ Overlay Support for PXA27x and later LCD controllers
1 - YUV444 PACKED
2 - YUV444 PLANAR
3 - YUV422 PLANAR
- 4 - YUR420 PLANAR
+ 4 - YUV420 PLANAR
- XPOS - starting horizontal position
YPOS - starting vertical position
+ XPOS - starting horizontal position
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 2d98de5..8f485ec 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1893,6 +1893,11 @@ config FB_PXA_OVERLAY
bool "Support PXA27x/PXA3xx Overlay(s) as framebuffer"
default n
depends on FB_PXA && (PXA27x || PXA3xx)
+ ---help---
+ Using the overlay 2 and YUV420 mode on a PXA27x C0 chip revision will
+ hang the LCD until the next SoC restart (errata E24).
+
+ If unsure, say N or check /proc/cpuinfo for "CPU revision : 4".
config FB_PXA_SMARTPANEL
bool "PXA Smartpanel LCD support"
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 7245611..e573699 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -30,6 +30,10 @@
*
* Copyright (C) 2006-2008 Marvell International Ltd.
* All Rights Reserved
+ *
+ * NOTICE
+ * According to PXA27x errata E24 an enable of the overlay 2 in YUV420 mode
+ * on C0 chip revision will hang the LCD until the next SoC reset.
*/
#include <linux/module.h>
--
1.7.12.1
^ permalink raw reply related
* Re: [PATCH v1, TRIVIAL] Fix pxafb nonstd field documentation and add errata info
From: Robert Jarzmik @ 2015-08-08 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55C64864.6070405@tul.cz>
Petr Cvek <petr.cvek@tul.cz> writes:
> Fix pxafb nonstd field documentation and add warning comment to Kconfig
> and pxafb.c (errata: YUV420 hangs LCD).
>
> Signed-off-by: Petr Cvek <petr.cvek@tul.cz>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Cheers.
--
Robert
^ permalink raw reply
* Re: [PATCH] fbdev: select versatile helpers for the integrator
From: Linus Walleij @ 2015-08-10 9:03 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1438090272-26266-1-git-send-email-linus.walleij@linaro.org>
On Tue, Jul 28, 2015 at 3:31 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> Commit 11c32d7b6274cb0f554943d65bd4a126c4a86dcd
> "video: move Versatile CLCD helpers" missed the fact
> that the Integrator/CP is also using the helper, and
> as a result the platform got only stubs and no graphics.
> Add this as a default selection to Kconfig so we have
> graphics again.
>
> Fixes: 11c32d7b6274 (video: move Versatile CLCD helpers)
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Ping on this.
Shall I take this into ARM SoC instead?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] fbdev: select versatile helpers for the integrator
From: Tomi Valkeinen @ 2015-08-10 9:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1438090272-26266-1-git-send-email-linus.walleij@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
On 10/08/15 12:03, Linus Walleij wrote:
> On Tue, Jul 28, 2015 at 3:31 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> Commit 11c32d7b6274cb0f554943d65bd4a126c4a86dcd
>> "video: move Versatile CLCD helpers" missed the fact
>> that the Integrator/CP is also using the helper, and
>> as a result the platform got only stubs and no graphics.
>> Add this as a default selection to Kconfig so we have
>> graphics again.
>>
>> Fixes: 11c32d7b6274 (video: move Versatile CLCD helpers)
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Ping on this.
>
> Shall I take this into ARM SoC instead?
Sorry, I still haven't gotten into dealing with fbdev related mails
after my vacation...
This is needed for 4.2, right? I can pick this up.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/2] OMAPDSS: Fix of-node refcount issues
From: Tomi Valkeinen @ 2015-08-10 9:23 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <cover.1438945255.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 916 bytes --]
On 07/08/15 14:04, Jyri Sarha wrote:
> Changes since the first version:
> - Fix commit descriptions and subject according to Tomi's comments
>
> I found couple of refcounting issues related to OMAP DSS of-node
> handling. Second patch should fix the "ERROR: Bad of_node_put() on
> /encoder@0/ports/port@1" -problem.
>
> In the long run it would make sense start using of_graph_*() functions
> in OMAP DSS too. However the semantics of of_graph_*() functions are
> quite different from omapdss_of_*() counterparts. So it better to just
> fix these bugs first.
>
> Jyri Sarha (2):
> OMAPDSS: Fix node refcount leak in omapdss_of_get_next_port()
> OMAPDSS: Fix omap_dss_find_output_by_port_node() port refcount
> decrement
>
> drivers/video/fbdev/omap2/dss/dss-of.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Thanks, I've picked these up for 4.2 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: pxa3xx_gcu: prepare the clocks
From: Tomi Valkeinen @ 2015-08-10 9:26 UTC (permalink / raw)
To: Robert Jarzmik, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <1438632934-29506-1-git-send-email-robert.jarzmik@free.fr>
[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]
On 03/08/15 23:15, Robert Jarzmik wrote:
> The clocks need to be prepared before being enabled. Without it a
> warning appears in the drivers probe path :
>
> WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:707 clk_core_enable+0x84/0xa0()
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc3-cm-x300+ #804
> Hardware name: CM-X300 module
> [<c000ed50>] (unwind_backtrace) from [<c000ce08>] (show_stack+0x10/0x14)
> [<c000ce08>] (show_stack) from [<c0017eb4>] (warn_slowpath_common+0x7c/0xb4)
> [<c0017eb4>] (warn_slowpath_common) from [<c0017f88>] (warn_slowpath_null+0x1c/0x24)
> [<c0017f88>] (warn_slowpath_null) from [<c02d30dc>] (clk_core_enable+0x84/0xa0)
> [<c02d30dc>] (clk_core_enable) from [<c02d3118>] (clk_enable+0x20/0x34)
> [<c02d3118>] (clk_enable) from [<c0200dfc>] (pxa3xx_gcu_probe+0x148/0x338)
> [<c0200dfc>] (pxa3xx_gcu_probe) from
> [<c022eccc>] (platform_drv_probe+0x30/0x94)
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> drivers/video/fbdev/pxa3xx-gcu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks, I've queued this for 4.2 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM/fb: ep93xx: switch framebuffer to use modedb only
From: Tomi Valkeinen @ 2015-08-10 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1437744688-29915-1-git-send-email-linus.walleij@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]
On 24/07/15 16:31, Linus Walleij wrote:
> All the EP93xx boards exclusively use modedb to look up video
> modes from the command line. Root out the parametrization of
> custom video modes from the platform data and board files
> and simplify the driver.
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> J-C/Tomi: please ACK this if you're OK with it so I can take
> it through the ARM SoC tree.
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [patch] fbdev: off by one test (harmless)
From: Tomi Valkeinen @ 2015-08-10 9:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20141226172657.GA14762@mwanda>
[-- Attachment #1: Type: text/plain, Size: 112 bytes --]
On 23/07/15 15:39, Dan Carpenter wrote:
> This was never merged.
Oops. I'll pick this up now.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: Fix possible leak in of_get_videomode()
From: Tomi Valkeinen @ 2015-08-10 9:53 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]
On 11/07/15 20:46, Christian Engelmayer wrote:
> In case videomode_from_timings() fails in function of_get_videomode(), the
> allocated display timing data is not freed in the exit path. Make sure that
> display_timings_release() is called in any case. Detected by Coverity CID
> 1309681.
>
> Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
> ---
> Compile tested only. Applies against linux-next.
> ---
> drivers/video/of_videomode.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
> index 111c2d1911d3..3495b48c6805 100644
> --- a/drivers/video/of_videomode.c
> +++ b/drivers/video/of_videomode.c
> @@ -44,11 +44,7 @@ int of_get_videomode(struct device_node *np, struct videomode *vm,
> index = disp->native_mode;
>
> ret = videomode_from_timings(disp, vm, index);
> - if (ret)
> - return ret;
> -
> display_timings_release(disp);
> -
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(of_get_videomode);
>
Thanks, queued for 4.2.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: yalin wang @ 2015-08-10 10:12 UTC (permalink / raw)
To: adaplas, plagnioj, tomi.valkeinen, linux-fbdev, open list
This change to use swab32(bitrev32()) to implement reverse_order()
function, have better performance on some platforms.
Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
---
drivers/video/fbdev/riva/fbdev.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index f1ad274..4803901 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -40,6 +40,7 @@
#include <linux/init.h>
#include <linux/pci.h>
#include <linux/backlight.h>
+#include <linux/swab.h>
#include <linux/bitrev.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/machdep.h>
@@ -84,6 +85,7 @@
#define SetBit(n) (1<<(n))
#define Set8Bits(value) ((value)&0xff)
+#define reverse_order(v) swab32(bitrev32(v))
/* HW cursor parameters */
#define MAX_CURS 32
@@ -451,15 +453,6 @@ static inline unsigned char MISCin(struct riva_par *par)
return (VGA_RD08(par->riva.PVIO, 0x3cc));
}
-static inline void reverse_order(u32 *l)
-{
- u8 *a = (u8 *)l;
- a[0] = bitrev8(a[0]);
- a[1] = bitrev8(a[1]);
- a[2] = bitrev8(a[2]);
- a[3] = bitrev8(a[3]);
-}
-
/* ------------------------------------------------------------------------- *
*
* cursor stuff
@@ -497,8 +490,8 @@ static void rivafb_load_cursor_image(struct riva_par *par, u8 *data8,
for (i = 0; i < h; i++) {
b = *data++;
- reverse_order(&b);
-
+ b = reverse_order(b);
+
for (j = 0; j < w/2; j++) {
tmp = 0;
#if defined (__BIG_ENDIAN)
@@ -1545,7 +1538,7 @@ static void rivafb_imageblit(struct fb_info *info,
for (i = 0; i < 16; i++) {
tmp = *((u32 *)cdat);
cdat = (u8 *)((u32 *)cdat + 1);
- reverse_order(&tmp);
+ tmp = reverse_order(tmp);
NV_WR32(d, i*4, tmp);
}
size -= 16;
@@ -1555,7 +1548,7 @@ static void rivafb_imageblit(struct fb_info *info,
for (i = 0; i < size; i++) {
tmp = *((u32 *) cdat);
cdat = (u8 *)((u32 *)cdat + 1);
- reverse_order(&tmp);
+ tmp = reverse_order(tmp);
NV_WR32(d, i*4, tmp);
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] drm: Remove two-level menu in Kconfig
From: Tomi Valkeinen @ 2015-08-10 10:32 UTC (permalink / raw)
To: Thierry Reding, Jean-Christophe Plagniol-Villard; +Cc: linux-fbdev, dri-devel
In-Reply-To: <1438963817-9695-1-git-send-email-thierry.reding@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
On 07/08/15 19:10, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The Direct Rendering Manager Kconfig option is already a separate menu,
> so remove the extra level to make it easier to navigate.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/video/Kconfig | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 8bf495ffb020..e0606c01e8ac 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig"
> source "drivers/gpu/host1x/Kconfig"
> source "drivers/gpu/ipu-v3/Kconfig"
>
> -menu "Direct Rendering Manager"
> source "drivers/gpu/drm/Kconfig"
> -endmenu
>
> menu "Frame buffer Devices"
> source "drivers/video/fbdev/Kconfig"
>
Thanks, queued for 4.3.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] drm: Remove two-level menu in Kconfig
From: Daniel Vetter @ 2015-08-10 11:47 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, Jean-Christophe Plagniol-Villard, dri-devel
In-Reply-To: <55C87DA8.1010206@ti.com>
On Mon, Aug 10, 2015 at 01:32:08PM +0300, Tomi Valkeinen wrote:
>
>
> On 07/08/15 19:10, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The Direct Rendering Manager Kconfig option is already a separate menu,
> > so remove the extra level to make it easier to navigate.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > drivers/video/Kconfig | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 8bf495ffb020..e0606c01e8ac 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig"
> > source "drivers/gpu/host1x/Kconfig"
> > source "drivers/gpu/ipu-v3/Kconfig"
> >
> > -menu "Direct Rendering Manager"
> > source "drivers/gpu/drm/Kconfig"
> > -endmenu
> >
> > menu "Frame buffer Devices"
> > source "drivers/video/fbdev/Kconfig"
> >
>
> Thanks, queued for 4.3.
Oops failed to send out my usual mail, but I picked this already up in
topic/drm-misc.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] drm: Remove two-level menu in Kconfig
From: Tomi Valkeinen @ 2015-08-10 11:54 UTC (permalink / raw)
To: Daniel Vetter; +Cc: linux-fbdev, Jean-Christophe Plagniol-Villard, dri-devel
In-Reply-To: <20150810114718.GU17734@phenom.ffwll.local>
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
On 10/08/15 14:47, Daniel Vetter wrote:
> On Mon, Aug 10, 2015 at 01:32:08PM +0300, Tomi Valkeinen wrote:
>>
>>
>> On 07/08/15 19:10, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The Direct Rendering Manager Kconfig option is already a separate menu,
>>> so remove the extra level to make it easier to navigate.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> drivers/video/Kconfig | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>> index 8bf495ffb020..e0606c01e8ac 100644
>>> --- a/drivers/video/Kconfig
>>> +++ b/drivers/video/Kconfig
>>> @@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig"
>>> source "drivers/gpu/host1x/Kconfig"
>>> source "drivers/gpu/ipu-v3/Kconfig"
>>>
>>> -menu "Direct Rendering Manager"
>>> source "drivers/gpu/drm/Kconfig"
>>> -endmenu
>>>
>>> menu "Frame buffer Devices"
>>> source "drivers/video/fbdev/Kconfig"
>>>
>>
>> Thanks, queued for 4.3.
>
> Oops failed to send out my usual mail, but I picked this already up in
> topic/drm-misc.
Ok, dropping this from fbdev tree.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] fbdev/nvidia:change reverse_order() macro
From: yalin wang @ 2015-08-10 13:54 UTC (permalink / raw)
To: adaplas, plagnioj, tomi.valkeinen, linux-fbdev, open list
This change reverse_order() to swab32(bitrev32()), so that
it can have better performance on some platforms.
Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
---
drivers/video/fbdev/nvidia/nv_accel.c | 4 ++--
drivers/video/fbdev/nvidia/nv_local.h | 13 ++++---------
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/video/fbdev/nvidia/nv_accel.c b/drivers/video/fbdev/nvidia/nv_accel.c
index ad6472a..c21cb34 100644
--- a/drivers/video/fbdev/nvidia/nv_accel.c
+++ b/drivers/video/fbdev/nvidia/nv_accel.c
@@ -382,7 +382,7 @@ static void nvidiafb_mono_color_expand(struct fb_info *info,
for (j = RECT_EXPAND_TWO_COLOR_DATA_MAX_DWORDS; j--;) {
tmp = data[k++];
- reverse_order(&tmp);
+ tmp = reverse_order(tmp);
NVDmaNext(par, tmp);
}
@@ -394,7 +394,7 @@ static void nvidiafb_mono_color_expand(struct fb_info *info,
for (j = dsize; j--;) {
tmp = data[k++];
- reverse_order(&tmp);
+ tmp = reverse_order(tmp);
NVDmaNext(par, tmp);
}
}
diff --git a/drivers/video/fbdev/nvidia/nv_local.h b/drivers/video/fbdev/nvidia/nv_local.h
index 68e508d..a0eb1f3 100644
--- a/drivers/video/fbdev/nvidia/nv_local.h
+++ b/drivers/video/fbdev/nvidia/nv_local.h
@@ -97,18 +97,13 @@
#ifdef __LITTLE_ENDIAN
+#include <linux/swab.h>
#include <linux/bitrev.h>
-#define reverse_order(l) \
-do { \
- u8 *a = (u8 *)(l); \
- a[0] = bitrev8(a[0]); \
- a[1] = bitrev8(a[1]); \
- a[2] = bitrev8(a[2]); \
- a[3] = bitrev8(a[3]); \
-} while(0)
+#define reverse_order(v) swab32(bitrev32(v))
+
#else
-#define reverse_order(l) do { } while(0)
+#define reverse_order(v) (v)
#endif /* __LITTLE_ENDIAN */
#endif /* __NV_LOCAL_H__ */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] fbcon: unconditionally initialize cursor blink interval
From: Scot Doyle @ 2015-08-10 14:05 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe Plagniol-Villard, Jonathan Liu, Pavel Machek,
Thierry Reding, Kevin Hilman, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, linux-fbdev, linux-kernel
In-Reply-To: <20150804203250.GA655@amd>
On Tue, 4 Aug 2015, Pavel Machek wrote:
> On Tue 2015-08-04 12:33:32, Scot Doyle wrote:
> > A sun7i-a20-olinuxino-micro fails to boot when kernel parameter
> > vt.global_cursor_default=0. The value is copied to vc->vc_deccm
> > causing the initialization of ops->cur_blink_jiffies to be skipped.
> > Unconditionally initialize it.
> >
> > Reported-and-tested-by: Jonathan Liu <net147@gmail.com>
> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
Tomi, this one should also go in 4.2 fixes.
^ permalink raw reply
* Re: [PATCH] fbcon: unconditionally initialize cursor blink interval
From: Tomi Valkeinen @ 2015-08-10 14:21 UTC (permalink / raw)
To: Scot Doyle
Cc: Jean-Christophe Plagniol-Villard, Jonathan Liu, Pavel Machek,
Thierry Reding, Kevin Hilman, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1508101358210.1286@local>
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
On 10/08/15 17:05, Scot Doyle wrote:
> On Tue, 4 Aug 2015, Pavel Machek wrote:
>> On Tue 2015-08-04 12:33:32, Scot Doyle wrote:
>>> A sun7i-a20-olinuxino-micro fails to boot when kernel parameter
>>> vt.global_cursor_default=0. The value is copied to vc->vc_deccm
>>> causing the initialization of ops->cur_blink_jiffies to be skipped.
>>> Unconditionally initialize it.
>>>
>>> Reported-and-tested-by: Jonathan Liu <net147@gmail.com>
>>> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
>>
>> Acked-by: Pavel Machek <pavel@ucw.cz>
>>
>
> Tomi, this one should also go in 4.2 fixes.
Thanks, I've queued for 4.2 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox