* Re: [PATCH v8 09/16] clockevents/drivers: Add STM32 Timer driver
From: Daniel Lezcano @ 2015-05-19 9:59 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Joe Perches,
Vladimir Zapolskiy, Lee Jones, Daniel Thompson, Jonathan Corbet,
Pawel Moll, Mark Rutland
In-Reply-To: <CALszF6Asfke0-cMQJcEQ8ya+rbPPPhUc6aXTZXQvQ8QF66cwYQ@mail.gmail.com>
On 05/19/2015 11:44 AM, Maxime Coquelin wrote:
> 2015-05-19 11:06 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
>>>
>>> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>
>>>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>>>
>>>>>
>>>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>>
>>>>>>
>>>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>>>> prescalers.
>>>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>>>> 1024 prescaler value if it is 16 bits.
>>>>>>>
>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>>> ---
>>>>>>> drivers/clocksource/Kconfig | 8 ++
>>>>>>> drivers/clocksource/Makefile | 1 +
>>>>>>> drivers/clocksource/timer-stm32.c | 184
>>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>> 3 files changed, 193 insertions(+)
>>>>>>> create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>>>
>>>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>>>> index bf9364c..2443520 100644
>>>>>>> --- a/drivers/clocksource/Kconfig
>>>>>>> +++ b/drivers/clocksource/Kconfig
>>>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>>> Support to use the timers of EFM32 SoCs as clock source
>>>>>>> and
>>>>>>> clock
>>>>>>> event device.
>>>>>>>
>>>>>>> +config CLKSRC_STM32
>>>>>>> + bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>>>> + depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>>>
>>>>>
>>>>> The interactive bool is necessary if we want to be able to
>>>>> select/deselect it in COMPILE_TEST configuration.
>>>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>>>
>>>>> Note that other timer drivers are doing the same thing today
>>>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>>>
>>>>> Do you have a specific concern regarding COMPILE_TEST?
>>>>
>>>>
>>>>
>>>> Actually, we try to keep the timer selection non-interactive and let the
>>>> platform's Kconfig to select the timer.
>>>
>>>
>>> Ok.
>>>
>>>>
>>>> I like when the code is consistent. The COMPILE_TEST was introduced and
>>>> created a precedence. I would like to get rid of the interactive timer
>>>> selection but I did not have time to go through this yet.
>>>
>>>
>>> Indeed, consistency is important.
>>> On my side, I don't have a strong opinion regarding the COMPILE_TEST
>>> thing.
>>> IMHO, it is more a subsystem's maintainer choice.
>>>
>>> So, if as a maintainer you don't use it and prefer not supporting it,
>>> I'm fine to provide you a new version without COMPILE_TEST.
>>> Doing that, the interactive selection will disappear too.
>>>
>>> I can provide you a new version this evenning.
>>
>>
>> Ok, great.
>
> Is the below Kconfig entry fine for you?
>
> config CLKSRC_STM32
> def_bool y if ARCH_STM32
> select CLKSRC_MMIO
config CLKSRC_STM32
bool
select CLKSRC_MMIO
and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32
> Best regards,
> Maxime
>
>
>>
>> Thanks
>> -- Daniel
>>
>>
>>>>
>>>>
>>>>
>>>> --
>>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>>
>>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>>>> <http://twitter.com/#!/linaroorg> Twitter |
>>>> <http://www.linaro.org/linaro-blog/> Blog
>>>>
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v8 09/16] clockevents/drivers: Add STM32 Timer driver
From: Maxime Coquelin @ 2015-05-19 9:44 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Joe Perches,
Vladimir Zapolskiy, Lee Jones, Daniel Thompson, Jonathan Corbet,
Pawel Moll, Mark Rutland, Ian Campbell
In-Reply-To: <555AFD18.2070009@linaro.org>
2015-05-19 11:06 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
>>
>> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>
>>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>>
>>>>
>>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>
>>>>>
>>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>>> prescalers.
>>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>>> 1024 prescaler value if it is 16 bits.
>>>>>>
>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>> ---
>>>>>> drivers/clocksource/Kconfig | 8 ++
>>>>>> drivers/clocksource/Makefile | 1 +
>>>>>> drivers/clocksource/timer-stm32.c | 184
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>> 3 files changed, 193 insertions(+)
>>>>>> create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>>
>>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>>> index bf9364c..2443520 100644
>>>>>> --- a/drivers/clocksource/Kconfig
>>>>>> +++ b/drivers/clocksource/Kconfig
>>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>> Support to use the timers of EFM32 SoCs as clock source
>>>>>> and
>>>>>> clock
>>>>>> event device.
>>>>>>
>>>>>> +config CLKSRC_STM32
>>>>>> + bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>>> + depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>>
>>>>
>>>> The interactive bool is necessary if we want to be able to
>>>> select/deselect it in COMPILE_TEST configuration.
>>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>>
>>>> Note that other timer drivers are doing the same thing today
>>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>>
>>>> Do you have a specific concern regarding COMPILE_TEST?
>>>
>>>
>>>
>>> Actually, we try to keep the timer selection non-interactive and let the
>>> platform's Kconfig to select the timer.
>>
>>
>> Ok.
>>
>>>
>>> I like when the code is consistent. The COMPILE_TEST was introduced and
>>> created a precedence. I would like to get rid of the interactive timer
>>> selection but I did not have time to go through this yet.
>>
>>
>> Indeed, consistency is important.
>> On my side, I don't have a strong opinion regarding the COMPILE_TEST
>> thing.
>> IMHO, it is more a subsystem's maintainer choice.
>>
>> So, if as a maintainer you don't use it and prefer not supporting it,
>> I'm fine to provide you a new version without COMPILE_TEST.
>> Doing that, the interactive selection will disappear too.
>>
>> I can provide you a new version this evenning.
>
>
> Ok, great.
Is the below Kconfig entry fine for you?
config CLKSRC_STM32
def_bool y if ARCH_STM32
select CLKSRC_MMIO
Best regards,
Maxime
>
> Thanks
> -- Daniel
>
>
>>>
>>>
>>>
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>
>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply
* Re: [PATCH 15/19] y2038: introduce timespec64_to_jiffies
From: Thomas Gleixner @ 2015-05-19 9:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: libc-alpha, baolin.wang, y2038, linux-api, ruchandani.tina,
linux-kernel, albert.aribaud, john.stultz, bamvor.zhangjian
In-Reply-To: <1430929826-318934-16-git-send-email-arnd@arndb.de>
On Wed, 6 May 2015, Arnd Bergmann wrote:
> This is needed to convert do_sigtimedwait to use timespec64.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 16/19] y2038: use __kernel_timespec in sys_rt_sigtimedwait
From: Thomas Gleixner @ 2015-05-19 9:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: libc-alpha, baolin.wang, y2038, linux-api, ruchandani.tina, LKML,
albert.aribaud, John Stultz, Oleg Nesterov, bamvor.zhangjian
In-Reply-To: <1430929826-318934-17-git-send-email-arnd@arndb.de>
On Wed, 6 May 2015, Arnd Bergmann wrote:
> This is a straightforward conversion of the native and compat
> sys_rt_sigtimedwait functions to use __kernel_timespec, so
> 32-bit user space can pass a 64-bit time_t into the native
> syscall and use the compat syscall for the traditional 32-bit
> time_t.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 19/19] y2038: use __kernel_timespec in sys_sched_rr_get_interval
From: Thomas Gleixner @ 2015-05-19 9:27 UTC (permalink / raw)
To: Arnd Bergmann
Cc: libc-alpha, baolin.wang, y2038, linux-api, ruchandani.tina,
linux-kernel, albert.aribaud, john.stultz, bamvor.zhangjian
In-Reply-To: <1430929826-318934-20-git-send-email-arnd@arndb.de>
On Wed, 6 May 2015, Arnd Bergmann wrote:
> sys_sched_rr_get_interval is easily converted to use __kernel_timespec,
> by changing the function prototype. In order to allow compat handling
> on 32-bit architectures, we also move compat_sys_sched_getaffinity
> into the same file and unify the implementation, which avoids converting
> the structure multiple times.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH 18/19] y2038: introduce jiffies_to_timespec64
From: Thomas Gleixner @ 2015-05-19 9:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038-cunTk1MwBs8s++Sfvej+rw, baolin.wang-QSEj5FYQhm4dnm+yROfE0A,
albert.aribaud-iEu9NFBzPZE, john.stultz-QSEj5FYQhm4dnm+yROfE0A,
bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A,
ruchandani.tina-Re5JQEeQqe8AvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
libc-alpha-9JcytcrH/bA+uJoB2kUjGw
In-Reply-To: <1430929826-318934-19-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
On Wed, 6 May 2015, Arnd Bergmann wrote:
> Needed for converting sys_sched_rr_get_interval
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Reviewed-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH 17/19] y2038: use __kernel_timespec in sys_futex
From: Thomas Gleixner @ 2015-05-19 9:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038-cunTk1MwBs8s++Sfvej+rw, baolin.wang-QSEj5FYQhm4dnm+yROfE0A,
albert.aribaud-iEu9NFBzPZE, john.stultz-QSEj5FYQhm4dnm+yROfE0A,
bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A,
ruchandani.tina-Re5JQEeQqe8AvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
libc-alpha-9JcytcrH/bA+uJoB2kUjGw
In-Reply-To: <1430929826-318934-18-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
On Wed, 6 May 2015, Arnd Bergmann wrote:
> Conversion for sys_futex is particularly easy, we can use the unmodified
> compat_sys_futex on 32-bit systems to provide compatibility for 32-bit
> time_t, and change sys_futex to pass a __kernel_timespec, which matches
> what future libc implementations will use as their struct timespec.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Reviewed-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH 13/19] y2038: add compat handling for sys_semtimedop
From: Thomas Gleixner @ 2015-05-19 9:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, baolin.wang, albert.aribaud, john.stultz, bamvor.zhangjian,
ruchandani.tina, linux-api, linux-kernel, libc-alpha
In-Reply-To: <5686498.JnFHvQTH21@wuerfel>
On Sat, 16 May 2015, Arnd Bergmann wrote:
> On Saturday 16 May 2015 00:46:44 Thomas Gleixner wrote:
> > On Wed, 6 May 2015, Arnd Bergmann wrote:
> > > +SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> > > + unsigned, nsops,
> > > + const struct __kernel_timespec __user *, timeout)
> > > +{
> > > + unsigned long jiffies_left = 0;
> > > +
> > > + if (timeout) {
> > > + struct timespec64 _timeout;
> > > + if (get_timespec64(&_timeout, timeout))
> >
> > Moo. I had to look 3 times to get not confused by the extra
> > underscore. What's wrong with a proper variable name which is easy to
> > distinguish?
> >
> > > + return -EFAULT;
> >
> > > + if (_timeout.tv_sec < 0 || _timeout.tv_nsec < 0 ||
> > > + _timeout.tv_nsec >= 1000000000L)
> > > + return -EINVAL;
> >
> > We have proper helper functions to validate time specs.
>
> I ended up fixing both issues you noticed in the same patch
> after all, and also simplified it slightly more.
>
> Finally, I also noticed that I had not done a timespec64_to_jiffies()
> call at the time when I wrote this patch, but it actually exists now,
> so I've reordered my series and am using it in the new version, as
> I should have done to start with.
Indeed. I didn't notice either.
> >From e04b14d49273c27d92f1799233b82bcdafb43d9a Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Mon, 27 Apr 2015 23:30:39 +0200
> Subject: [UPDATED PATCH] y2038: add compat handling for sys_semtimedop
>
> This moves the compat_sys_semtimedop function to ipc/sem.c so it
> can be shared with 32-bit architectures efficiently. Instead of
> copying the timespec back to user space, we take a shortcut and
> pass the kernel timespec64 value to the low-level implementation
> directly.
>
> The native sys_semtimedop() function is modified to take a
> __kernel_timespec structure, which will be based on a 64-bit
> time_t in the future.
>
> There is a small API change here: if multiple errors are present,
> and the timespec argument is an invalid pointer, we now return
> -EFAULT before checking any of the other error conditions.
> This is what the compat version has always done, but if it is a
> problem, we need a more sophisticated approach.
The important part of error checks is that they catch all
cases and combinations. In which order is completely irrelevant.
If something relies on the ordering of error check returns, it's
broken by definition.
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* Re: [PATCH v8 09/16] clockevents/drivers: Add STM32 Timer driver
From: Daniel Lezcano @ 2015-05-19 9:06 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Joe Perches,
Vladimir Zapolskiy, Lee Jones, Daniel Thompson, Jonathan Corbet,
Pawel Moll, Mark Rutland
In-Reply-To: <CALszF6CwGuKqgbX6gVrya1-_YOgvtrgC7pVqKTNjCRif_o532A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>
>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano-QSEj5FYQhm5QFI55V6+gNQ@public.gmane.orgg>:
>>>>
>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>
>>>>>
>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>> prescalers.
>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>> 1024 prescaler value if it is 16 bits.
>>>>>
>>>>> Reviewed-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>> Tested-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>> drivers/clocksource/Kconfig | 8 ++
>>>>> drivers/clocksource/Makefile | 1 +
>>>>> drivers/clocksource/timer-stm32.c | 184
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 193 insertions(+)
>>>>> create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>
>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>> index bf9364c..2443520 100644
>>>>> --- a/drivers/clocksource/Kconfig
>>>>> +++ b/drivers/clocksource/Kconfig
>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>> Support to use the timers of EFM32 SoCs as clock source and
>>>>> clock
>>>>> event device.
>>>>>
>>>>> +config CLKSRC_STM32
>>>>> + bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>> + depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>
>>>>
>>>>
>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>
>>>
>>> The interactive bool is necessary if we want to be able to
>>> select/deselect it in COMPILE_TEST configuration.
>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>
>>> Note that other timer drivers are doing the same thing today
>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>
>>> Do you have a specific concern regarding COMPILE_TEST?
>>
>>
>> Actually, we try to keep the timer selection non-interactive and let the
>> platform's Kconfig to select the timer.
>
> Ok.
>
>>
>> I like when the code is consistent. The COMPILE_TEST was introduced and
>> created a precedence. I would like to get rid of the interactive timer
>> selection but I did not have time to go through this yet.
>
> Indeed, consistency is important.
> On my side, I don't have a strong opinion regarding the COMPILE_TEST thing.
> IMHO, it is more a subsystem's maintainer choice.
>
> So, if as a maintainer you don't use it and prefer not supporting it,
> I'm fine to provide you a new version without COMPILE_TEST.
> Doing that, the interactive selection will disappear too.
>
> I can provide you a new version this evenning.
Ok, great.
Thanks
-- Daniel
>>
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v8 09/16] clockevents/drivers: Add STM32 Timer driver
From: Maxime Coquelin @ 2015-05-19 8:55 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Joe Perches,
Vladimir Zapolskiy, Lee Jones, Daniel Thompson, Jonathan Corbet,
Pawel Moll, Mark Rutland, Ian Campbell
In-Reply-To: <555AF143.2060807@linaro.org>
2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>
>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>
>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>
>>>>
>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>> prescalers.
>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>> 1024 prescaler value if it is 16 bits.
>>>>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>> ---
>>>> drivers/clocksource/Kconfig | 8 ++
>>>> drivers/clocksource/Makefile | 1 +
>>>> drivers/clocksource/timer-stm32.c | 184
>>>> ++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 193 insertions(+)
>>>> create mode 100644 drivers/clocksource/timer-stm32.c
>>>>
>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>> index bf9364c..2443520 100644
>>>> --- a/drivers/clocksource/Kconfig
>>>> +++ b/drivers/clocksource/Kconfig
>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>> Support to use the timers of EFM32 SoCs as clock source and
>>>> clock
>>>> event device.
>>>>
>>>> +config CLKSRC_STM32
>>>> + bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>> + depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>
>>>
>>>
>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>
>>
>> The interactive bool is necessary if we want to be able to
>> select/deselect it in COMPILE_TEST configuration.
>> And personnaly, I think COMPILE_TEST use makes sense.
>>
>> Note that other timer drivers are doing the same thing today
>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>
>> Do you have a specific concern regarding COMPILE_TEST?
>
>
> Actually, we try to keep the timer selection non-interactive and let the
> platform's Kconfig to select the timer.
Ok.
>
> I like when the code is consistent. The COMPILE_TEST was introduced and
> created a precedence. I would like to get rid of the interactive timer
> selection but I did not have time to go through this yet.
Indeed, consistency is important.
On my side, I don't have a strong opinion regarding the COMPILE_TEST thing.
IMHO, it is more a subsystem's maintainer choice.
So, if as a maintainer you don't use it and prefer not supporting it,
I'm fine to provide you a new version without COMPILE_TEST.
Doing that, the interactive selection will disappear too.
I can provide you a new version this evenning.
Best regards,
Maxime
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply
* Re: [PATCH v8 09/16] clockevents/drivers: Add STM32 Timer driver
From: Daniel Lezcano @ 2015-05-19 8:16 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Joe Perches,
Vladimir Zapolskiy, Lee Jones, Daniel Thompson, Jonathan Corbet,
Pawel Moll, Mark Rutland
In-Reply-To: <CALszF6CK7axU0qrWNf38aqrt2R2126ZACn7wx5jNY_1KE+4H+g@mail.gmail.com>
On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>
>>> STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>> 1024 prescaler value if it is 16 bits.
>>>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> ---
>>> drivers/clocksource/Kconfig | 8 ++
>>> drivers/clocksource/Makefile | 1 +
>>> drivers/clocksource/timer-stm32.c | 184
>>> ++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 193 insertions(+)
>>> create mode 100644 drivers/clocksource/timer-stm32.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index bf9364c..2443520 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>> Support to use the timers of EFM32 SoCs as clock source and
>>> clock
>>> event device.
>>>
>>> +config CLKSRC_STM32
>>> + bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>> + depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>
>>
>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>
>
> The interactive bool is necessary if we want to be able to
> select/deselect it in COMPILE_TEST configuration.
> And personnaly, I think COMPILE_TEST use makes sense.
>
> Note that other timer drivers are doing the same thing today
> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>
> Do you have a specific concern regarding COMPILE_TEST?
Actually, we try to keep the timer selection non-interactive and let the
platform's Kconfig to select the timer.
I like when the code is consistent. The COMPILE_TEST was introduced and
created a precedence. I would like to get rid of the interactive timer
selection but I did not have time to go through this yet.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH man-pages v2] capabilities.7, prctl.2: Document ambient capabilities
From: Michael Kerrisk (man-pages) @ 2015-05-19 7:56 UTC (permalink / raw)
To: Andy Lutomirski, Serge Hallyn, Andrew Morton
Cc: mtk.manpages, Jarkko Sakkinen, Ted Ts'o, Andrew G. Morgan,
Linux API, Mimi Zohar, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet
In-Reply-To: <f85ae308af4946436e82aaa9dd71bcd4b39bd196.1431672156.git.luto@kernel.org>
Hi Andy,
Thanks for this patch. There are some broken pieces though. Also,
I have some minor questions about the API design. See below.
On 05/15/2015 08:43 AM, Andy Lutomirski wrote:
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>
> There was no v1. I'm calling this v2 to keep it in sync with the kernel
> patch versioning.
>
> man2/prctl.2 | 10 ++++++++++
> man7/capabilities.7 | 32 ++++++++++++++++++++++++++------
> 2 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index b352f6283624..5861e3aefe9a 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -949,6 +949,16 @@ had been called.
> For further information on Intel MPX, see the kernel source file
> .IR Documentation/x86/intel_mpx.txt .
> .\"
> +.TP
> +.BR PR_CAP_AMBIENT " (since Linux 4.2)"
> +Reads or changes the ambient capability set. If arg2 is PR_CAP_AMBIENT_RAISE,
> +then the capability specified in arg3 is added to the ambient set. This will
> +fail, returning EPERM, if the capability is not already both permitted and
> +inheritable or if the SECBIT_NO_CAP_AMBIENT_RAISE securebit is set. If arg2
> +is PR_CAP_AMBIENT_LOWER, then the capability specified in arg3 is removed
> +from the ambient set. If arg2 is PR_CAP_AMBIENT_GET, then
> +.BR prctl (2)
> +will return 1 if the capability in arg3 is in the ambient set and 0 if not.
Some API design questions:
1. We already have prctl() operations that work on some capability sets:
PR_CAPBSET_READ and PR_CAPBSET_DROP. These don't use arg3; the operation
is directly encoded in the first argument of prctl(). Just to keep some
consistency, why not do things the same way for these new operations?
Also, you could opt for some consistency in the naming, so using "READ"
rather than "GET", for example. On the other hand, both "READ" and "GET"
are suboptimal names: this is really a test operation. So, maybe a
clean break to a good name, PR_CAP_AMBIENT_IS_SET, is best?
Thus:
prctl(PR_CAP_AMBIENT_READ, cap, 0, 0, 0); // or PR_CAP_AMBIENT_IS_SET?
prctl(PR_CAP_AMBIENT_RAISE, cap, 0, 0, 0);
prctl(PR_CAP_AMBIENT_LOWER, cap, 0, 0, 0);
2. In terms of the API design, would it be useful to have a prctl() operation
that clears the entire ambient set?
prctl(PR_CAP_AMBIENT_ZERO, 0, 0, 0, 0); // or PR_CAP_AMBIENT_EMPTY?
> .SH RETURN VALUE
> On success,
> .BR PR_GET_DUMPABLE ,
> diff --git a/man7/capabilities.7 b/man7/capabilities.7
> index d75ec65de05b..dae62f0be3b7 100644
> --- a/man7/capabilities.7
> +++ b/man7/capabilities.7
> @@ -697,13 +697,26 @@ a program whose associated file capabilities grant that capability).
> .IR Inheritable :
> This is a set of capabilities preserved across an
> .BR execve (2).
> -It provides a mechanism for a process to assign capabilities
> -to the permitted set of the new program during an
> -.BR execve (2).
> +Inheritable capabilities remain inheritable when executing any program,
> +and inheritable capabilities are added to the permitted set when executing
> +a program that has the corresponding bits set in the file inheritable set.
> +When executing programs without file capabilities, ambient capabilities
That last line is incomplete. Something needs adding/removing.
> .TP
> .IR Effective :
> This is the set of capabilities used by the kernel to
> perform permission checks for the thread.
> +.TP
> +.IR Ambient " (since Linux 4.2) :"
Minor knit: s/ :/:/ for next version.
> +This is a set of capabilities that are preserved across an
> +.BR execve (2)
> +of a program that does not have file capabilities. The ambient capability
> +set obeys the invariant that no capability can ever be ambient if it is
> +not both permitted and inheritable. Ambient capabilities are, with some
> +exceptions, preserved in the permitted set and added to the effective
> +set when
> +.BR execve (2)
> +is called. The ambient capability set is modified using
> +.BR prctl (2).
I think it would be helpful to add a couple of sentences here on why the
ambient set is useful (i.e., explain what deficiencies in the pre-existing
API are addressed by the addition of this set--a brief piece from your
1/2 patch, for example).
> .PP
> A child created via
> .BR fork (2)
> @@ -785,10 +798,12 @@ the process using the following algorithm:
> .in +4n
> .nf
>
> +P'(ambient) = (file has capabilities or is setuid or setgid) ? 0 : P(ambient)
> +
> P'(permitted) = (P(inheritable) & F(inheritable)) |
> - (F(permitted) & cap_bset)
> + (F(permitted) & cap_bset) | P'(ambient)
>
> -P'(effective) = F(effective) ? P'(permitted) : 0
> +P'(effective) = F(effective) ? P'(permitted) : P'(ambient)
>
> P'(inheritable) = P(inheritable) [i.e., unchanged]
>
> @@ -1071,6 +1086,10 @@ an effective or real UID of 0 calls
> .BR execve (2).
> (See the subsection
> .IR "Capabilities and execution of programs by root" .)
> +.TP
> +.B SECBIT_NO_CAP_AMBIENT_RAISE
> +Setting this flag disallows
> +.BR PR_CAP_AMBIENT_RAISE .
> .PP
> Each of the above "base" flags has a companion "locked" flag.
> Setting any of the "locked" flags is irreversible,
> @@ -1079,8 +1098,9 @@ corresponding "base" flag.
> The locked flags are:
> .BR SECBIT_KEEP_CAPS_LOCKED ,
> .BR SECBIT_NO_SETUID_FIXUP_LOCKED ,
> +.BR SECBIT_NOROOT_LOCKED ,
> and
> -.BR SECBIT_NOROOT_LOCKED .
> +.BR SECBIT_NO_CAP_AMBIENT_RAISE .
> .PP
> The
> .I securebits
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH v2] Documentation/arch: Add kernel feature descriptions and arch support status under Documentation/features/
From: Michael Ellerman @ 2015-05-19 2:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Josh Triplett, Borislav Petkov, Jonathan Corbet,
Peter Zijlstra, Andy Lutomirski, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Linus Torvalds, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-arch-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150518085423.GA24944-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, 2015-05-18 at 10:54 +0200, Ingo Molnar wrote:
> * Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
>
> > > > Yes it is. I have my own version I've cobbled together for
> > > > powerpc, but this is much better.
> > >
> > > Please double check the PowerPC support matrix for correctness (if
> > > you haven't yet):
> >
> > It looks good except for:
> >
> > > rwsem-optimized: | ok | Optimized asm/rwsem.h # arch provides optimized rwsem APIs
> >
> > I don't see an rwsem.h in powerpc anywhere?
>
> Indeed, that slipped through. I fixed it to:
>
> triton:~/tip> cat Documentation/features/locking/rwsem-optimized/arch-support.txt
> #
> # Feature name: rwsem-optimized
> # Kconfig: Optimized asm/rwsem.h
> # description: arch provides optimized rwsem APIs
> #
> | powerpc: | TODO |
>
> triton:~/tip> cat Documentation/features/time/irq-time-acct/arch-support.txt
> #
> # Feature name: irq-time-acct
> # Kconfig: HAVE_IRQ_TIME_ACCOUNTING
> # description: arch supports precise IRQ time accounting
> #
> | powerpc: | .. |
>
> i.e. it's not marked as TODO, but not as supported either which would
> be misleading.
Great thanks.
cheers
^ permalink raw reply
* Re: [klibc] kernel/libc uapi changes for y2038
From: Arnd Bergmann @ 2015-05-18 20:35 UTC (permalink / raw)
To: y2038
Cc: klibc, libc-alpha, linux-api, musl, linux-kernel, Rich Felker,
Thorsten Glaser, cferris, enh, Joseph S. Myers
In-Reply-To: <Pine.BSM.4.64L.1505181658330.32668@herc.mirbsd.org>
On Monday 18 May 2015 17:03:08 Thorsten Glaser wrote:
> >MIPS on the other hand is no more broken than any of the other 32-bit
> >ABIs, because it does not use 64-bit __kernel_long_t in its n32 ABI.
>
> I have heard from a MIPS porter that one of the flavours suffers
> from similar problems as x32, just not to that extent. But I
> don’t recall my source…
MIPS n32 has a lot of the same issues as x86 x32, but I'm pretty
sure that the time_t one is not among them.
> >ioctls. My plan at this point is to eliminate all uses of time_t in
> >the kernel and replace them with time64_t or other safe types.
> >This way, we will eventually find all code that passes 32-bit time types
> >to user space and can fix it. This will take care of the time_t
> >related problems on x32 as well.
>
> Ah, interesting approach. And existing userspace, as well as new
> userspace that does not declare 64-bit time_t readiness, is still
> safe on currently-not-broken architectures? So, there’s enough time
> to fix this before the various libcs turn that on (and it had better
> be fixed by then, because it becomes ABI by then). Nice idea.
Correct. Another aspect of the approach I'm taking is that the
system-call implementation is shared between the native 64-bit
case and the new 32-bit case, while the handling for the existing
syscalls on 32-bit architectures is shared with the 32-bit compat
code on 64-bit architectures. This means if we introduce a bug
in either of them, we will find out very quickly and don't have
to wait until people start using 64-bit time_t on 32-bit user land.
> I am wondering a bit about the ioctls being hard to find. I have
> not much experience with kernel programming, and even less with
> Linux than with MS-DOS and BSD, but should not each driver have
> a central ioctl entry point, from which it should cast the user
> space data into a (possibly locally declared) structure?
Yes, that's how it works, but unfortunately we have a few thousand
drivers that declare an ioctl function, and I hope to do something
better than brute-force search all of them. The other point is that
we really need to fix all y2038-related bug in drivers, not just
the ones in ioctl. This includes things like file systems storing
time in 32-bit units on disk, or drivers trying to measure how much
time has elapsed without communicating that value elsewhere, but
failing when the time_t number goes negative.
Arnd
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH v2 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-05-18 19:48 UTC (permalink / raw)
To: Christoph Lameter
Cc: Andy Lutomirski, Serge Hallyn, Andrew Morton, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1505150916330.20103@gentwo.org>
On Fri, May 15, 2015 at 7:19 AM, Christoph Lameter <cl@linux.com> wrote:
> On Thu, 14 May 2015, Andy Lutomirski wrote:
>
>> Cc: Christoph Lameter <cl@linux.com>
>
> This is partially my code it seems. So there should be a
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> here.
>
> Could you provide an example to demonstrate how it is to be used?
> Something similar to what I had in my patch?
>
Do you mean something like:
setpriv --ambient-caps=+net_bind_service --inh-haps=+net_bind_service
--euid=500 --ruid=500 bash
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v2 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-05-18 19:43 UTC (permalink / raw)
To: Christoph Lameter
Cc: Jarkko Sakkinen, Ted Ts'o, Andrew G. Morgan, Andrew Morton,
Serge Hallyn, Michael Kerrisk, Mimi Zohar, Linux API,
Austin S Hemmelgarn, linux-security-module, Aaron Jones, LKML,
Serge Hallyn, Markku Savela, Kees Cook, Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1505150921040.20285@gentwo.org>
On May 15, 2015 11:31 PM, "Christoph Lameter" <cl@linux.com> wrote:
>
> It would be best to start a complete new thread about this. You
> replied to earlier posts about ambient capabilities and
> people may not see it as a new release.
>
> > pA obeys the invariant that no bit can ever be set in pA if it is
> > not set in both pP and pI. Dropping a bit from pP or pI drops that
> > bit from pA. This ensures that existing programs that try to drop
> > capabilities still do so, with a complication. Because capability
>
> Ok that is a good improvement.
>
> > inheritance is so broken, setting KEEPCAPS, using setresuid to
> > switch to nonroot uids, or calling execve effectively drops
> > capabilities. Therefore, setresuid from root to nonroot
> > conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set.
> > Processes that don't like this can re-add bits to pA afterwards.
> >
> > The capability evolution rules are changed:
> >
> > pA' = (file caps or setuid or setgid ? 0 : pA)
> > pP' = (X & fP) | (pI & fI) | pA'
> > pI' = pI
> > pE' = (fE ? pP' : pA')
>
> Isnt this equal to
>
> pE' = (fE & pP') | pA'
>
> which does not require conditionals and is symmetric to how pP' is
> calculated. Your formula seems to indicate that pA' bits are not set if
> fE is set. However they are already set unconditionally in pP' regardless.
> This makes it more explicit I think. And I thought we are dealing with
> bitmask arithmetic here?
I think you're right, except that fE is a Boolean, not a bit mask, so
fE | pP' is an odd thing to talk about.
We could say (fE ? pP' : 0) | pA', which could simplify the code a tiny bit.
>
>
> > If you are nonroot but you have a capability, you can add it to pA.
> > If you do so, your children get that capability in pA, pP, and pE.
> > For example, you can set pA = CAP_NET_BIND_SERVICE, and your
> > children can automatically bind low-numbered ports. Hallelujah!
>
> I love this solution.
>
> > [2] The libcap capability mask parsers and formatters are
> > dangerously misleading and the documentation is flat-out wrong. fE
> > is *not* a mask; it's a single bit. This has probably confused
> > every single person who has tried to use file capabilities.
>
> Hmmm... yes lets clean that up as well. Then your formula makes sense.
>
Maybe a follow-up patch to change the docs would be a good idea.
--Andy
^ permalink raw reply
* Re: [PATCH 0/4] Add support for V4L2_PIX_FMT_Y16_BE
From: Ricardo Ribalda Delgado @ 2015-05-18 19:31 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
Ramakrishnan Muthukrishnan, linux-media, LKML,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150518162657.031a86fc-+RedX5hVuTR+urZeOPWqwQ@public.gmane.org>
Hello Mauro
it is here https://patchwork.linuxtv.org/patch/29669/
Thanks!
On Mon, May 18, 2015 at 9:26 PM, Mauro Carvalho Chehab
<mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> Em Mon, 4 May 2015 10:07:28 +0200
> Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> escreveu:
>
>> New pixel format type Y16_BE (16 bits greyscale big-endian).
>>
>> Once I get the fist feedback on this patch I will send the patches for
>> v4lconvert and qv4l2.
>
> Hmm...
>
> Error: no ID for constraint linkend: V4L2-PIX-FMT-Y16-BE.
>
> Where's the documentation for this new format?
>
> Regards,
> Mauro
>
>
>>
>>
>> Thanks
>>
>> Ricardo Ribalda Delgado (4):
>> media/vivid: Add support for Y16 format
>> media/v4l2-core: Add support for V4L2_PIX_FMT_Y16_BE
>> media/vivid: Add support for Y16_BE format
>> media/vivid: Code cleanout
>>
>> drivers/media/platform/vivid/vivid-tpg.c | 20 ++++++++++++++++----
>> drivers/media/platform/vivid/vivid-vid-common.c | 16 ++++++++++++++++
>> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>> include/uapi/linux/videodev2.h | 1 +
>> 4 files changed, 34 insertions(+), 4 deletions(-)
>>
--
Ricardo Ribalda
^ permalink raw reply
* Re: [PATCH 0/4] Add support for V4L2_PIX_FMT_Y16_BE
From: Mauro Carvalho Chehab @ 2015-05-18 19:26 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Hans Verkuil
Cc: Laurent Pinchart, Sakari Ailus, Ramakrishnan Muthukrishnan,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1430726852-11715-1-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Em Mon, 4 May 2015 10:07:28 +0200
Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> escreveu:
> New pixel format type Y16_BE (16 bits greyscale big-endian).
>
> Once I get the fist feedback on this patch I will send the patches for
> v4lconvert and qv4l2.
Hmm...
Error: no ID for constraint linkend: V4L2-PIX-FMT-Y16-BE.
Where's the documentation for this new format?
Regards,
Mauro
>
>
> Thanks
>
> Ricardo Ribalda Delgado (4):
> media/vivid: Add support for Y16 format
> media/v4l2-core: Add support for V4L2_PIX_FMT_Y16_BE
> media/vivid: Add support for Y16_BE format
> media/vivid: Code cleanout
>
> drivers/media/platform/vivid/vivid-tpg.c | 20 ++++++++++++++++----
> drivers/media/platform/vivid/vivid-vid-common.c | 16 ++++++++++++++++
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/uapi/linux/videodev2.h | 1 +
> 4 files changed, 34 insertions(+), 4 deletions(-)
>
^ permalink raw reply
* Re: [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
From: Robert Bragg @ 2015-05-18 17:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dri-devel, David Airlie, linux-api, intel-gfx, linux-kernel,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Daniel Vetter
In-Reply-To: <20150508162143.GQ27504@twins.programming.kicks-ass.net>
On Fri, May 8, 2015 at 5:21 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> So I've not yet went through the entire series; but I'm wondering if its
> at all possible to re-use some of this work:
>
> lkml.kernel.org/r/1428453299-19121-1-git-send-email-sukadev@linux.vnet.ibm.com
>
> That's for a Power8 HV call that can basically return an array of
> values; which on a superficial level sounds a bit like what this GPU
> hardware does.
Thanks for this pointer.
I think the main similarity here is the ability to capture multiple
counters consistent for the same point in time, but in our case we
don't have an explicitly controlled transaction mechanism like this.
Although we can collect a large set of counters in a latched fashion -
so they are self consistent - the selection of counters included in
our OA unit reports is more rigid.
Most of our counters aren't independently aggregated, they are derived
from signals selected as part of the OA unit configuration and the
values are only maintained by the OA unit itself, so a
re-configuration to select different signals will discard the counter
values of currently selected signals.
afik re-configuring our signal selection is also relatively slow too
(I was told this last week at least, but I haven't tested it myself)
and so it's really geared towards applications or tools choosing a
configuration to maintain for a relatively long time while profiling a
workload.
I think the other big difference here is that we don't have a way to
explicitly trigger a report to be written from the cpu. (Although we
can read the OA counters via mmio, it's only intended for debug
purposes as this subverts the hw latching of counters) This means it
would be difficult to try and treat this like a transaction including
a fixed set of event->read()s without a way for pmu->commit_txn() to
trigger a report.
>
> Let me read more of this..
Thanks.
- Robert
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* [RFC PATCH v2] perf: Add PERF_EVENT_IOC_FLUSH ioctl
From: Robert Bragg @ 2015-05-18 17:25 UTC (permalink / raw)
To: intel-gfx
Cc: Peter Zijlstra, David Airlie, linux-api, dri-devel, linux-kernel,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Daniel Vetter
In-Reply-To: <20150507142009.GX22099@nuc-i3427.alporthouse.com>
To allow for pmus that may have internal buffering (e.g. the hardware
itself writes out data to its own circular buffer which is only
periodically forwarded to userspace via perf) this ioctl enables
userspace to explicitly ensure it has received all samples before a
point in time.
v2: return int error status
Signed-off-by: Robert Bragg <robert@sixbynine.org>
---
include/linux/perf_event.h | 7 +++++++
include/uapi/linux/perf_event.h | 1 +
kernel/events/core.c | 5 +++++
3 files changed, 13 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index cf1d096..0c591eb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -305,6 +305,13 @@ struct pmu {
* Free pmu-private AUX data structures
*/
void (*free_aux) (void *aux); /* optional */
+
+ /*
+ * Flush buffered samples (E.g. for pmu hardware that writes samples to
+ * some intermediate buffer) userspace may need to explicitly ensure
+ * such samples have been forwarded to perf.
+ */
+ int (*flush) (struct perf_event *event); /*optional */
};
/**
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 309211b..cbf1b80 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -389,6 +389,7 @@ struct perf_event_attr {
#define PERF_EVENT_IOC_SET_FILTER _IOW('$', 6, char *)
#define PERF_EVENT_IOC_ID _IOR('$', 7, __u64 *)
#define PERF_EVENT_IOC_SET_BPF _IOW('$', 8, __u32)
+#define PERF_EVENT_IOC_FLUSH _IO ('$', 9)
enum perf_event_ioc_flags {
PERF_IOC_FLAG_GROUP = 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3fe532a..72daee6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4079,6 +4079,11 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
case PERF_EVENT_IOC_SET_BPF:
return perf_event_set_bpf_prog(event, arg);
+ case PERF_EVENT_IOC_FLUSH:
+ if (event->pmu->flush)
+ return event->pmu->flush(event);
+ return 0;
+
default:
return -ENOTTY;
}
--
2.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [RFC PATCH] squash: be more careful stopping oacontrol updates
From: Robert Bragg @ 2015-05-18 17:21 UTC (permalink / raw)
To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Daniel Vetter, Jani Nikula, David Airlie, Peter Zijlstra,
Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA, Chris Wilson
In-Reply-To: <CAMou1-1QsATpCLMxmzGNTWyTEic4GtihiaRib0GJ55vsLYyaCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
This makes sure we've stopped touching oacontrol before we start
resetting OA, PM and clock gating. Shouldn't strictly be needed since we
know that oacontrol will have been disabled before we start destroying
an event but it seems worth highlighting that update_oacontrol() could
still be running asynchronously and stopping it early in case it might
become an issue in the future.
---
drivers/gpu/drm/i915/i915_oa_perf.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c
index bf1c1d6..e47ed90 100644
--- a/drivers/gpu/drm/i915/i915_oa_perf.c
+++ b/drivers/gpu/drm/i915/i915_oa_perf.c
@@ -166,12 +166,21 @@ static void i915_oa_event_destroy(struct perf_event *event)
{
struct drm_i915_private *i915 =
container_of(event->pmu, typeof(*i915), oa_pmu.pmu);
+ unsigned long lock_flags;
WARN_ON(event->parent);
- oa_buffer_destroy(i915);
-
+ /* Stop updating oacontrol via _oa_context_pin_[un]notify()... */
+ spin_lock_irqsave(&i915->oa_pmu.lock, lock_flags);
i915->oa_pmu.specific_ctx = NULL;
+ spin_unlock_irqrestore(&i915->oa_pmu.lock, lock_flags);
+
+ /* Don't let the compiler start resetting OA, PM and clock gating
+ * state before we've stopped update_oacontrol()
+ */
+ barrier();
+
+ oa_buffer_destroy(i915);
BUG_ON(i915->oa_pmu.exclusive_event != event);
i915->oa_pmu.exclusive_event = NULL;
@@ -513,6 +522,11 @@ static int i915_oa_event_init(struct perf_event *event)
return 0;
}
+/* Note: Although pmu methods are called with the corresponding
+ * perf_event_context lock taken (so we don't need to worry about our pmu
+ * methods contending with each other) update_oacontrol() may be called
+ * asynchronously via the i915_oa_pmu_[un]register() hooks.
+ */
static void update_oacontrol(struct drm_i915_private *dev_priv)
{
BUG_ON(!spin_is_locked(&dev_priv->oa_pmu.lock));
--
2.4.1
^ permalink raw reply related
* [RFC PATCH v2] drm/i915: Expose PMU for Observation Architecture
From: Robert Bragg @ 2015-05-18 17:17 UTC (permalink / raw)
To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Daniel Vetter, Jani Nikula, David Airlie, Peter Zijlstra,
Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA, Chris Wilson
In-Reply-To: <CAMou1-1QsATpCLMxmzGNTWyTEic4GtihiaRib0GJ55vsLYyaCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer and this patch exposes that
capability to userspace via the perf interface.
To start with this only enables the A (aggregating) counters with the
simplest configuration requirements.
Only Haswell is supported currently.
v2:
- fix deadlock in init_oa_buffer error path
- EBADF for bad drm fd, EINVAL for failure to lookup ctx
- mmio write barriers, after OA reconfigure, before unlocks
- use i915_mutex_lock_interruptible within event init
Signed-off-by: Robert Bragg <robert-St23OQVBDYPNLxjTenLetw@public.gmane.org>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_dma.c | 6 +
drivers/gpu/drm/i915/i915_drv.h | 53 +++
drivers/gpu/drm/i915/i915_gem_context.c | 45 +-
drivers/gpu/drm/i915/i915_oa_perf.c | 762 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_reg.h | 68 +++
include/uapi/drm/i915_drm.h | 29 ++
7 files changed, 954 insertions(+), 10 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_oa_perf.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b7ddf48..b5ebfbe 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -15,6 +15,7 @@ i915-y := i915_drv.o \
i915-$(CONFIG_COMPAT) += i915_ioc32.o
i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_PERF_EVENTS) += i915_oa_perf.o
# GEM code
i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a238889..c299e18 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -818,6 +818,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
mutex_init(&dev_priv->modeset_restore_lock);
mutex_init(&dev_priv->csr_lock);
+ /* Must at least be registered before trying to pin any context
+ * otherwise i915_oa_context_pin_notify() will lock an un-initialized
+ * spinlock, upsetting lockdep checks */
+ i915_oa_pmu_register(dev);
+
intel_pm_setup(dev);
intel_display_crc_init(dev);
@@ -1067,6 +1072,7 @@ int i915_driver_unload(struct drm_device *dev)
return ret;
}
+ i915_oa_pmu_unregister(dev);
intel_power_domains_fini(dev_priv);
intel_gpu_ips_teardown();
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a66d6b..dd475ca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -49,6 +49,7 @@
#include <linux/hashtable.h>
#include <linux/intel-iommu.h>
#include <linux/kref.h>
+#include <linux/perf_event.h>
#include <linux/pm_qos.h>
/* General customization:
@@ -1839,6 +1840,35 @@ struct drm_i915_private {
*/
struct workqueue_struct *dp_wq;
+#ifdef CONFIG_PERF_EVENTS
+ struct {
+ struct pmu pmu;
+ spinlock_t lock;
+ struct hrtimer timer;
+ struct pt_regs dummy_regs;
+
+ struct perf_event *exclusive_event;
+ struct intel_context *specific_ctx;
+ bool event_active;
+
+ bool periodic;
+ u32 period_exponent;
+
+ u32 metrics_set;
+
+ struct {
+ struct drm_i915_gem_object *obj;
+ u32 gtt_offset;
+ u8 *addr;
+ u32 head;
+ u32 tail;
+ int format;
+ int format_size;
+ spinlock_t flush_lock;
+ } oa_buffer;
+ } oa_pmu;
+#endif
+
/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
struct {
int (*execbuf_submit)(struct drm_device *dev, struct drm_file *file,
@@ -3012,6 +3042,20 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+#ifdef CONFIG_PERF_EVENTS
+void i915_oa_context_pin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context);
+void i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context);
+#else
+static inline void
+i915_oa_context_pin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context) {}
+static inline void
+i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context) {}
+#endif
+
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev,
struct i915_address_space *vm,
@@ -3121,6 +3165,15 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
u32 batch_len,
bool is_master);
+/* i915_oa_perf.c */
+#ifdef CONFIG_PERF_EVENTS
+extern void i915_oa_pmu_register(struct drm_device *dev);
+extern void i915_oa_pmu_unregister(struct drm_device *dev);
+#else
+static inline void i915_oa_pmu_register(struct drm_device *dev) {}
+static inline void i915_oa_pmu_unregister(struct drm_device *dev) {}
+#endif
+
/* i915_suspend.c */
extern int i915_save_state(struct drm_device *dev);
extern int i915_restore_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5a47eb5..3e9a7f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -133,6 +133,33 @@ static int get_context_size(struct drm_device *dev)
return ret;
}
+static int i915_gem_context_pin_state(struct drm_device *dev,
+ struct intel_context *ctx)
+{
+ int ret;
+
+ BUG_ON(!mutex_is_locked(&dev->struct_mutex));
+
+ ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+ get_context_alignment(dev), 0);
+ if (ret)
+ return ret;
+
+ i915_oa_context_pin_notify(dev->dev_private, ctx);
+
+ return 0;
+}
+
+static void i915_gem_context_unpin_state(struct drm_device *dev,
+ struct intel_context *ctx)
+{
+ /* Ensure that we stop the OA unit referencing the context *before*
+ * actually unpinning the ctx */
+ i915_oa_context_unpin_notify(dev->dev_private, ctx);
+
+ i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+}
+
void i915_gem_context_free(struct kref *ctx_ref)
{
struct intel_context *ctx = container_of(ctx_ref,
@@ -260,8 +287,7 @@ i915_gem_create_context(struct drm_device *dev,
* be available. To avoid this we always pin the default
* context.
*/
- ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
- get_context_alignment(dev), 0);
+ ret = i915_gem_context_pin_state(dev, ctx);
if (ret) {
DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
goto err_destroy;
@@ -287,7 +313,7 @@ i915_gem_create_context(struct drm_device *dev,
err_unpin:
if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(dev, ctx);
err_destroy:
i915_gem_context_unreference(ctx);
return ERR_PTR(ret);
@@ -314,7 +340,7 @@ void i915_gem_context_reset(struct drm_device *dev)
if (lctx) {
if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
- i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(dev, lctx);
i915_gem_context_unreference(lctx);
ring->last_context = NULL;
@@ -388,12 +414,12 @@ void i915_gem_context_fini(struct drm_device *dev)
if (dev_priv->ring[RCS].last_context == dctx) {
/* Fake switch to NULL context */
WARN_ON(dctx->legacy_hw_ctx.rcs_state->active);
- i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(dev, dctx);
i915_gem_context_unreference(dctx);
dev_priv->ring[RCS].last_context = NULL;
}
- i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(dev, dctx);
}
for (i = 0; i < I915_NUM_RINGS; i++) {
@@ -642,8 +668,7 @@ static int do_switch(struct intel_engine_cs *ring,
/* Trying to pin first makes error handling easier. */
if (ring == &dev_priv->ring[RCS]) {
- ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
- get_context_alignment(ring->dev), 0);
+ ret = i915_gem_context_pin_state(ring->dev, to);
if (ret)
return ret;
}
@@ -757,7 +782,7 @@ static int do_switch(struct intel_engine_cs *ring,
from->legacy_hw_ctx.rcs_state->last_read_req) != ring);
/* obj is kept alive until the next request by its active ref */
- i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(ring->dev, from);
i915_gem_context_unreference(from);
}
@@ -780,7 +805,7 @@ done:
unpin_out:
if (ring->id == RCS)
- i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unpin_state(ring->dev, to);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c b/drivers/gpu/drm/i915/i915_oa_perf.c
new file mode 100644
index 0000000..bf1c1d6
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_perf.c
@@ -0,0 +1,762 @@
+#include <linux/perf_event.h>
+#include <linux/sizes.h>
+
+#include "i915_drv.h"
+#include "intel_ringbuffer.h"
+
+/* Must be a power of two */
+#define OA_BUFFER_SIZE SZ_16M
+#define OA_TAKEN(tail, head) ((tail - head) & (OA_BUFFER_SIZE - 1))
+
+#define FREQUENCY 200
+#define PERIOD max_t(u64, 10000, NSEC_PER_SEC / FREQUENCY)
+
+static int hsw_perf_format_sizes[] = {
+ 64, /* A13_HSW */
+ 128, /* A29_HSW */
+ 128, /* A13_B8_C8_HSW */
+ -1, /* Disallowed since 192 bytes doesn't factor into buffer size
+ (A29_B8_C8_HSW) */
+ 64, /* B4_C8_HSW */
+ 256, /* A45_B8_C8_HSW */
+ 128, /* B4_C8_A16_HSW */
+ 64 /* C4_B8_HSW */
+};
+
+static void forward_one_oa_snapshot_to_event(struct drm_i915_private *dev_priv,
+ u8 *snapshot,
+ struct perf_event *event)
+{
+ struct perf_sample_data data;
+ int snapshot_size = dev_priv->oa_pmu.oa_buffer.format_size;
+ struct perf_raw_record raw;
+
+ WARN_ON(snapshot_size == 0);
+
+ perf_sample_data_init(&data, 0, event->hw.last_period);
+
+ /* Note: the combined u32 raw->size member + raw data itself must be 8
+ * byte aligned. (See note in init_oa_buffer for more details) */
+ raw.size = snapshot_size + 4;
+ raw.data = snapshot;
+
+ data.raw = &raw;
+
+ perf_event_overflow(event, &data, &dev_priv->oa_pmu.dummy_regs);
+}
+
+static u32 forward_oa_snapshots(struct drm_i915_private *dev_priv,
+ u32 head,
+ u32 tail)
+{
+ struct perf_event *exclusive_event = dev_priv->oa_pmu.exclusive_event;
+ int snapshot_size = dev_priv->oa_pmu.oa_buffer.format_size;
+ u8 *oa_buf_base = dev_priv->oa_pmu.oa_buffer.addr;
+ u32 mask = (OA_BUFFER_SIZE - 1);
+ u8 *snapshot;
+ u32 taken;
+
+ head -= dev_priv->oa_pmu.oa_buffer.gtt_offset;
+ tail -= dev_priv->oa_pmu.oa_buffer.gtt_offset;
+
+ /* Note: the gpu doesn't wrap the tail according to the OA buffer size
+ * so when we need to make sure our head/tail values are in-bounds we
+ * use the above mask.
+ */
+
+ while ((taken = OA_TAKEN(tail, head))) {
+ /* The tail increases in 64 byte increments, not in
+ * format_size steps. */
+ if (taken < snapshot_size)
+ break;
+
+ snapshot = oa_buf_base + (head & mask);
+ head += snapshot_size;
+
+ /* We currently only allow exclusive access to the counters
+ * so only have one event to forward too... */
+ if (dev_priv->oa_pmu.event_active)
+ forward_one_oa_snapshot_to_event(dev_priv, snapshot,
+ exclusive_event);
+ }
+
+ return dev_priv->oa_pmu.oa_buffer.gtt_offset + head;
+}
+
+static void flush_oa_snapshots(struct drm_i915_private *dev_priv,
+ bool skip_if_flushing)
+{
+ unsigned long flags;
+ u32 oastatus2;
+ u32 oastatus1;
+ u32 head;
+ u32 tail;
+
+ /* Can either flush via hrtimer callback or pmu methods/fops */
+ if (skip_if_flushing) {
+
+ /* If the hrtimer triggers at the same time that we are
+ * responding to a userspace initiated flush then we can
+ * just bail out...
+ */
+ if (!spin_trylock_irqsave(&dev_priv->oa_pmu.oa_buffer.flush_lock,
+ flags))
+ return;
+ } else
+ spin_lock_irqsave(&dev_priv->oa_pmu.oa_buffer.flush_lock, flags);
+
+ WARN_ON(!dev_priv->oa_pmu.oa_buffer.addr);
+
+ oastatus2 = I915_READ(GEN7_OASTATUS2);
+ oastatus1 = I915_READ(GEN7_OASTATUS1);
+
+ head = oastatus2 & GEN7_OASTATUS2_HEAD_MASK;
+ tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
+
+ if (oastatus1 & (GEN7_OASTATUS1_OABUFFER_OVERFLOW |
+ GEN7_OASTATUS1_REPORT_LOST)) {
+
+ /* XXX: How can we convey report-lost errors to userspace? It
+ * doesn't look like perf's _REPORT_LOST mechanism is
+ * appropriate in this case; that's just for cases where we
+ * run out of space for samples in the perf circular buffer.
+ *
+ * Maybe we can claim a special report-id and use that to
+ * forward status flags?
+ */
+ pr_debug("OA buffer read error: addr = %p, head = %u, offset = %u, tail = %u cnt o'flow = %d, buf o'flow = %d, rpt lost = %d\n",
+ dev_priv->oa_pmu.oa_buffer.addr,
+ head,
+ head - dev_priv->oa_pmu.oa_buffer.gtt_offset,
+ tail,
+ oastatus1 & GEN7_OASTATUS1_COUNTER_OVERFLOW ? 1 : 0,
+ oastatus1 & GEN7_OASTATUS1_OABUFFER_OVERFLOW ? 1 : 0,
+ oastatus1 & GEN7_OASTATUS1_REPORT_LOST ? 1 : 0);
+
+ I915_WRITE(GEN7_OASTATUS1, oastatus1 &
+ ~(GEN7_OASTATUS1_OABUFFER_OVERFLOW |
+ GEN7_OASTATUS1_REPORT_LOST));
+ }
+
+ head = forward_oa_snapshots(dev_priv, head, tail);
+
+ I915_WRITE(GEN7_OASTATUS2, (head & GEN7_OASTATUS2_HEAD_MASK) |
+ GEN7_OASTATUS2_GGTT);
+
+ spin_unlock_irqrestore(&dev_priv->oa_pmu.oa_buffer.flush_lock, flags);
+}
+
+static void
+oa_buffer_destroy(struct drm_i915_private *i915)
+{
+ mutex_lock(&i915->dev->struct_mutex);
+
+ vunmap(i915->oa_pmu.oa_buffer.addr);
+ i915_gem_object_ggtt_unpin(i915->oa_pmu.oa_buffer.obj);
+ drm_gem_object_unreference(&i915->oa_pmu.oa_buffer.obj->base);
+
+ i915->oa_pmu.oa_buffer.obj = NULL;
+ i915->oa_pmu.oa_buffer.gtt_offset = 0;
+ i915->oa_pmu.oa_buffer.addr = NULL;
+
+ mutex_unlock(&i915->dev->struct_mutex);
+}
+
+static void i915_oa_event_destroy(struct perf_event *event)
+{
+ struct drm_i915_private *i915 =
+ container_of(event->pmu, typeof(*i915), oa_pmu.pmu);
+
+ WARN_ON(event->parent);
+
+ oa_buffer_destroy(i915);
+
+ i915->oa_pmu.specific_ctx = NULL;
+
+ BUG_ON(i915->oa_pmu.exclusive_event != event);
+ i915->oa_pmu.exclusive_event = NULL;
+
+ intel_uncore_forcewake_put(i915, FORCEWAKE_ALL);
+ intel_runtime_pm_put(i915);
+}
+
+static void *vmap_oa_buffer(struct drm_i915_gem_object *obj)
+{
+ int i;
+ void *addr = NULL;
+ struct sg_page_iter sg_iter;
+ struct page **pages;
+
+ pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
+ if (pages == NULL) {
+ DRM_DEBUG_DRIVER("Failed to get space for pages\n");
+ goto finish;
+ }
+
+ i = 0;
+ for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0) {
+ pages[i] = sg_page_iter_page(&sg_iter);
+ i++;
+ }
+
+ addr = vmap(pages, i, 0, PAGE_KERNEL);
+ if (addr == NULL) {
+ DRM_DEBUG_DRIVER("Failed to vmap pages\n");
+ goto finish;
+ }
+
+finish:
+ if (pages)
+ drm_free_large(pages);
+ return addr;
+}
+
+static int init_oa_buffer(struct perf_event *event)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu);
+ struct drm_i915_gem_object *bo;
+ int ret;
+
+ BUG_ON(!IS_HASWELL(dev_priv->dev));
+ BUG_ON(mutex_is_locked(&dev_priv->dev->struct_mutex));
+ BUG_ON(dev_priv->oa_pmu.oa_buffer.obj);
+
+ ret = i915_mutex_lock_interruptible(dev_priv->dev);
+ if (ret)
+ return ret;
+
+ spin_lock_init(&dev_priv->oa_pmu.oa_buffer.flush_lock);
+
+ /* NB: We over allocate the OA buffer due to the way raw sample data
+ * gets copied from the gpu mapped circular buffer into the perf
+ * circular buffer so that only one copy is required.
+ *
+ * For each perf sample (raw->size + 4) needs to be 8 byte aligned,
+ * where the 4 corresponds to the 32bit raw->size member that's
+ * added to the sample header that userspace sees.
+ *
+ * Due to the + 4 for the size member: when we copy a report to the
+ * userspace facing perf buffer we always copy an additional 4 bytes
+ * from the subsequent report to make up for the miss alignment, but
+ * when a report is at the end of the gpu mapped buffer we need to
+ * read 4 bytes past the end of the buffer.
+ */
+ bo = i915_gem_alloc_object(dev_priv->dev, OA_BUFFER_SIZE + PAGE_SIZE);
+ if (bo == NULL) {
+ DRM_ERROR("Failed to allocate OA buffer\n");
+ ret = -ENOMEM;
+ goto unlock;
+ }
+ dev_priv->oa_pmu.oa_buffer.obj = bo;
+
+ ret = i915_gem_object_set_cache_level(bo, I915_CACHE_LLC);
+ if (ret)
+ goto err_unref;
+
+ /* PreHSW required 512K alignment, HSW requires 16M */
+ ret = i915_gem_obj_ggtt_pin(bo, SZ_16M, 0);
+ if (ret)
+ goto err_unref;
+
+ dev_priv->oa_pmu.oa_buffer.gtt_offset = i915_gem_obj_ggtt_offset(bo);
+ dev_priv->oa_pmu.oa_buffer.addr = vmap_oa_buffer(bo);
+
+ /* Pre-DevBDW: OABUFFER must be set with counters off,
+ * before OASTATUS1, but after OASTATUS2 */
+ I915_WRITE(GEN7_OASTATUS2, dev_priv->oa_pmu.oa_buffer.gtt_offset |
+ GEN7_OASTATUS2_GGTT); /* head */
+ I915_WRITE(GEN7_OABUFFER, dev_priv->oa_pmu.oa_buffer.gtt_offset);
+ I915_WRITE(GEN7_OASTATUS1, dev_priv->oa_pmu.oa_buffer.gtt_offset |
+ GEN7_OASTATUS1_OABUFFER_SIZE_16M); /* tail */
+
+ DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr = %p",
+ dev_priv->oa_pmu.oa_buffer.gtt_offset,
+ dev_priv->oa_pmu.oa_buffer.addr);
+
+ goto unlock;
+
+err_unref:
+ drm_gem_object_unreference(&bo->base);
+
+unlock:
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+ return ret;
+}
+
+static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
+{
+ struct drm_i915_private *i915 =
+ container_of(hrtimer, typeof(*i915), oa_pmu.timer);
+
+ flush_oa_snapshots(i915, true);
+
+ hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
+ return HRTIMER_RESTART;
+}
+
+static struct intel_context *
+lookup_context(struct drm_i915_private *dev_priv,
+ struct file *user_filp,
+ u32 ctx_user_handle)
+{
+ struct intel_context *ctx;
+
+ mutex_lock(&dev_priv->dev->struct_mutex);
+ list_for_each_entry(ctx, &dev_priv->context_list, link) {
+ struct drm_file *drm_file;
+
+ if (!ctx->file_priv)
+ continue;
+
+ drm_file = ctx->file_priv->file;
+
+ if (user_filp->private_data == drm_file &&
+ ctx->user_handle == ctx_user_handle) {
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+ return ctx;
+ }
+ }
+ mutex_unlock(&dev_priv->dev->struct_mutex);
+
+ return NULL;
+}
+
+static int i915_oa_copy_attr(drm_i915_oa_attr_t __user *uattr,
+ drm_i915_oa_attr_t *attr)
+{
+ u32 size;
+ int ret;
+
+ if (!access_ok(VERIFY_WRITE, uattr, I915_OA_ATTR_SIZE_VER0))
+ return -EFAULT;
+
+ /*
+ * zero the full structure, so that a short copy will be nice.
+ */
+ memset(attr, 0, sizeof(*attr));
+
+ ret = get_user(size, &uattr->size);
+ if (ret)
+ return ret;
+
+ if (size > PAGE_SIZE) /* silly large */
+ goto err_size;
+
+ if (size < I915_OA_ATTR_SIZE_VER0)
+ goto err_size;
+
+ /*
+ * If we're handed a bigger struct than we know of,
+ * ensure all the unknown bits are 0 - i.e. new
+ * user-space does not rely on any kernel feature
+ * extensions we dont know about yet.
+ */
+ if (size > sizeof(*attr)) {
+ unsigned char __user *addr;
+ unsigned char __user *end;
+ unsigned char val;
+
+ addr = (void __user *)uattr + sizeof(*attr);
+ end = (void __user *)uattr + size;
+
+ for (; addr < end; addr++) {
+ ret = get_user(val, addr);
+ if (ret)
+ return ret;
+ if (val)
+ goto err_size;
+ }
+ size = sizeof(*attr);
+ }
+
+ ret = copy_from_user(attr, uattr, size);
+ if (ret)
+ return -EFAULT;
+
+ if (attr->__reserved_1)
+ return -EINVAL;
+
+out:
+ return ret;
+
+err_size:
+ put_user(sizeof(*attr), &uattr->size);
+ ret = -E2BIG;
+ goto out;
+}
+
+static int i915_oa_event_init(struct perf_event *event)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu);
+ drm_i915_oa_attr_t oa_attr;
+ u64 report_format;
+ int ret = 0;
+
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ ret = i915_oa_copy_attr(to_user_ptr(event->attr.config), &oa_attr);
+ if (ret)
+ return ret;
+
+ /* To avoid the complexity of having to accurately filter
+ * counter snapshots and marshal to the appropriate client
+ * we currently only allow exclusive access */
+ if (dev_priv->oa_pmu.oa_buffer.obj)
+ return -EBUSY;
+
+ report_format = oa_attr.format;
+ dev_priv->oa_pmu.oa_buffer.format = report_format;
+ dev_priv->oa_pmu.metrics_set = oa_attr.metrics_set;
+
+ if (IS_HASWELL(dev_priv->dev)) {
+ int snapshot_size;
+
+ if (report_format >= ARRAY_SIZE(hsw_perf_format_sizes))
+ return -EINVAL;
+
+ snapshot_size = hsw_perf_format_sizes[report_format];
+ if (snapshot_size < 0)
+ return -EINVAL;
+
+ dev_priv->oa_pmu.oa_buffer.format_size = snapshot_size;
+ } else {
+ BUG(); /* pmu shouldn't have been registered */
+ return -ENODEV;
+ }
+
+ /* Since we are limited to an exponential scale for
+ * programming the OA sampling period we don't allow userspace
+ * to pass a precise attr.sample_period. */
+ if (event->attr.freq ||
+ (event->attr.sample_period != 0 &&
+ event->attr.sample_period != 1))
+ return -EINVAL;
+
+ dev_priv->oa_pmu.periodic = event->attr.sample_period;
+
+ /* Instead of allowing userspace to configure the period via
+ * attr.sample_period we instead accept an exponent whereby
+ * the sample_period will be:
+ *
+ * 80ns * 2^(period_exponent + 1)
+ *
+ * Programming a period of 160 nanoseconds would not be very
+ * polite, so higher frequencies are reserved for root.
+ */
+ if (dev_priv->oa_pmu.periodic) {
+ u64 period_exponent = oa_attr.timer_exponent;
+
+ if (period_exponent > 63)
+ return -EINVAL;
+
+ if (period_exponent < 15 && !capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ dev_priv->oa_pmu.period_exponent = period_exponent;
+ } else if (oa_attr.timer_exponent)
+ return -EINVAL;
+
+ /* We bypass the default perf core perf_paranoid_cpu() ||
+ * CAP_SYS_ADMIN check by using the PERF_PMU_CAP_IS_DEVICE
+ * flag and instead authenticate based on whether the current
+ * pid owns the specified context, or require CAP_SYS_ADMIN
+ * when collecting cross-context metrics.
+ */
+ dev_priv->oa_pmu.specific_ctx = NULL;
+ if (oa_attr.single_context) {
+ u32 ctx_id = oa_attr.ctx_id;
+ unsigned int drm_fd = oa_attr.drm_fd;
+ struct fd fd = fdget(drm_fd);
+
+ if (!fd.file)
+ return -EBADF;
+
+ dev_priv->oa_pmu.specific_ctx =
+ lookup_context(dev_priv, fd.file, ctx_id);
+ fdput(fd);
+
+ if (!dev_priv->oa_pmu.specific_ctx)
+ return -EINVAL;
+ }
+
+ if (!dev_priv->oa_pmu.specific_ctx && !capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ ret = init_oa_buffer(event);
+ if (ret)
+ return ret;
+
+ BUG_ON(dev_priv->oa_pmu.exclusive_event);
+ dev_priv->oa_pmu.exclusive_event = event;
+
+ event->destroy = i915_oa_event_destroy;
+
+ /* PRM - observability performance counters:
+ *
+ * OACONTROL, performance counter enable, note:
+ *
+ * "When this bit is set, in order to have coherent counts,
+ * RC6 power state and trunk clock gating must be disabled.
+ * This can be achieved by programming MMIO registers as
+ * 0xA094=0 and 0xA090[31]=1"
+ *
+ * In our case we are expected that taking pm + FORCEWAKE
+ * references will effectively disable RC6 and trunk clock
+ * gating.
+ */
+ intel_runtime_pm_get(dev_priv);
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+ return 0;
+}
+
+static void update_oacontrol(struct drm_i915_private *dev_priv)
+{
+ BUG_ON(!spin_is_locked(&dev_priv->oa_pmu.lock));
+
+ if (dev_priv->oa_pmu.event_active) {
+ unsigned long ctx_id = 0;
+ bool pinning_ok = false;
+
+ if (dev_priv->oa_pmu.specific_ctx) {
+ struct intel_context *ctx =
+ dev_priv->oa_pmu.specific_ctx;
+ struct drm_i915_gem_object *obj =
+ ctx->legacy_hw_ctx.rcs_state;
+
+ if (i915_gem_obj_is_pinned(obj)) {
+ ctx_id = i915_gem_obj_ggtt_offset(obj);
+ pinning_ok = true;
+ }
+ }
+
+ if ((ctx_id == 0 || pinning_ok)) {
+ bool periodic = dev_priv->oa_pmu.periodic;
+ u32 period_exponent = dev_priv->oa_pmu.period_exponent;
+ u32 report_format = dev_priv->oa_pmu.oa_buffer.format;
+
+ I915_WRITE(GEN7_OACONTROL,
+ (ctx_id & GEN7_OACONTROL_CTX_MASK) |
+ (period_exponent <<
+ GEN7_OACONTROL_TIMER_PERIOD_SHIFT) |
+ (periodic ?
+ GEN7_OACONTROL_TIMER_ENABLE : 0) |
+ (report_format <<
+ GEN7_OACONTROL_FORMAT_SHIFT) |
+ (ctx_id ?
+ GEN7_OACONTROL_PER_CTX_ENABLE : 0) |
+ GEN7_OACONTROL_ENABLE);
+ return;
+ }
+ }
+
+ I915_WRITE(GEN7_OACONTROL, 0);
+}
+
+static void i915_oa_event_start(struct perf_event *event, int flags)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu);
+ unsigned long lock_flags;
+ u32 oastatus1, tail;
+
+ /* PRM - observability performance counters:
+ *
+ * OACONTROL, specific context enable:
+ *
+ * "OA unit level clock gating must be ENABLED when using
+ * specific ContextID feature."
+ *
+ * Assuming we don't ever disable OA unit level clock gating
+ * lets just assert that this condition is met...
+ */
+ WARN_ONCE(I915_READ(GEN6_UCGCTL3) & GEN6_OACSUNIT_CLOCK_GATE_DISABLE,
+ "disabled OA unit level clock gating will result in incorrect per-context OA counters");
+
+ /* XXX: On Haswell, when threshold disable mode is desired,
+ * instead of setting the threshold enable to '0', we need to
+ * program it to '1' and set OASTARTTRIG1 bits 15:0 to 0
+ * (threshold value of 0)
+ */
+ I915_WRITE(OASTARTTRIG6, (OASTARTTRIG6_B4_TO_B7_THRESHOLD_ENABLE |
+ OASTARTTRIG6_B4_CUSTOM_EVENT_ENABLE));
+ I915_WRITE(OASTARTTRIG5, 0); /* threshold value */
+
+ I915_WRITE(OASTARTTRIG2, (OASTARTTRIG2_B0_TO_B3_THRESHOLD_ENABLE |
+ OASTARTTRIG2_B0_CUSTOM_EVENT_ENABLE));
+ I915_WRITE(OASTARTTRIG1, 0); /* threshold value */
+
+ /* Setup B0 as the gpu clock counter... */
+ I915_WRITE(OACEC0_0, OACEC0_0_B0_COMPARE_GREATER_OR_EQUAL); /* to 0 */
+ I915_WRITE(OACEC0_1, 0xfffe); /* Select NOA[0] */
+
+ spin_lock_irqsave(&dev_priv->oa_pmu.lock, lock_flags);
+
+ dev_priv->oa_pmu.event_active = true;
+ update_oacontrol(dev_priv);
+
+ /* Reset the head ptr to ensure we don't forward reports relating
+ * to a previous perf event */
+ oastatus1 = I915_READ(GEN7_OASTATUS1);
+ tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
+ I915_WRITE(GEN7_OASTATUS2, (tail & GEN7_OASTATUS2_HEAD_MASK) |
+ GEN7_OASTATUS2_GGTT);
+
+ mmiowb();
+ spin_unlock_irqrestore(&dev_priv->oa_pmu.lock, lock_flags);
+
+ if (event->attr.sample_period)
+ __hrtimer_start_range_ns(&dev_priv->oa_pmu.timer,
+ ns_to_ktime(PERIOD), 0,
+ HRTIMER_MODE_REL_PINNED, 0);
+
+ event->hw.state = 0;
+}
+
+static void i915_oa_event_stop(struct perf_event *event, int flags)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu);
+ unsigned long lock_flags;
+
+ spin_lock_irqsave(&dev_priv->oa_pmu.lock, lock_flags);
+
+ dev_priv->oa_pmu.event_active = false;
+ update_oacontrol(dev_priv);
+
+ mmiowb();
+ spin_unlock_irqrestore(&dev_priv->oa_pmu.lock, lock_flags);
+
+ if (event->attr.sample_period) {
+ hrtimer_cancel(&dev_priv->oa_pmu.timer);
+ flush_oa_snapshots(dev_priv, false);
+ }
+
+ event->hw.state = PERF_HES_STOPPED;
+}
+
+static int i915_oa_event_add(struct perf_event *event, int flags)
+{
+ if (flags & PERF_EF_START)
+ i915_oa_event_start(event, flags);
+
+ return 0;
+}
+
+static void i915_oa_event_del(struct perf_event *event, int flags)
+{
+ i915_oa_event_stop(event, flags);
+}
+
+static void i915_oa_event_read(struct perf_event *event)
+{
+ struct drm_i915_private *i915 =
+ container_of(event->pmu, typeof(*i915), oa_pmu.pmu);
+
+ /* XXX: What counter would be useful here? */
+ local64_set(&event->count, 0);
+}
+
+static int i915_oa_event_flush(struct perf_event *event)
+{
+ if (event->attr.sample_period) {
+ struct drm_i915_private *i915 =
+ container_of(event->pmu, typeof(*i915), oa_pmu.pmu);
+
+ flush_oa_snapshots(i915, true);
+ }
+
+ return 0;
+}
+
+static int i915_oa_event_event_idx(struct perf_event *event)
+{
+ return 0;
+}
+
+void i915_oa_context_pin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context)
+{
+ unsigned long flags;
+
+ if (dev_priv->oa_pmu.pmu.event_init == NULL)
+ return;
+
+ spin_lock_irqsave(&dev_priv->oa_pmu.lock, flags);
+
+ if (dev_priv->oa_pmu.specific_ctx == context)
+ update_oacontrol(dev_priv);
+
+ mmiowb();
+ spin_unlock_irqrestore(&dev_priv->oa_pmu.lock, flags);
+}
+
+void i915_oa_context_unpin_notify(struct drm_i915_private *dev_priv,
+ struct intel_context *context)
+{
+ unsigned long flags;
+
+ if (dev_priv->oa_pmu.pmu.event_init == NULL)
+ return;
+
+ spin_lock_irqsave(&dev_priv->oa_pmu.lock, flags);
+
+ if (dev_priv->oa_pmu.specific_ctx == context)
+ update_oacontrol(dev_priv);
+
+ mmiowb();
+ spin_unlock_irqrestore(&dev_priv->oa_pmu.lock, flags);
+}
+
+void i915_oa_pmu_register(struct drm_device *dev)
+{
+ struct drm_i915_private *i915 = to_i915(dev);
+
+ if (!IS_HASWELL(dev))
+ return;
+
+ /* We need to be careful about forwarding cpu metrics to
+ * userspace considering that PERF_PMU_CAP_IS_DEVICE bypasses
+ * the events/core security check that stops an unprivileged
+ * process collecting metrics for other processes.
+ */
+ i915->oa_pmu.dummy_regs = *task_pt_regs(current);
+
+ hrtimer_init(&i915->oa_pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ i915->oa_pmu.timer.function = hrtimer_sample;
+
+ spin_lock_init(&i915->oa_pmu.lock);
+
+ i915->oa_pmu.pmu.capabilities = PERF_PMU_CAP_IS_DEVICE;
+
+ /* Effectively disallow opening an event with a specific pid
+ * since we aren't interested in processes running on the cpu...
+ */
+ i915->oa_pmu.pmu.task_ctx_nr = perf_invalid_context;
+
+ i915->oa_pmu.pmu.event_init = i915_oa_event_init;
+ i915->oa_pmu.pmu.add = i915_oa_event_add;
+ i915->oa_pmu.pmu.del = i915_oa_event_del;
+ i915->oa_pmu.pmu.start = i915_oa_event_start;
+ i915->oa_pmu.pmu.stop = i915_oa_event_stop;
+ i915->oa_pmu.pmu.read = i915_oa_event_read;
+ i915->oa_pmu.pmu.flush = i915_oa_event_flush;
+ i915->oa_pmu.pmu.event_idx = i915_oa_event_event_idx;
+
+ if (perf_pmu_register(&i915->oa_pmu.pmu, "i915_oa", -1))
+ i915->oa_pmu.pmu.event_init = NULL;
+}
+
+void i915_oa_pmu_unregister(struct drm_device *dev)
+{
+ struct drm_i915_private *i915 = to_i915(dev);
+
+ if (i915->oa_pmu.pmu.event_init == NULL)
+ return;
+
+ perf_pmu_unregister(&i915->oa_pmu.pmu);
+ i915->oa_pmu.pmu.event_init = NULL;
+}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc6907b..40fc44f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -516,6 +516,73 @@
#define GEN7_3DPRIM_BASE_VERTEX 0x2440
#define GEN7_OACONTROL 0x2360
+#define GEN7_OACONTROL_CTX_MASK 0xFFFFF000
+#define GEN7_OACONTROL_TIMER_PERIOD_MASK 0x3F
+#define GEN7_OACONTROL_TIMER_PERIOD_SHIFT 6
+#define GEN7_OACONTROL_TIMER_ENABLE (1<<5)
+#define GEN7_OACONTROL_FORMAT_A13 (0<<2)
+#define GEN7_OACONTROL_FORMAT_A29 (1<<2)
+#define GEN7_OACONTROL_FORMAT_A13_B8_C8 (2<<2)
+#define GEN7_OACONTROL_FORMAT_A29_B8_C8 (3<<2)
+#define GEN7_OACONTROL_FORMAT_B4_C8 (4<<2)
+#define GEN7_OACONTROL_FORMAT_A45_B8_C8 (5<<2)
+#define GEN7_OACONTROL_FORMAT_B4_C8_A16 (6<<2)
+#define GEN7_OACONTROL_FORMAT_C4_B8 (7<<2)
+#define GEN7_OACONTROL_FORMAT_SHIFT 2
+#define GEN7_OACONTROL_PER_CTX_ENABLE (1<<1)
+#define GEN7_OACONTROL_ENABLE (1<<0)
+
+#define OASTARTTRIG5 0x02720
+#define OASTARTTRIG5_THRESHOLD_VALUE_MASK 0xffff
+
+#define OASTARTTRIG6 0x02724
+#define OASTARTTRIG6_B4_TO_B7_THRESHOLD_ENABLE (1<<23)
+#define OASTARTTRIG6_B4_CUSTOM_EVENT_ENABLE (1<<28)
+
+#define OASTARTTRIG1 0x02710
+#define OASTARTTRIG1_THRESHOLD_VALUE_MASK 0xffff
+
+#define OASTARTTRIG2 0x02714
+#define OASTARTTRIG2_B0_TO_B3_THRESHOLD_ENABLE (1<<23)
+#define OASTARTTRIG2_B0_CUSTOM_EVENT_ENABLE (1<<28)
+
+#define OACEC0_0 0x2770
+#define OACEC0_0_B0_COMPARE_ANY_EQUAL 0
+#define OACEC0_0_B0_COMPARE_OR 0
+#define OACEC0_0_B0_COMPARE_GREATER_THAN 1
+#define OACEC0_0_B0_COMPARE_EQUAL 2
+#define OACEC0_0_B0_COMPARE_GREATER_OR_EQUAL 3
+#define OACEC0_0_B0_COMPARE_LESS_THAN 4
+#define OACEC0_0_B0_COMPARE_NOT_EQUAL 5
+#define OACEC0_0_B0_COMPARE_LESS_OR_EQUAL 6
+#define OACEC0_0_B0_COMPARE_VALUE_MASK 0xffff
+#define OACEC0_0_B0_COMPARE_VALUE_SHIFT 3
+
+#define OACEC0_1 0x2774
+
+#define GEN7_OABUFFER 0x23B0 /* R/W */
+#define GEN7_OABUFFER_OVERRUN_DISABLE (1<<3)
+#define GEN7_OABUFFER_EDGE_TRIGGER (1<<2)
+#define GEN7_OABUFFER_STOP_RESUME_ENABLE (1<<1)
+#define GEN7_OABUFFER_RESUME (1<<0)
+
+#define GEN7_OASTATUS1 0x2364
+#define GEN7_OASTATUS1_TAIL_MASK 0xffffffc0
+#define GEN7_OASTATUS1_OABUFFER_SIZE_128K (0<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_256K (1<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_512K (2<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_1M (3<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_2M (4<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_4M (5<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_8M (6<<3)
+#define GEN7_OASTATUS1_OABUFFER_SIZE_16M (7<<3)
+#define GEN7_OASTATUS1_COUNTER_OVERFLOW (1<<2)
+#define GEN7_OASTATUS1_OABUFFER_OVERFLOW (1<<1)
+#define GEN7_OASTATUS1_REPORT_LOST (1<<0)
+
+#define GEN7_OASTATUS2 0x2368
+#define GEN7_OASTATUS2_HEAD_MASK 0xffffffc0
+#define GEN7_OASTATUS2_GGTT 0x1
#define _GEN7_PIPEA_DE_LOAD_SL 0x70068
#define _GEN7_PIPEB_DE_LOAD_SL 0x71068
@@ -6545,6 +6612,7 @@ enum skl_disp_power_wells {
# define GEN6_RCCUNIT_CLOCK_GATE_DISABLE (1 << 11)
#define GEN6_UCGCTL3 0x9408
+# define GEN6_OACSUNIT_CLOCK_GATE_DISABLE (1 << 20)
#define GEN7_UCGCTL4 0x940c
#define GEN7_L3BANK2X_CLOCK_GATE_DISABLE (1<<25)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 4851d66..f78f232 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -58,6 +58,35 @@
#define I915_ERROR_UEVENT "ERROR"
#define I915_RESET_UEVENT "RESET"
+/**
+ * DOC: perf events configuration exposed by i915 through /sys/bus/event_sources/drivers/i915_oa
+ *
+ */
+
+#define I915_OA_FORMAT_A13_HSW 0
+#define I915_OA_FORMAT_A29_HSW 1
+#define I915_OA_FORMAT_A13_B8_C8_HSW 2
+#define I915_OA_FORMAT_B4_C8_HSW 4
+#define I915_OA_FORMAT_A45_B8_C8_HSW 5
+#define I915_OA_FORMAT_B4_C8_A16_HSW 6
+#define I915_OA_FORMAT_C4_B8_HSW 7
+
+#define I915_OA_ATTR_SIZE_VER0 32 /* sizeof first published struct */
+
+typedef struct _drm_i915_oa_attr {
+ __u32 size;
+
+ __u32 format;
+ __u32 metrics_set;
+ __u32 timer_exponent;
+
+ __u32 drm_fd;
+ __u32 ctx_id;
+
+ __u64 single_context : 1,
+ __reserved_1 : 63;
+} drm_i915_oa_attr_t;
+
/* Each region is a minimum of 16k, and there are at most 255 of them.
*/
#define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use
--
2.4.1
^ permalink raw reply related
* Re: [klibc] kernel/libc uapi changes for y2038
From: Thorsten Glaser @ 2015-05-18 17:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: klibc, libc-alpha, y2038, linux-api, musl, linux-kernel,
Rich Felker, cferris, enh, Joseph S. Myers
In-Reply-To: <10173485.f8yUt0lQ60@wuerfel>
fup2p, this is offtopic for most lists
Arnd Bergmann dixit:
>It's hard because we don't even know what ioctls are affected at this point,
>and I was hoping to get this part merged as a stepping stone in the process
>of finding out.
Oh okay.
>e) ioctls that pass a time value as a 'long' or '__u32' instead of
> 'time_t'. Fixing them requires adding new ioctl commands to let
> them work beyond 2038, independent of what we do here.
Yeah, that’s going to be fun.
>MIPS on the other hand is no more broken than any of the other 32-bit
>ABIs, because it does not use 64-bit __kernel_long_t in its n32 ABI.
I have heard from a MIPS porter that one of the flavours suffers
from similar problems as x32, just not to that extent. But I
don’t recall my source…
>ioctls. My plan at this point is to eliminate all uses of time_t in
>the kernel and replace them with time64_t or other safe types.
>This way, we will eventually find all code that passes 32-bit time types
>to user space and can fix it. This will take care of the time_t
>related problems on x32 as well.
Ah, interesting approach. And existing userspace, as well as new
userspace that does not declare 64-bit time_t readiness, is still
safe on currently-not-broken architectures? So, there’s enough time
to fix this before the various libcs turn that on (and it had better
be fixed by then, because it becomes ABI by then). Nice idea.
I am wondering a bit about the ioctls being hard to find. I have
not much experience with kernel programming, and even less with
Linux than with MS-DOS and BSD, but should not each driver have
a central ioctl entry point, from which it should cast the user
space data into a (possibly locally declared) structure?
bye,
//mirabilos
--
<igli> exceptions: a truly awful implementation of quite a nice idea.
<igli> just about the worst way you could do something like that, afaic.
<igli> it's like anti-design. <mirabilos> that too… may I quote you on that?
<igli> sure, tho i doubt anyone will listen ;)
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [RFC PATCH 07/11] drm/i915: Expose PMU for Observation Architecture
From: Robert Bragg @ 2015-05-18 16:36 UTC (permalink / raw)
To: dri-devel, David Airlie, Daniel Vetter, Ingo Molnar,
Peter Zijlstra, Paul Mackerras, Chris Wilson,
Arnaldo Carvalho de Melo, intel-gfx, linux-api, linux-kernel
In-Reply-To: <20150507145800.GZ22099@nuc-i3427.alporthouse.com>
On 7 May 2015 15:58, "Chris Wilson" <chris@chris-wilson.co.uk> wrote:
>
> On Thu, May 07, 2015 at 03:15:50PM +0100, Robert Bragg wrote:
> > + /* We bypass the default perf core perf_paranoid_cpu() ||
> > + * CAP_SYS_ADMIN check by using the PERF_PMU_CAP_IS_DEVICE
> > + * flag and instead authenticate based on whether the current
> > + * pid owns the specified context, or require CAP_SYS_ADMIN
> > + * when collecting cross-context metrics.
> > + */
> > + dev_priv->oa_pmu.specific_ctx = NULL;
> > + if (oa_attr.single_context) {
> > + u32 ctx_id = oa_attr.ctx_id;
> > + unsigned int drm_fd = oa_attr.drm_fd;
> > + struct fd fd = fdget(drm_fd);
> > +
> > + if (fd.file) {
>
> Specify a ctx and not providing the right fd should be its own error,
> either EBADF or EINVAL.
Right, I went for both in the end; EBADF if fdget fails and EINVAL if
the fd is ok but we fail to lookup a context with it.
>
> > + dev_priv->oa_pmu.specific_ctx =
> > + lookup_context(dev_priv, fd.file, ctx_id);
> > + }
>
> Missing fdput
Ah yes; fixed.
>
> > + }
> > +
> > + if (!dev_priv->oa_pmu.specific_ctx && !capable(CAP_SYS_ADMIN))
> > + return -EACCES;
> > +
> > + mutex_lock(&dev_priv->dev->struct_mutex);
>
> i915_mutex_interruptible, probably best to couple into the GPU error
> handling here as well especially as init_oa_buffer() will go onto touch
> GPU internals.
Ok, using i915_mutex_interruptible makes sense, I've also moved the
locking into init_oa_buffer.
About the GPU error handling, do you have any thoughts on what could
be most helpful here? I'm thinking a.t.m of extending
i915_capture_reg_state() in i915_gpu_error.c to capture the OACONTROL
+ OASTATUS state and perhaps all the UCGCTL unit clock gating state
too.
>
> > + ret = init_oa_buffer(event);
> > + mutex_unlock(&dev_priv->dev->struct_mutex);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + BUG_ON(dev_priv->oa_pmu.exclusive_event);
> > + dev_priv->oa_pmu.exclusive_event = event;
> > +
> > + event->destroy = i915_oa_event_destroy;
> > +
> > + /* PRM - observability performance counters:
> > + *
> > + * OACONTROL, performance counter enable, note:
> > + *
> > + * "When this bit is set, in order to have coherent counts,
> > + * RC6 power state and trunk clock gating must be disabled.
> > + * This can be achieved by programming MMIO registers as
> > + * 0xA094=0 and 0xA090[31]=1"
> > + *
> > + * In our case we are expected that taking pm + FORCEWAKE
> > + * references will effectively disable RC6 and trunk clock
> > + * gating.
> > + */
> > + intel_runtime_pm_get(dev_priv);
> > + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> That is a nuisance. Aside: Why isn't OA inside the powerctx? Is a subset
> valid with forcewake? It does perturb the system greatly to disable rc6,
> so I wonder if it could be made optional?
Yes, it's a shame.
I probably only really know enough about the OA unit design to be
dangerous and won't try and comment in detail here, but I think
there's more to it than not saving state in a power context. As I
understand it, there were a number of design changes made to enable
OA+RC6 support for BDW+, including having the OA unit automatically
write out reports to the OA buffer when entering RC6.
I think just FORCEWAKE_RENDER would work here, but only say that
because it looks like HSW only has the render forcewake domain from
what I could tell.
I think I need to update the comment above these lines as I don't
think these will affect crclk gating; these just handle disabling RC6.
The WIP patch I sent out basically represents me trying to get to the
bottom of the clock gating constraints we have.
At this point I think I need to disable CS unit gating via UCGCTL1, as
well as DOP gating for the render trunk clock via MISCCPCTL but I'm
not entirely confident about that just yet. At least empirically I see
these fixing some issues in rudimentary testing.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void update_oacontrol(struct drm_i915_private *dev_priv)
> > +{
> > + BUG_ON(!spin_is_locked(&dev_priv->oa_pmu.lock));
> > +
> > + if (dev_priv->oa_pmu.event_active) {
> > + unsigned long ctx_id = 0;
> > + bool pinning_ok = false;
> > +
> > + if (dev_priv->oa_pmu.specific_ctx) {
> > + struct intel_context *ctx =
> > + dev_priv->oa_pmu.specific_ctx;
> > + struct drm_i915_gem_object *obj =
> > + ctx->legacy_hw_ctx.rcs_state;
>
> If only there was ctx->legacy_hw_ctx.rcs_vma...
ok, not sure if this is a prod to add that, a heads up that this is
coming or seething because some prior attempt to add this was nack'd.
>
> > +
> > + if (i915_gem_obj_is_pinned(obj)) {
> > + ctx_id = i915_gem_obj_ggtt_offset(obj);
> > + pinning_ok = true;
> > + }
> > + }
> > +
> > + if ((ctx_id == 0 || pinning_ok)) {
> > + bool periodic = dev_priv->oa_pmu.periodic;
> > + u32 period_exponent = dev_priv->oa_pmu.period_exponent;
> > + u32 report_format = dev_priv->oa_pmu.oa_buffer.format;
> > +
> > + I915_WRITE(GEN7_OACONTROL,
> > + (ctx_id & GEN7_OACONTROL_CTX_MASK) |
> > + (period_exponent <<
> > + GEN7_OACONTROL_TIMER_PERIOD_SHIFT) |
> > + (periodic ?
> > + GEN7_OACONTROL_TIMER_ENABLE : 0) |
> > + (report_format <<
> > + GEN7_OACONTROL_FORMAT_SHIFT) |
> > + (ctx_id ?
> > + GEN7_OACONTROL_PER_CTX_ENABLE : 0) |
> > + GEN7_OACONTROL_ENABLE);
>
> I notice you don't use any write barriers...
ok, so I still haven't put write barriers within update_oacontrol()
itself, but I've now added mmiowb()s just before unlocking which is
done outside of the update_oacontrol(). I think a barrier just within
update_oacontrol() could be ok a.t.m while the pinning hooks currently
just use update_oacontol(), but in case we might introduce more
overlapping mmio configuration within these critical sections, waiting
until the unlock might be preferable. On the other hand, a.t.m the
pinning callbacks now have redundant wb()s while there is no specific
context filtering - not sure if that should be a concern.
Looking at this, I also didn't feel happy with the way I reset
oa_pmu->specific_context when destroying an event, considering that
->specific_context being set is what determines whether the pinning
callbacks may call update_oacontrol() asynchronously with respect to
the pmu methods. Although we know oacontrol will be disabled by the
time we come to destroy an event, it didn't seem great that that we
could be continuing to run update_oacontrol() up to the point where we
are resetting all the clock gating, power management and NOA enable
state. I'll attach a separate patch to see if you agree this is worth
changing.
Thanks for the comments.
- Robert
> -Chris
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [RFC PATCH 07/11] drm/i915: Expose PMU for Observation Architecture
From: Robert Bragg @ 2015-05-18 16:21 UTC (permalink / raw)
To: dri-devel, David Airlie, Daniel Vetter, Ingo Molnar,
Peter Zijlstra, Paul Mackerras, Chris Wilson,
Arnaldo Carvalho de Melo, intel-gfx, linux-api, linux-kernel
In-Reply-To: <20150507143639.GY22099@nuc-i3427.alporthouse.com>
[-- Attachment #1.1: Type: text/plain, Size: 3385 bytes --]
On 7 May 2015 15:37, "Chris Wilson" <chris@chris-wilson.co.uk> wrote:
>
> On Thu, May 07, 2015 at 03:15:50PM +0100, Robert Bragg wrote:
> > +static int init_oa_buffer(struct perf_event *event)
> > +{
> > + struct drm_i915_private *dev_priv =
> > + container_of(event->pmu, typeof(*dev_priv), oa_pmu.pmu);
> > + struct drm_i915_gem_object *bo;
> > + int ret;
> > +
> > + BUG_ON(!IS_HASWELL(dev_priv->dev));
> > + BUG_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
> > + BUG_ON(dev_priv->oa_pmu.oa_buffer.obj);
> > +
> > + spin_lock_init(&dev_priv->oa_pmu.oa_buffer.flush_lock);
> > +
> > + /* NB: We over allocate the OA buffer due to the way raw sample
data
> > + * gets copied from the gpu mapped circular buffer into the perf
> > + * circular buffer so that only one copy is required.
> > + *
> > + * For each perf sample (raw->size + 4) needs to be 8 byte
aligned,
> > + * where the 4 corresponds to the 32bit raw->size member that's
> > + * added to the sample header that userspace sees.
> > + *
> > + * Due to the + 4 for the size member: when we copy a report to
the
> > + * userspace facing perf buffer we always copy an additional 4
bytes
> > + * from the subsequent report to make up for the miss alignment,
but
> > + * when a report is at the end of the gpu mapped buffer we need to
> > + * read 4 bytes past the end of the buffer.
> > + */
> > + bo = i915_gem_alloc_object(dev_priv->dev, OA_BUFFER_SIZE +
PAGE_SIZE);
> > + if (bo == NULL) {
> > + DRM_ERROR("Failed to allocate OA buffer\n");
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > + dev_priv->oa_pmu.oa_buffer.obj = bo;
> > +
> > + ret = i915_gem_object_set_cache_level(bo, I915_CACHE_LLC);
> > + if (ret)
> > + goto err_unref;
> > +
> > + /* PreHSW required 512K alignment, HSW requires 16M */
> > + ret = i915_gem_obj_ggtt_pin(bo, SZ_16M, 0);
> > + if (ret)
> > + goto err_unref;
> > +
> > + dev_priv->oa_pmu.oa_buffer.gtt_offset =
i915_gem_obj_ggtt_offset(bo);
> > + dev_priv->oa_pmu.oa_buffer.addr = vmap_oa_buffer(bo);
>
> You can look forward to both i915_gem_object_create_internal() and
> i915_gem_object_pin_vmap()
Okey, will do, thanks.
>
> > +
> > + /* Pre-DevBDW: OABUFFER must be set with counters off,
> > + * before OASTATUS1, but after OASTATUS2 */
> > + I915_WRITE(GEN7_OASTATUS2, dev_priv->oa_pmu.oa_buffer.gtt_offset |
> > + GEN7_OASTATUS2_GGTT); /* head */
> > + I915_WRITE(GEN7_OABUFFER, dev_priv->oa_pmu.oa_buffer.gtt_offset);
> > + I915_WRITE(GEN7_OASTATUS1, dev_priv->oa_pmu.oa_buffer.gtt_offset |
> > + GEN7_OASTATUS1_OABUFFER_SIZE_16M); /* tail */
> > +
> > + DRM_DEBUG_DRIVER("OA Buffer initialized, gtt offset = 0x%x, vaddr
= %p",
> > + dev_priv->oa_pmu.oa_buffer.gtt_offset,
> > + dev_priv->oa_pmu.oa_buffer.addr);
> > +
> > + return 0;
> > +
> > +err_unref:
> > + drm_gem_object_unreference_unlocked(&bo->base);
>
> But what I really what to say was:
> mutex deadlock^^^
Yikes, I've pushed an updated patch addressing this and can reply with a
new patch here in a bit.
Thanks,
- Robert
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
[-- Attachment #1.2: Type: text/html, Size: 4690 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ 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