Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems
On 29/03/17 10:22 PM, Christian König wrote:
> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier:
>> Signed-off-by: Jan Burgmeier <jan.burgme...@unicon-software.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 99424cb8020b..583d22974e14 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>> *adev,
>> struct amdgpu_bo *aobj = NULL;
>> uint64_t offset;
>> uint8_t *kptr;
>> +uint64_t it_last;
>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start,
>> );
>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>> *adev,
>> return -EINVAL;
>> }
>> +it_last = m->it.last;
>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
>> -(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>> +(it_last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>
> Nice catch, but just adding a u64 case should do here as well. E.g:
>
> if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
That won't work correctly if m->it.last == 0x ? Or is that not
possible?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems
On 29/03/17 10:22 PM, Christian König wrote:
> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier:
>> Signed-off-by: Jan Burgmeier
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 99424cb8020b..583d22974e14 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>> *adev,
>> struct amdgpu_bo *aobj = NULL;
>> uint64_t offset;
>> uint8_t *kptr;
>> +uint64_t it_last;
>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start,
>> );
>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>> *adev,
>> return -EINVAL;
>> }
>> +it_last = m->it.last;
>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
>> -(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>> +(it_last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>
> Nice catch, but just adding a u64 case should do here as well. E.g:
>
> if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
That won't work correctly if m->it.last == 0x ? Or is that not
possible?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH] drm/fb-helper: Allow var->x/yres(_virtual) < fb->width/height again
From: Michel Dänzer <michel.daen...@amd.com>
Otherwise this can also prevent modesets e.g. for switching VTs, when
multiple monitors with different native resolutions are connected.
The depths must match though, so keep the != test for that.
Also update the DRM_DEBUG output to be slightly more accurate, this
doesn't only affect requests from userspace.
Bugzilla: https://bugs.freedesktop.org/99841
Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
drivers/gpu/drm/drm_fb_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..324a688b3f30 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1260,9 +1260,9 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
* to KMS, hence fail if different settings are requested.
*/
if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
- DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
+ var->xres > fb->width || var->yres > fb->height ||
+ var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
+ DRM_DEBUG("fb requested width/height/bpp can't fit in current
fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
var->xres_virtual, var->yres_virtual,
--
2.11.0
[PATCH] drm/fb-helper: Allow var->x/yres(_virtual) < fb->width/height again
From: Michel Dänzer
Otherwise this can also prevent modesets e.g. for switching VTs, when
multiple monitors with different native resolutions are connected.
The depths must match though, so keep the != test for that.
Also update the DRM_DEBUG output to be slightly more accurate, this
doesn't only affect requests from userspace.
Bugzilla: https://bugs.freedesktop.org/99841
Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/drm_fb_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..324a688b3f30 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1260,9 +1260,9 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
* to KMS, hence fail if different settings are requested.
*/
if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
- DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
+ var->xres > fb->width || var->yres > fb->height ||
+ var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
+ DRM_DEBUG("fb requested width/height/bpp can't fit in current
fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
var->xres_virtual, var->yres_virtual,
--
2.11.0
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 17/03/17 08:13 PM, Ville Syrjälä wrote: > On Fri, Mar 17, 2017 at 07:19:27PM +0900, Michel Dänzer wrote: >> On 17/03/17 07:01 PM, Ville Syrjälä wrote: >>> On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: >>>> On 16/03/17 07:09 PM, Ville Syrjälä wrote: >>>>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: >>>>>> From: Michel Dänzer <michel.daen...@amd.com> >>>>>> >>>>>> Otherwise this can also prevent modesets e.g. for switching VTs. >>>>>> >>>>>> FB_MISC_USER_EVENT is set when the request originates from userspace, >>>>>> which is what we're interested in here according to the DRM_DEBUG >>>>>> output. >>>>> >>>>> So why is the kernel allowed to violate this? >>>>> >>>>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' >>>>> check makes some kind of sense. Although I don't see why the virtual >>>>> resolution couldn't be smaller than the fb size. But requiring that the >>>>> visible resolutionn matches the fb size as well definitely seems wrong. >>>> >>>> Agreed on all counts. So, I think what's needed is almost a revert of >>>> 865afb11949e, except for the bits_per_pixel comparison, since we really >>>> can't change that. See diff below. >>> >>> So one semi-crazy idea I had was: >>> 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't >>> implement a modeset in set_par, i guess what we really >>> should do is look at the currently active crtcs and see if the mode on >>> one of them more or less matches what the user is asking for >> >> I don't get the idea. What's the point of comparing the var->* values to >> whatever is currently active in the hardware? > > Not currently active in the hardware, but currently active as far as > fb_helper is concerned. I guess I should say "matches fb_helper's > current crtc configuration" or something. I see, thanks for clarifying. Something like that could work, but it might still be artificially limiting. Thinking through all the ramifications is too involved for me right now. Anyway, I don't think we can do anything fancier than the quasi-revert for 4.10 or even 4.11. >>> I tried to trip this current check by booting with a big screen and >>> later switching to a small screen. For some reason that worked out >>> just fine, >> >> I'd need more details about what exactly you tried. >> >>> so I'm not even sure what kind of values we stuff into xres & co. >> >> It should be the values shown / attempted to set by fbset. > > I meant what does the kernel put there for fbcon etc. I was expecting > that it xres/yres would be the visible resolution of the smallest > screen, and the virtual resolution would be either the same or the > size of the fb perhaps. But that can't be the case or else my experiment > would have produced a failure. So I would have to dump that stuff out to > see what really ends up there. I'd also assume something like what you describe when fbdev emulation is initialized, but I'm not sure there's any code to update any of that stuff when hotplugging displays. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 17/03/17 08:13 PM, Ville Syrjälä wrote: > On Fri, Mar 17, 2017 at 07:19:27PM +0900, Michel Dänzer wrote: >> On 17/03/17 07:01 PM, Ville Syrjälä wrote: >>> On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: >>>> On 16/03/17 07:09 PM, Ville Syrjälä wrote: >>>>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: >>>>>> From: Michel Dänzer >>>>>> >>>>>> Otherwise this can also prevent modesets e.g. for switching VTs. >>>>>> >>>>>> FB_MISC_USER_EVENT is set when the request originates from userspace, >>>>>> which is what we're interested in here according to the DRM_DEBUG >>>>>> output. >>>>> >>>>> So why is the kernel allowed to violate this? >>>>> >>>>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' >>>>> check makes some kind of sense. Although I don't see why the virtual >>>>> resolution couldn't be smaller than the fb size. But requiring that the >>>>> visible resolutionn matches the fb size as well definitely seems wrong. >>>> >>>> Agreed on all counts. So, I think what's needed is almost a revert of >>>> 865afb11949e, except for the bits_per_pixel comparison, since we really >>>> can't change that. See diff below. >>> >>> So one semi-crazy idea I had was: >>> 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't >>> implement a modeset in set_par, i guess what we really >>> should do is look at the currently active crtcs and see if the mode on >>> one of them more or less matches what the user is asking for >> >> I don't get the idea. What's the point of comparing the var->* values to >> whatever is currently active in the hardware? > > Not currently active in the hardware, but currently active as far as > fb_helper is concerned. I guess I should say "matches fb_helper's > current crtc configuration" or something. I see, thanks for clarifying. Something like that could work, but it might still be artificially limiting. Thinking through all the ramifications is too involved for me right now. Anyway, I don't think we can do anything fancier than the quasi-revert for 4.10 or even 4.11. >>> I tried to trip this current check by booting with a big screen and >>> later switching to a small screen. For some reason that worked out >>> just fine, >> >> I'd need more details about what exactly you tried. >> >>> so I'm not even sure what kind of values we stuff into xres & co. >> >> It should be the values shown / attempted to set by fbset. > > I meant what does the kernel put there for fbcon etc. I was expecting > that it xres/yres would be the visible resolution of the smallest > screen, and the virtual resolution would be either the same or the > size of the fb perhaps. But that can't be the case or else my experiment > would have produced a failure. So I would have to dump that stuff out to > see what really ends up there. I'd also assume something like what you describe when fbdev emulation is initialized, but I'm not sure there's any code to update any of that stuff when hotplugging displays. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 17/03/17 07:01 PM, Ville Syrjälä wrote: > On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: >> On 16/03/17 07:09 PM, Ville Syrjälä wrote: >>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: >>>> From: Michel Dänzer <michel.daen...@amd.com> >>>> >>>> Otherwise this can also prevent modesets e.g. for switching VTs. >>>> >>>> FB_MISC_USER_EVENT is set when the request originates from userspace, >>>> which is what we're interested in here according to the DRM_DEBUG >>>> output. >>> >>> So why is the kernel allowed to violate this? >>> >>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' >>> check makes some kind of sense. Although I don't see why the virtual >>> resolution couldn't be smaller than the fb size. But requiring that the >>> visible resolutionn matches the fb size as well definitely seems wrong. >> >> Agreed on all counts. So, I think what's needed is almost a revert of >> 865afb11949e, except for the bits_per_pixel comparison, since we really >> can't change that. See diff below. > > So one semi-crazy idea I had was: > 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't > implement a modeset in set_par, i guess what we really > should do is look at the currently active crtcs and see if the mode on > one of them more or less matches what the user is asking for I don't get the idea. What's the point of comparing the var->* values to whatever is currently active in the hardware? The purpose of the code is to check that the requested parameters fit into fb_helper's framebuffer. > I tried to trip this current check by booting with a big screen and > later switching to a small screen. For some reason that worked out > just fine, I'd need more details about what exactly you tried. > so I'm not even sure what kind of values we stuff into xres & co. It should be the values shown / attempted to set by fbset. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 17/03/17 07:01 PM, Ville Syrjälä wrote: > On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: >> On 16/03/17 07:09 PM, Ville Syrjälä wrote: >>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: >>>> From: Michel Dänzer >>>> >>>> Otherwise this can also prevent modesets e.g. for switching VTs. >>>> >>>> FB_MISC_USER_EVENT is set when the request originates from userspace, >>>> which is what we're interested in here according to the DRM_DEBUG >>>> output. >>> >>> So why is the kernel allowed to violate this? >>> >>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' >>> check makes some kind of sense. Although I don't see why the virtual >>> resolution couldn't be smaller than the fb size. But requiring that the >>> visible resolutionn matches the fb size as well definitely seems wrong. >> >> Agreed on all counts. So, I think what's needed is almost a revert of >> 865afb11949e, except for the bits_per_pixel comparison, since we really >> can't change that. See diff below. > > So one semi-crazy idea I had was: > 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't > implement a modeset in set_par, i guess what we really > should do is look at the currently active crtcs and see if the mode on > one of them more or less matches what the user is asking for I don't get the idea. What's the point of comparing the var->* values to whatever is currently active in the hardware? The purpose of the code is to check that the requested parameters fit into fb_helper's framebuffer. > I tried to trip this current check by booting with a big screen and > later switching to a small screen. For some reason that worked out > just fine, I'd need more details about what exactly you tried. > so I'm not even sure what kind of values we stuff into xres & co. It should be the values shown / attempted to set by fbset. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 16/03/17 07:09 PM, Ville Syrjälä wrote:
> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daen...@amd.com>
>>
>> Otherwise this can also prevent modesets e.g. for switching VTs.
>>
>> FB_MISC_USER_EVENT is set when the request originates from userspace,
>> which is what we're interested in here according to the DRM_DEBUG
>> output.
>
> So why is the kernel allowed to violate this?
>
> The checks look somewhat bogus to me anyway. The 'virtual size == fb size'
> check makes some kind of sense. Although I don't see why the virtual
> resolution couldn't be smaller than the fb size. But requiring that the
> visible resolutionn matches the fb size as well definitely seems wrong.
Agreed on all counts. So, I think what's needed is almost a revert of
865afb11949e, except for the bits_per_pixel comparison, since we really
can't change that. See diff below.
Does that make sense? Stefan, would this break any test case you wrote
your patch for?
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..f4f70ce24fcc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1260,8 +1260,8 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
* to KMS, hence fail if different settings are requested.
*/
if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
+ var->xres > fb->width || var->yres > fb->height ||
+ var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
>> Bugzilla: https://bugs.freedesktop.org/99841
>> Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
>> ---
>>
>> I'm not entirely sure why the values can not match for a VT switch. If
>> anybody thinks this just papers over the real issue, please speak up.
>>
>> drivers/gpu/drm/drm_fb_helper.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index f6d4d9700734..9663f3b8f287 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1259,9 +1259,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo
>> *var,
>> * Changes struct fb_var_screeninfo are currently not pushed back
>> * to KMS, hence fail if different settings are requested.
>> */
>> -if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
>> -var->xres != fb->width || var->yres != fb->height ||
>> -var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
>> +if ((info->flags & FBINFO_MISC_USEREVENT) &&
>> +(var->bits_per_pixel != fb->format->cpp[0] * 8 ||
>> + var->xres != fb->width || var->yres != fb->height ||
>> + var->xres_virtual != fb->width || var->yres_virtual !=
>> fb->height)) {
>> DRM_DEBUG("fb userspace requested width/height/bpp different
>> than current fb "
>>"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
>>var->xres, var->yres, var->bits_per_pixel,
>> --
>> 2.11.0
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 16/03/17 07:09 PM, Ville Syrjälä wrote:
> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote:
>> From: Michel Dänzer
>>
>> Otherwise this can also prevent modesets e.g. for switching VTs.
>>
>> FB_MISC_USER_EVENT is set when the request originates from userspace,
>> which is what we're interested in here according to the DRM_DEBUG
>> output.
>
> So why is the kernel allowed to violate this?
>
> The checks look somewhat bogus to me anyway. The 'virtual size == fb size'
> check makes some kind of sense. Although I don't see why the virtual
> resolution couldn't be smaller than the fb size. But requiring that the
> visible resolutionn matches the fb size as well definitely seems wrong.
Agreed on all counts. So, I think what's needed is almost a revert of
865afb11949e, except for the bits_per_pixel comparison, since we really
can't change that. See diff below.
Does that make sense? Stefan, would this break any test case you wrote
your patch for?
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..f4f70ce24fcc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1260,8 +1260,8 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
* to KMS, hence fail if different settings are requested.
*/
if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
+ var->xres > fb->width || var->yres > fb->height ||
+ var->xres_virtual > fb->width || var->yres_virtual > fb->height) {
DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
>> Bugzilla: https://bugs.freedesktop.org/99841
>> Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
>> Signed-off-by: Michel Dänzer
>> ---
>>
>> I'm not entirely sure why the values can not match for a VT switch. If
>> anybody thinks this just papers over the real issue, please speak up.
>>
>> drivers/gpu/drm/drm_fb_helper.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index f6d4d9700734..9663f3b8f287 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1259,9 +1259,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo
>> *var,
>> * Changes struct fb_var_screeninfo are currently not pushed back
>> * to KMS, hence fail if different settings are requested.
>> */
>> -if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
>> -var->xres != fb->width || var->yres != fb->height ||
>> -var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
>> +if ((info->flags & FBINFO_MISC_USEREVENT) &&
>> +(var->bits_per_pixel != fb->format->cpp[0] * 8 ||
>> + var->xres != fb->width || var->yres != fb->height ||
>> + var->xres_virtual != fb->width || var->yres_virtual !=
>> fb->height)) {
>> DRM_DEBUG("fb userspace requested width/height/bpp different
>> than current fb "
>>"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
>>var->xres, var->yres, var->bits_per_pixel,
>> --
>> 2.11.0
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 16/03/17 07:09 PM, Daniel Stone wrote:
> On 16 March 2017 at 09:55, Michel Dänzer <mic...@daenzer.net> wrote:
>> Otherwise this can also prevent modesets e.g. for switching VTs.
>>
>> FB_MISC_USER_EVENT is set when the request originates from userspace,
>> which is what we're interested in here according to the DRM_DEBUG
>> output.
>>
>> Bugzilla: https://bugs.freedesktop.org/99841
>> Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
>> ---
>>
>> I'm not entirely sure why the values can not match for a VT switch. If
>> anybody thinks this just papers over the real issue, please speak up.
>
> It happens for me in multi-head with different resolutions. A real
> compositor will set native resolutions with separate framebuffers, and
> then fbcon will try to set one buffer for both outputs. This works on
> the output with the larger resolution, but the one with the smaller
> resolution will fail due to [xy]res_virtual (IIRC) being different.
That's more or less the line of thinking that lead me to writing this
patch, based on the assumption that the fb->* values correspond to what
was set by whatever we're VT switching from. However, it occurred to me
that it's an invalid assumption; fb here is always fb_helper's
framebuffer for fbdev. I think Ville is right that the tests are bogus
in the first place.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
On 16/03/17 07:09 PM, Daniel Stone wrote:
> On 16 March 2017 at 09:55, Michel Dänzer wrote:
>> Otherwise this can also prevent modesets e.g. for switching VTs.
>>
>> FB_MISC_USER_EVENT is set when the request originates from userspace,
>> which is what we're interested in here according to the DRM_DEBUG
>> output.
>>
>> Bugzilla: https://bugs.freedesktop.org/99841
>> Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
>> Signed-off-by: Michel Dänzer
>> ---
>>
>> I'm not entirely sure why the values can not match for a VT switch. If
>> anybody thinks this just papers over the real issue, please speak up.
>
> It happens for me in multi-head with different resolutions. A real
> compositor will set native resolutions with separate framebuffers, and
> then fbcon will try to set one buffer for both outputs. This works on
> the output with the larger resolution, but the one with the smaller
> resolution will fail due to [xy]res_virtual (IIRC) being different.
That's more or less the line of thinking that lead me to writing this
patch, based on the assumption that the fb->* values correspond to what
was set by whatever we're VT switching from. However, it occurred to me
that it's an invalid assumption; fb here is always fb_helper's
framebuffer for fbdev. I think Ville is right that the tests are bogus
in the first place.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
From: Michel Dänzer <michel.daen...@amd.com>
Otherwise this can also prevent modesets e.g. for switching VTs.
FB_MISC_USER_EVENT is set when the request originates from userspace,
which is what we're interested in here according to the DRM_DEBUG
output.
Bugzilla: https://bugs.freedesktop.org/99841
Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
I'm not entirely sure why the values can not match for a VT switch. If
anybody thinks this just papers over the real issue, please speak up.
drivers/gpu/drm/drm_fb_helper.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..9663f3b8f287 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1259,9 +1259,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo
*var,
* Changes struct fb_var_screeninfo are currently not pushed back
* to KMS, hence fail if different settings are requested.
*/
- if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
+ if ((info->flags & FBINFO_MISC_USEREVENT) &&
+ (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
+var->xres != fb->width || var->yres != fb->height ||
+var->xres_virtual != fb->width || var->yres_virtual !=
fb->height)) {
DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
--
2.11.0
[PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set
From: Michel Dänzer
Otherwise this can also prevent modesets e.g. for switching VTs.
FB_MISC_USER_EVENT is set when the request originates from userspace,
which is what we're interested in here according to the DRM_DEBUG
output.
Bugzilla: https://bugs.freedesktop.org/99841
Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev")
Signed-off-by: Michel Dänzer
---
I'm not entirely sure why the values can not match for a VT switch. If
anybody thinks this just papers over the real issue, please speak up.
drivers/gpu/drm/drm_fb_helper.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..9663f3b8f287 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1259,9 +1259,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo
*var,
* Changes struct fb_var_screeninfo are currently not pushed back
* to KMS, hence fail if different settings are requested.
*/
- if (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
- var->xres != fb->width || var->yres != fb->height ||
- var->xres_virtual != fb->width || var->yres_virtual != fb->height) {
+ if ((info->flags & FBINFO_MISC_USEREVENT) &&
+ (var->bits_per_pixel != fb->format->cpp[0] * 8 ||
+var->xres != fb->width || var->yres != fb->height ||
+var->xres_virtual != fb->width || var->yres_virtual !=
fb->height)) {
DRM_DEBUG("fb userspace requested width/height/bpp different
than current fb "
"request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n",
var->xres, var->yres, var->bits_per_pixel,
--
2.11.0
Re: Linux 4.11: Reported regressions as of Tuesday, 20176-03-14
[ Moving this sub-thread to the amd-gfx mailing list ] On 14/03/17 07:02 PM, Thorsten Leemhuis wrote: > Hi! Find below my first regression report for Linux 4.11. It lists 9 > regressions I'm currently aware of. [...] > Desc: DRM BUG while initializing cape verde (2nd card) > Repo: 2017-03-13 https://bugzilla.kernel.org/show_bug.cgi?id=194867 > Stat: n/a > Note: patch proposed by reporter I don't see any patch. Looks like the amdgpu driver has never fully initialized GDS support for SI family GPUs, and this now triggers the DRM_MM_BUG_ON which was added to drm_mm_init in 4.11. AMD folks, should this be addressed by fleshing out SI GDS support, or by completely disabling GDS initialization for SI? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: Linux 4.11: Reported regressions as of Tuesday, 20176-03-14
[ Moving this sub-thread to the amd-gfx mailing list ] On 14/03/17 07:02 PM, Thorsten Leemhuis wrote: > Hi! Find below my first regression report for Linux 4.11. It lists 9 > regressions I'm currently aware of. [...] > Desc: DRM BUG while initializing cape verde (2nd card) > Repo: 2017-03-13 https://bugzilla.kernel.org/show_bug.cgi?id=194867 > Stat: n/a > Note: patch proposed by reporter I don't see any patch. Looks like the amdgpu driver has never fully initialized GDS support for SI family GPUs, and this now triggers the DRM_MM_BUG_ON which was added to drm_mm_init in 4.11. AMD folks, should this be addressed by fleshing out SI GDS support, or by completely disabling GDS initialization for SI? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 3.16 302/370] drm/radeon: Use mode h/vdisplay fields to hide out of bounds HW cursor
On 10/03/17 08:46 PM, Ben Hutchings wrote:
> 3.16.42-rc1 review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Michel Dänzer <michel.daen...@amd.com>
>
> commit d74c67dd7800fc7aae381f272875c337f268806c upstream.
>
> The crtc_h/vdisplay fields may not match the CRTC viewport dimensions
> with special modes such as interlaced ones.
>
> Fixes the HW cursor disappearing in the bottom half of the screen with
> interlaced modes.
>
> Fixes: 6b16cf7785a4 ("drm/radeon: Hide the HW cursor while it's out of
> bounds")
It might make sense to squash together the backports of this commit and
6b16cf7785a4, or at least move them closer together in the series, to
prevent people from hitting the regressed state.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 3.16 302/370] drm/radeon: Use mode h/vdisplay fields to hide out of bounds HW cursor
On 10/03/17 08:46 PM, Ben Hutchings wrote:
> 3.16.42-rc1 review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Michel Dänzer
>
> commit d74c67dd7800fc7aae381f272875c337f268806c upstream.
>
> The crtc_h/vdisplay fields may not match the CRTC viewport dimensions
> with special modes such as interlaced ones.
>
> Fixes the HW cursor disappearing in the bottom half of the screen with
> interlaced modes.
>
> Fixes: 6b16cf7785a4 ("drm/radeon: Hide the HW cursor while it's out of
> bounds")
It might make sense to squash together the backports of this commit and
6b16cf7785a4, or at least move them closer together in the series, to
prevent people from hitting the regressed state.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC
On 21/02/17 07:00 PM, Stefan Lengfeld wrote: > > 2/ Wait for a single vsync event on all active crtcs as Ville Syrjälä >described here [2] in the optimal implementation. FWIW, this seems like a bad idea, since with multiple active CRTCs it would make it essentially random at which intervals the ioctl returns, and which CRTCs are in vertical blank when it does. So it would be useless for both timing and tearing prevention purposes, i.e. more or less completely useless. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC
On 21/02/17 07:00 PM, Stefan Lengfeld wrote: > > 2/ Wait for a single vsync event on all active crtcs as Ville Syrjälä >described here [2] in the optimal implementation. FWIW, this seems like a bad idea, since with multiple active CRTCs it would make it essentially random at which intervals the ioctl returns, and which CRTCs are in vertical blank when it does. So it would be useless for both timing and tearing prevention purposes, i.e. more or less completely useless. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/amdgpu: shut up #warning for compile testing
On 02/02/17 06:36 PM, Christian König wrote: > Am 02.02.2017 um 07:09 schrieb Michel Dänzer: >> [SNIP] >> OTOH the people running the kernel aren't always the same people >> building it, so the downside is that this would potentially delay >> getting X86_PAT enabled. > > And exactly for this reason I would rather like to keep the warning. Right, so let's take the patch as is: Reviewed-by: Michel Dänzer <michel.daen...@amd.com> Arnd, can you make the corresponding patch for radeon as well? Otherwise I can probably do it next week. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/amdgpu: shut up #warning for compile testing
On 02/02/17 06:36 PM, Christian König wrote: > Am 02.02.2017 um 07:09 schrieb Michel Dänzer: >> [SNIP] >> OTOH the people running the kernel aren't always the same people >> building it, so the downside is that this would potentially delay >> getting X86_PAT enabled. > > And exactly for this reason I would rather like to keep the warning. Right, so let's take the patch as is: Reviewed-by: Michel Dänzer Arnd, can you make the corresponding patch for radeon as well? Otherwise I can probably do it next week. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/amdgpu: shut up #warning for compile testing
On 02/02/17 12:59 AM, Arnd Bergmann wrote: > My randconfig tests on linux-next showed a newly introduced warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function > 'amdgpu_bo_create_restricted': > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:377:2: error: #warning Please > enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to > write-combining [-Werror=cpp] > > Generally speaking, warnings about bad kernel configuration are not > particularly > helpful. We could enforce the selection of X86_PAT through Kconfig, so the > driver > cannot even be used unless it is enabled, Making AMDGPU depend on X86_PAT would be wrong I think, and any fancier Kconfig solution might be overkill. > or we could just rely on the runtime warning that is also there. That might make sense, since the runtime warning is only triggered when the lack of PAT actually makes a difference. OTOH the people running the kernel aren't always the same people building it, so the downside is that this would potentially delay getting X86_PAT enabled. > In this version, I'm making the warning conditional on CONFIG_COMPILE_TEST, > which > shuts it up for me, but not people that may actually want to run the kernel > as a compromize. This is fine with me as well. Whichever way we end up going for this, it should be applied to the radeon driver as well. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/amdgpu: shut up #warning for compile testing
On 02/02/17 12:59 AM, Arnd Bergmann wrote: > My randconfig tests on linux-next showed a newly introduced warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function > 'amdgpu_bo_create_restricted': > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:377:2: error: #warning Please > enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to > write-combining [-Werror=cpp] > > Generally speaking, warnings about bad kernel configuration are not > particularly > helpful. We could enforce the selection of X86_PAT through Kconfig, so the > driver > cannot even be used unless it is enabled, Making AMDGPU depend on X86_PAT would be wrong I think, and any fancier Kconfig solution might be overkill. > or we could just rely on the runtime warning that is also there. That might make sense, since the runtime warning is only triggered when the lack of PAT actually makes a difference. OTOH the people running the kernel aren't always the same people building it, so the downside is that this would potentially delay getting X86_PAT enabled. > In this version, I'm making the warning conditional on CONFIG_COMPILE_TEST, > which > shuts it up for me, but not people that may actually want to run the kernel > as a compromize. This is fine with me as well. Whichever way we end up going for this, it should be applied to the radeon driver as well. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable
On 26/01/17 09:46 AM, Sinclair Yeh wrote: > On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote: >> Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom: >>> On 01/25/2017 09:21 AM, Michel Dänzer wrote: >>>> From: Michel Dänzer <michel.daen...@amd.com> >>>> >>>> The current caching state may not be tt_cached, even though the >>>> placement contains TTM_PL_FLAG_CACHED, because placement can contain >>>> multiple caching flags. Trying to swap out such a BO would trip up the >>>> >>>>BUG_ON(ttm->caching_state != tt_cached); >>>> >>>> in ttm_tt_swapout. >>>> >>>> Cc: sta...@vger.kernel.org >>>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com> >>> Reviewed-by: Thomas Hellstrom <thellst...@vmware.com> >> >> Reviewed-by: Christian König <christian.koe...@amd.com>. > > Reviewed-by: Sinclair Yeh <s...@vmware.com> Thanks for the reviews! Via which tree should we merge this? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable
On 26/01/17 09:46 AM, Sinclair Yeh wrote: > On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote: >> Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom: >>> On 01/25/2017 09:21 AM, Michel Dänzer wrote: >>>> From: Michel Dänzer >>>> >>>> The current caching state may not be tt_cached, even though the >>>> placement contains TTM_PL_FLAG_CACHED, because placement can contain >>>> multiple caching flags. Trying to swap out such a BO would trip up the >>>> >>>>BUG_ON(ttm->caching_state != tt_cached); >>>> >>>> in ttm_tt_swapout. >>>> >>>> Cc: sta...@vger.kernel.org >>>> Signed-off-by: Michel Dänzer >>> Reviewed-by: Thomas Hellstrom >> >> Reviewed-by: Christian König . > > Reviewed-by: Sinclair Yeh Thanks for the reviews! Via which tree should we merge this? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [git pull] drm fixes for 4.10-rc6 (just missed rc5 tagging :-)
On 25/01/17 05:33 PM, Markus Trippelsdorf wrote: > On 2017.01.23 at 09:38 +1000, Dave Airlie wrote: >> >> Alex Deucher (8): >> drm/radeon/si: load special ucode for certain MC configs >> drm/amdgpu/si: load special ucode for certain MC configs >> drm/amdgpu: drop oland quirks >> drm/amdgpu: drop the mclk quirk for hainan >> drm/radeon: drop oland quirks >> drm/radeon: drop the mclk quirk for hainan >> drm/radeon: add support for new hainan variants >> drm/amdgpu: add support for new hainan variants > > Since the merge I get the following warning during boot: [...] > [2.627043] WARNING: CPU: 0 PID: 1 at ./include/drm/drm_crtc.h:857 > drm_kms_helper_poll_init+0x127/0x140 This is likely due to https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3846fd9b86001bea171943cc3bb9222cb6da6b42 Daniel, please take a look. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [git pull] drm fixes for 4.10-rc6 (just missed rc5 tagging :-)
On 25/01/17 05:33 PM, Markus Trippelsdorf wrote: > On 2017.01.23 at 09:38 +1000, Dave Airlie wrote: >> >> Alex Deucher (8): >> drm/radeon/si: load special ucode for certain MC configs >> drm/amdgpu/si: load special ucode for certain MC configs >> drm/amdgpu: drop oland quirks >> drm/amdgpu: drop the mclk quirk for hainan >> drm/radeon: drop oland quirks >> drm/radeon: drop the mclk quirk for hainan >> drm/radeon: add support for new hainan variants >> drm/amdgpu: add support for new hainan variants > > Since the merge I get the following warning during boot: [...] > [2.627043] WARNING: CPU: 0 PID: 1 at ./include/drm/drm_crtc.h:857 > drm_kms_helper_poll_init+0x127/0x140 This is likely due to https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3846fd9b86001bea171943cc3bb9222cb6da6b42 Daniel, please take a look. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH] drm/ttm: Make sure BOs being swapped out are cacheable
From: Michel Dänzer <michel.daen...@amd.com>
The current caching state may not be tt_cached, even though the
placement contains TTM_PL_FLAG_CACHED, because placement can contain
multiple caching flags. Trying to swap out such a BO would trip up the
BUG_ON(ttm->caching_state != tt_cached);
in ttm_tt_swapout.
Cc: sta...@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d5063618efa7..86e3b233b722 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1670,7 +1670,6 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
struct ttm_buffer_object *bo;
int ret = -EBUSY;
int put_count;
- uint32_t swap_placement = (TTM_PL_FLAG_CACHED | TTM_PL_FLAG_SYSTEM);
spin_lock(>lru_lock);
list_for_each_entry(bo, >swap_lru, swap) {
@@ -1701,7 +1700,8 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
* Move to system cached
*/
- if ((bo->mem.placement & swap_placement) != swap_placement) {
+ if (bo->mem.mem_type != TTM_PL_SYSTEM ||
+ bo->ttm->caching_state != tt_cached) {
struct ttm_mem_reg evict_mem;
evict_mem = bo->mem;
--
2.11.0
[PATCH] drm/ttm: Make sure BOs being swapped out are cacheable
From: Michel Dänzer
The current caching state may not be tt_cached, even though the
placement contains TTM_PL_FLAG_CACHED, because placement can contain
multiple caching flags. Trying to swap out such a BO would trip up the
BUG_ON(ttm->caching_state != tt_cached);
in ttm_tt_swapout.
Cc: sta...@vger.kernel.org
Signed-off-by: Michel Dänzer
---
drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d5063618efa7..86e3b233b722 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1670,7 +1670,6 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
struct ttm_buffer_object *bo;
int ret = -EBUSY;
int put_count;
- uint32_t swap_placement = (TTM_PL_FLAG_CACHED | TTM_PL_FLAG_SYSTEM);
spin_lock(>lru_lock);
list_for_each_entry(bo, >swap_lru, swap) {
@@ -1701,7 +1700,8 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
* Move to system cached
*/
- if ((bo->mem.placement & swap_placement) != swap_placement) {
+ if (bo->mem.mem_type != TTM_PL_SYSTEM ||
+ bo->ttm->caching_state != tt_cached) {
struct ttm_mem_reg evict_mem;
evict_mem = bo->mem;
--
2.11.0
Re: amdgpu: Corrupted video on 32 bit systems (possible fix)
On 20/01/17 04:44 PM, Nils Holland wrote: > On Fri, Jan 20, 2017 at 11:47:53AM +0900, Michel Dänzer wrote: >> On 20/01/17 04:35 AM, Nils Holland wrote: >>> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c2016-12-11 >>> 20:17:54.0 +0100 >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c2017-01-19 >>> 15:38:56.972034489 +0100 >>> @@ -372,6 +372,10 @@ >>> if (!drm_arch_can_wc_memory()) >>> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; >>> >>> + #ifdef CONFIG_X86_32 >>> + bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; >>> + #endif >>> + >>> amdgpu_fill_placement_to_bo(bo, placement); >>> /* Kernel allocation are uninterruptible */ >>> r = ttm_bo_init(>mman.bdev, >tbo, size, type, >> >> The corresponding code in the radeon driver has changed quite a bit >> since this original fix. It would be better to bring the amdgpu code in >> line with the current radeon code. >> >> >>> With this patch, the amdgpu driver works fine for me on my 32 bit >>> kernel: All graphics output looks the way it's supposed to, even with >>> full acceleration enabled - great! >>> >>> I'd suggest that it might be a good idea to put to apply the above >>> patch or something similar to the official sources. >> >> Indeed. Do you want to create a proper patch and submit it to the >> amd-gfx mailing list for review? See Documentation/SubmittingPatches for >> more information. > > Sounds like a good idea! I was a bit heasitant because, to be honest, > I'm not at all an expert about the code in question and basically only > saw how you fixed the issue in radeon and thought: "Well, let's see if > I can do the same thing in amdgpu and if so, if it helps there, too". > ;-) > > However, since you've said that a 32 bit fix in amdgpu generally seems > like a good idea, Actually, unless your CPU can't run 64-bit code, I'd say running a 64-bit kernel would be an overall even better idea for you, even with 32-bit userspace. :) Anyway, this problem clearly needs to be fixed. > I would indeed use a little time on the weekend to get a proper patch > ready and submit it for review. Even if the "no wc for x86_32" part is > probably the only thing it'll contain I wouldn't bother with that. There is no real reason against bringing it all over in one go. > - more of "bringing the amdgpu code in line with the current radeon > code" might, for the time being, be beyond my capabilities, at least > if we assume that the code should stay in a sane and working > condition. ;-) Don't worry, it's mostly a matter of copy'n'paste, testing on your end, review and more testing by others. :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: amdgpu: Corrupted video on 32 bit systems (possible fix)
On 20/01/17 04:44 PM, Nils Holland wrote: > On Fri, Jan 20, 2017 at 11:47:53AM +0900, Michel Dänzer wrote: >> On 20/01/17 04:35 AM, Nils Holland wrote: >>> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c2016-12-11 >>> 20:17:54.0 +0100 >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c2017-01-19 >>> 15:38:56.972034489 +0100 >>> @@ -372,6 +372,10 @@ >>> if (!drm_arch_can_wc_memory()) >>> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; >>> >>> + #ifdef CONFIG_X86_32 >>> + bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; >>> + #endif >>> + >>> amdgpu_fill_placement_to_bo(bo, placement); >>> /* Kernel allocation are uninterruptible */ >>> r = ttm_bo_init(>mman.bdev, >tbo, size, type, >> >> The corresponding code in the radeon driver has changed quite a bit >> since this original fix. It would be better to bring the amdgpu code in >> line with the current radeon code. >> >> >>> With this patch, the amdgpu driver works fine for me on my 32 bit >>> kernel: All graphics output looks the way it's supposed to, even with >>> full acceleration enabled - great! >>> >>> I'd suggest that it might be a good idea to put to apply the above >>> patch or something similar to the official sources. >> >> Indeed. Do you want to create a proper patch and submit it to the >> amd-gfx mailing list for review? See Documentation/SubmittingPatches for >> more information. > > Sounds like a good idea! I was a bit heasitant because, to be honest, > I'm not at all an expert about the code in question and basically only > saw how you fixed the issue in radeon and thought: "Well, let's see if > I can do the same thing in amdgpu and if so, if it helps there, too". > ;-) > > However, since you've said that a 32 bit fix in amdgpu generally seems > like a good idea, Actually, unless your CPU can't run 64-bit code, I'd say running a 64-bit kernel would be an overall even better idea for you, even with 32-bit userspace. :) Anyway, this problem clearly needs to be fixed. > I would indeed use a little time on the weekend to get a proper patch > ready and submit it for review. Even if the "no wc for x86_32" part is > probably the only thing it'll contain I wouldn't bother with that. There is no real reason against bringing it all over in one go. > - more of "bringing the amdgpu code in line with the current radeon > code" might, for the time being, be beyond my capabilities, at least > if we assume that the code should stay in a sane and working > condition. ;-) Don't worry, it's mostly a matter of copy'n'paste, testing on your end, review and more testing by others. :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: amdgpu: Corrupted video on 32 bit systems (possible fix)
On 20/01/17 04:35 AM, Nils Holland wrote: > Hi folks, > > there seems to be an issue on 32 bit kernels which makes graphics > output look all messed up under X when using the amdgpu drm kernel > driver. > > In fact, the same issue was present at some time in 2015 using the > radeon driver too, but it has been fixed a long time ago, as can be > seen here: > > https://bugs.freedesktop.org/show_bug.cgi?id=84627#c35 > > Now, I reported the same issue in conjunction with the amdgpu driver > back when I first noticed it back then. Nothing has happened since > then, but my bug report can still be found here: > > https://bugs.freedesktop.org/show_bug.cgi?id=91831 > > Personally, I had been using the (working) radeon driver again, and > only today I decided to check if the issue in amdgpu still exists. And > yes, it seems so: I still get garbled output when using a current > kernel with a current version of the amdgpu driver. > > Now, I've tried to just "port" the fix that had been done to the radeon > driver in 2015 (see the first link above) to the amdgpu driver and created > myself the following little patch: > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 2016-12-11 > 20:17:54.0 +0100 > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 2017-01-19 > 15:38:56.972034489 +0100 > @@ -372,6 +372,10 @@ > if (!drm_arch_can_wc_memory()) > bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; > > + #ifdef CONFIG_X86_32 > + bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; > + #endif > + > amdgpu_fill_placement_to_bo(bo, placement); > /* Kernel allocation are uninterruptible */ > r = ttm_bo_init(>mman.bdev, >tbo, size, type, The corresponding code in the radeon driver has changed quite a bit since this original fix. It would be better to bring the amdgpu code in line with the current radeon code. > With this patch, the amdgpu driver works fine for me on my 32 bit > kernel: All graphics output looks the way it's supposed to, even with > full acceleration enabled - great! > > I'd suggest that it might be a good idea to put to apply the above > patch or something similar to the official sources. Indeed. Do you want to create a proper patch and submit it to the amd-gfx mailing list for review? See Documentation/SubmittingPatches for more information. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: amdgpu: Corrupted video on 32 bit systems (possible fix)
On 20/01/17 04:35 AM, Nils Holland wrote: > Hi folks, > > there seems to be an issue on 32 bit kernels which makes graphics > output look all messed up under X when using the amdgpu drm kernel > driver. > > In fact, the same issue was present at some time in 2015 using the > radeon driver too, but it has been fixed a long time ago, as can be > seen here: > > https://bugs.freedesktop.org/show_bug.cgi?id=84627#c35 > > Now, I reported the same issue in conjunction with the amdgpu driver > back when I first noticed it back then. Nothing has happened since > then, but my bug report can still be found here: > > https://bugs.freedesktop.org/show_bug.cgi?id=91831 > > Personally, I had been using the (working) radeon driver again, and > only today I decided to check if the issue in amdgpu still exists. And > yes, it seems so: I still get garbled output when using a current > kernel with a current version of the amdgpu driver. > > Now, I've tried to just "port" the fix that had been done to the radeon > driver in 2015 (see the first link above) to the amdgpu driver and created > myself the following little patch: > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 2016-12-11 > 20:17:54.0 +0100 > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 2017-01-19 > 15:38:56.972034489 +0100 > @@ -372,6 +372,10 @@ > if (!drm_arch_can_wc_memory()) > bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; > > + #ifdef CONFIG_X86_32 > + bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC; > + #endif > + > amdgpu_fill_placement_to_bo(bo, placement); > /* Kernel allocation are uninterruptible */ > r = ttm_bo_init(>mman.bdev, >tbo, size, type, The corresponding code in the radeon driver has changed quite a bit since this original fix. It would be better to bring the amdgpu code in line with the current radeon code. > With this patch, the amdgpu driver works fine for me on my 32 bit > kernel: All graphics output looks the way it's supposed to, even with > full acceleration enabled - great! > > I'd suggest that it might be a good idea to put to apply the above > patch or something similar to the official sources. Indeed. Do you want to create a proper patch and submit it to the amd-gfx mailing list for review? See Documentation/SubmittingPatches for more information. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [RFC PATCH 0/3] staging: remove fbdev drivers
On 10/12/16 05:27 AM, Benjamin Herrenschmidt wrote: > On Fri, 2016-12-09 at 14:35 +0100, Daniel Vetter wrote: >>> As for multi userspace client, well, swapping an mmap between HW and >>> memory backing store is a somewhat solved problem already. >> >> Hm, I didn't know that, but then all existing drm drivers have fairly >> simplistic fbdev mmap implementations. > > Hrm, I though the TTM did it ... I remember talking with Thomas > Hellstrom about that back in the day... you use unmap_mapping_range > to unmap the existing mappings basically so you can take new faults > and route them to a different page, but I can't see a call in there > so maybe he ended up not doing it. I think he did, it was working fine for userspace mappings when I tried making radeon use a non-pinned BO for fbdev years ago (the problem was fbcon potentially trying to access the framebuffer at the most inconvenient times). There's still ttm_fbdev_mmap, but I'm not sure everything to make this fully work for userspace fbdev mappings is still there. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [RFC PATCH 0/3] staging: remove fbdev drivers
On 10/12/16 05:27 AM, Benjamin Herrenschmidt wrote: > On Fri, 2016-12-09 at 14:35 +0100, Daniel Vetter wrote: >>> As for multi userspace client, well, swapping an mmap between HW and >>> memory backing store is a somewhat solved problem already. >> >> Hm, I didn't know that, but then all existing drm drivers have fairly >> simplistic fbdev mmap implementations. > > Hrm, I though the TTM did it ... I remember talking with Thomas > Hellstrom about that back in the day... you use unmap_mapping_range > to unmap the existing mappings basically so you can take new faults > and route them to a different page, but I can't see a call in there > so maybe he ended up not doing it. I think he did, it was working fine for userspace mappings when I tried making radeon use a non-pinned BO for fbdev years ago (the problem was fbcon potentially trying to access the framebuffer at the most inconvenient times). There's still ttm_fbdev_mmap, but I'm not sure everything to make this fully work for userspace fbdev mappings is still there. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/radeon: don't add files at control minor debugfs directory
On 05/12/16 05:48 PM, Christian König wrote: > Am 05.12.2016 um 09:39 schrieb Nicolai Stange:>> >> I'll send compile-only tested patches either tonight or tomorrow. Shall >> they cover the oopses only or the dead code as well? > > Please start with the ops, cause we certainly will get complains about > that rather fast. Suspect we already did: https://bugs.freedesktop.org/show_bug.cgi?id=98915 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/radeon: don't add files at control minor debugfs directory
On 05/12/16 05:48 PM, Christian König wrote: > Am 05.12.2016 um 09:39 schrieb Nicolai Stange:>> >> I'll send compile-only tested patches either tonight or tomorrow. Shall >> they cover the oopses only or the dead code as well? > > Please start with the ops, cause we certainly will get complains about > that rather fast. Suspect we already did: https://bugs.freedesktop.org/show_bug.cgi?id=98915 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: drm: GPF in drm_getcap
On 28/11/16 03:55 PM, Daniel Vetter wrote: > On Sat, Nov 26, 2016 at 7:22 PM, David Herrmann <dh.herrm...@gmail.com> wrote: >> On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov <dvyu...@google.com> wrote: >>> grep "card0" dmesg: >>> [5.298617] device: 'card0': device_add >>> [5.298946] PM: Adding info for No Bus:card0 >>> [6.436178] device: 'card0': device_add >>> [6.436488] PM: Adding info for No Bus:card0 >>> >>> >>> # ls -l /dev/dri/card0 >>> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0 >>> >>> # ls -lt /sys/class/drm/card0/device/ >>> ls: cannot access /sys/class/drm/card0/device/: No such file or directory >>> >>> # ls -lt /sys/class/drm/card0/device/driver >>> ls: cannot access /sys/class/drm/card0/device/driver: No such file or >>> directory >> >> Looks like vgem. Something like this should help: >> >> https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2 >> >> I wonder whether it would be more appropriate to return -ENOTSUPP rather >> than 0. Can't see how that would matter FWIW. > Seems a bit overkill, but can't hurt. This is most likely a > regression, probably introduced in > > commit f837297ad82480024d3ad08cd84f6670bcafa862 > Author: Michel Dänzer <michel.daen...@amd.com> > Date: Mon Aug 8 16:23:39 2016 +0900 > > drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2 > > Michel, can you pls take care of this? Either with a minimal fix, or > by adopting David's patch? Can't we just use David's patch as-is? If not, I think Dmitry or someone else would be better equipped than me to extract a minimal fix from it and test it. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: drm: GPF in drm_getcap
On 28/11/16 03:55 PM, Daniel Vetter wrote: > On Sat, Nov 26, 2016 at 7:22 PM, David Herrmann wrote: >> On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov wrote: >>> grep "card0" dmesg: >>> [5.298617] device: 'card0': device_add >>> [5.298946] PM: Adding info for No Bus:card0 >>> [6.436178] device: 'card0': device_add >>> [6.436488] PM: Adding info for No Bus:card0 >>> >>> >>> # ls -l /dev/dri/card0 >>> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0 >>> >>> # ls -lt /sys/class/drm/card0/device/ >>> ls: cannot access /sys/class/drm/card0/device/: No such file or directory >>> >>> # ls -lt /sys/class/drm/card0/device/driver >>> ls: cannot access /sys/class/drm/card0/device/driver: No such file or >>> directory >> >> Looks like vgem. Something like this should help: >> >> https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2 >> >> I wonder whether it would be more appropriate to return -ENOTSUPP rather >> than 0. Can't see how that would matter FWIW. > Seems a bit overkill, but can't hurt. This is most likely a > regression, probably introduced in > > commit f837297ad82480024d3ad08cd84f6670bcafa862 > Author: Michel Dänzer > Date: Mon Aug 8 16:23:39 2016 +0900 > > drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2 > > Michel, can you pls take care of this? Either with a minimal fix, or > by adopting David's patch? Can't we just use David's patch as-is? If not, I think Dmitry or someone else would be better equipped than me to extract a minimal fix from it and test it. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: radeon_connector_unregister NULL ptr deref
On 24/10/16 04:34 PM, Borislav Petkov wrote: > Hi guys, > > I'm getting a NULL ptr deref splat when hibernating my box with > 4.9-rc1+. All I got so far is an ugly camera shot from the splat which > I'm typing in by hand. > > Any ideas or already a fix? > > The callstack looks like this: > > unable to handle kernel NULL pointer dereference at 00...0890 (I think it is > 890) > IP: radeon_connector_unregister+0xc/0x40 > ... > > ? drm_connector_unregister.part > drm_connector_unregister_all > drm_modeset_unregister_all > drm_dev_unregister > drm_put_dev > radeon_pci_shutdown > pci_device_shutdown > device_shutdown > kernel_power_off > power_down > hibernate > ... Should be fixed in -rc2, specifically these commits: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=b0c80bd5d2e317f7596fe2badc1a3379fb3211e5 https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=9305ee6fe52035f63d70d023235b792ba22107f0 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: radeon_connector_unregister NULL ptr deref
On 24/10/16 04:34 PM, Borislav Petkov wrote: > Hi guys, > > I'm getting a NULL ptr deref splat when hibernating my box with > 4.9-rc1+. All I got so far is an ugly camera shot from the splat which > I'm typing in by hand. > > Any ideas or already a fix? > > The callstack looks like this: > > unable to handle kernel NULL pointer dereference at 00...0890 (I think it is > 890) > IP: radeon_connector_unregister+0xc/0x40 > ... > > ? drm_connector_unregister.part > drm_connector_unregister_all > drm_modeset_unregister_all > drm_dev_unregister > drm_put_dev > radeon_pci_shutdown > pci_device_shutdown > device_shutdown > kernel_power_off > power_down > hibernate > ... Should be fixed in -rc2, specifically these commits: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=b0c80bd5d2e317f7596fe2badc1a3379fb3211e5 https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=9305ee6fe52035f63d70d023235b792ba22107f0 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: kernel panic caused by raedon driver during reboot
On 17/10/16 04:17 PM, Baoquan He wrote: > In fact this could happen in radeon_pci_shutdown. I tried reboot and > poweroff, kernel panic happened in both cases. Attachments are console > log and kernel config of 4.9-rc1 on linus's tree. This is fixed with these commits, which should land in Linus' tree soon: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=b0c80bd5d2e317f7596fe2badc1a3379fb3211e5 https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=9305ee6fe52035f63d70d023235b792ba22107f0 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: kernel panic caused by raedon driver during reboot
On 17/10/16 04:17 PM, Baoquan He wrote: > In fact this could happen in radeon_pci_shutdown. I tried reboot and > poweroff, kernel panic happened in both cases. Attachments are console > log and kernel config of 4.9-rc1 on linus's tree. This is fixed with these commits, which should land in Linus' tree soon: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=b0c80bd5d2e317f7596fe2badc1a3379fb3211e5 https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=9305ee6fe52035f63d70d023235b792ba22107f0 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 07.07.2016 16:43, Christian König wrote: > Am 07.07.2016 um 05:32 schrieb Michel Dänzer: >> On 06.07.2016 22:45, Tejun Heo wrote: >>> On Wed, Jul 06, 2016 at 12:12:52PM +0900, Michel Dänzer wrote: >>> >>>> Not being very familiar with the workqueue APIs, I'll describe how it's >>>> supposed to work from a driver POV, which will hopefully help you guys >>>> decide on the most appropriate alloc_workqueue parameters. >>>> >>>> There is one flip work queue for each hardware CRTC. At most one >>>> radeon_flip_work_func item can be queued for any of them at any time. >>>> When a radeon_flip_work_func item is queued, it should be executed ASAP >>>> (so WQ_HIGHPRI might be appropriate?). >>> Hmmm... the only time WQ_HIGHPRI should be used is when it'd otherwise >>> require a kthread w/ nice value at -20. Would that be the case here? >>> What are the consequences of the work item getting delayed? >> A page flip may be delayed to a later display refresh cycle. >> >> >>> Also, what kind of delays matter here? Is it millisec range or micro? >> It can be the latter in theory, but normally rather the former. > > Well to be precise with a typical 1920x1080@60 resolution you have about > 2.16ms time under ideal conditions for the flip. > > So using the high priority queue still sounds like a good idea to me. How did you arrive at 2.16ms? Userspace can call the ioctl up to one full refresh cycle ahead of time, which is ~16ms at 60 Hz. On the other hand userspace can also call the ioctl arbitrarily close to the vertical blank period, in which case even a delay of just 1ms (or even significantly less) may cause the flip to be delayed by one refresh cycle. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 07.07.2016 16:43, Christian König wrote: > Am 07.07.2016 um 05:32 schrieb Michel Dänzer: >> On 06.07.2016 22:45, Tejun Heo wrote: >>> On Wed, Jul 06, 2016 at 12:12:52PM +0900, Michel Dänzer wrote: >>> >>>> Not being very familiar with the workqueue APIs, I'll describe how it's >>>> supposed to work from a driver POV, which will hopefully help you guys >>>> decide on the most appropriate alloc_workqueue parameters. >>>> >>>> There is one flip work queue for each hardware CRTC. At most one >>>> radeon_flip_work_func item can be queued for any of them at any time. >>>> When a radeon_flip_work_func item is queued, it should be executed ASAP >>>> (so WQ_HIGHPRI might be appropriate?). >>> Hmmm... the only time WQ_HIGHPRI should be used is when it'd otherwise >>> require a kthread w/ nice value at -20. Would that be the case here? >>> What are the consequences of the work item getting delayed? >> A page flip may be delayed to a later display refresh cycle. >> >> >>> Also, what kind of delays matter here? Is it millisec range or micro? >> It can be the latter in theory, but normally rather the former. > > Well to be precise with a typical 1920x1080@60 resolution you have about > 2.16ms time under ideal conditions for the flip. > > So using the high priority queue still sounds like a good idea to me. How did you arrive at 2.16ms? Userspace can call the ioctl up to one full refresh cycle ahead of time, which is ~16ms at 60 Hz. On the other hand userspace can also call the ioctl arbitrarily close to the vertical blank period, in which case even a delay of just 1ms (or even significantly less) may cause the flip to be delayed by one refresh cycle. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 06.07.2016 22:45, Tejun Heo wrote: > On Wed, Jul 06, 2016 at 12:12:52PM +0900, Michel Dänzer wrote: > >> Not being very familiar with the workqueue APIs, I'll describe how it's >> supposed to work from a driver POV, which will hopefully help you guys >> decide on the most appropriate alloc_workqueue parameters. >> >> There is one flip work queue for each hardware CRTC. At most one >> radeon_flip_work_func item can be queued for any of them at any time. >> When a radeon_flip_work_func item is queued, it should be executed ASAP >> (so WQ_HIGHPRI might be appropriate?). > > Hmmm... the only time WQ_HIGHPRI should be used is when it'd otherwise > require a kthread w/ nice value at -20. Would that be the case here? > What are the consequences of the work item getting delayed? A page flip may be delayed to a later display refresh cycle. > Also, what kind of delays matter here? Is it millisec range or micro? It can be the latter in theory, but normally rather the former. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 06.07.2016 22:45, Tejun Heo wrote: > On Wed, Jul 06, 2016 at 12:12:52PM +0900, Michel Dänzer wrote: > >> Not being very familiar with the workqueue APIs, I'll describe how it's >> supposed to work from a driver POV, which will hopefully help you guys >> decide on the most appropriate alloc_workqueue parameters. >> >> There is one flip work queue for each hardware CRTC. At most one >> radeon_flip_work_func item can be queued for any of them at any time. >> When a radeon_flip_work_func item is queued, it should be executed ASAP >> (so WQ_HIGHPRI might be appropriate?). > > Hmmm... the only time WQ_HIGHPRI should be used is when it'd otherwise > require a kthread w/ nice value at -20. Would that be the case here? > What are the consequences of the work item getting delayed? A page flip may be delayed to a later display refresh cycle. > Also, what kind of delays matter here? Is it millisec range or micro? It can be the latter in theory, but normally rather the former. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 06.07.2016 06:06, Tejun Heo wrote: > On Mon, Jul 04, 2016 at 12:58:32PM +0900, Michel Dänzer wrote: >> On 02.07.2016 22:46, Tejun Heo wrote: >>> On Sat, Jul 02, 2016 at 04:33:50PM +0530, Bhaktipriya Shridhar wrote: >>>> alloc_workqueue replaces deprecated create_singlethread_workqueue(). >>>> >>>> A dedicated workqueue has been used since work items need to be flushed >>>> as a group rather than individually. > > There seem to be two work items involved, the flip one and unpin one. > Provided that there's no ordering requirement between the two, can't > we just flush them individually? There is an ordering requirement between the two queues, but it's enforced by the driver (by only queuing the unpin work once a flip has completed, which only happens after the corresponding flip work has run). Not being very familiar with the workqueue APIs, I'll describe how it's supposed to work from a driver POV, which will hopefully help you guys decide on the most appropriate alloc_workqueue parameters. There is one flip work queue for each hardware CRTC. At most one radeon_flip_work_func item can be queued for any of them at any time. When a radeon_flip_work_func item is queued, it should be executed ASAP (so WQ_HIGHPRI might be appropriate?). In contrast, the radeon_unpin_work_func items aren't particularly urgent, and it's okay for several of them to be queued up. So I guess it would actually make sense to use a different workqueue for them, maybe even the default one? >>>> Since there are only a fixed number of work items, explicit concurrency >>>> limit is unnecessary here. >>> >>> What are the involved work items? >> >> drivers/gpu/drm/radeon/radeon_display.c:radeon_flip_work_func() >> >>> Is it safe to run them concurrently? >> >> Concurrently with what exactly? > > The flip and unpin work items. Yes, it's safe to run those concurrently. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 06.07.2016 06:06, Tejun Heo wrote: > On Mon, Jul 04, 2016 at 12:58:32PM +0900, Michel Dänzer wrote: >> On 02.07.2016 22:46, Tejun Heo wrote: >>> On Sat, Jul 02, 2016 at 04:33:50PM +0530, Bhaktipriya Shridhar wrote: >>>> alloc_workqueue replaces deprecated create_singlethread_workqueue(). >>>> >>>> A dedicated workqueue has been used since work items need to be flushed >>>> as a group rather than individually. > > There seem to be two work items involved, the flip one and unpin one. > Provided that there's no ordering requirement between the two, can't > we just flush them individually? There is an ordering requirement between the two queues, but it's enforced by the driver (by only queuing the unpin work once a flip has completed, which only happens after the corresponding flip work has run). Not being very familiar with the workqueue APIs, I'll describe how it's supposed to work from a driver POV, which will hopefully help you guys decide on the most appropriate alloc_workqueue parameters. There is one flip work queue for each hardware CRTC. At most one radeon_flip_work_func item can be queued for any of them at any time. When a radeon_flip_work_func item is queued, it should be executed ASAP (so WQ_HIGHPRI might be appropriate?). In contrast, the radeon_unpin_work_func items aren't particularly urgent, and it's okay for several of them to be queued up. So I guess it would actually make sense to use a different workqueue for them, maybe even the default one? >>>> Since there are only a fixed number of work items, explicit concurrency >>>> limit is unnecessary here. >>> >>> What are the involved work items? >> >> drivers/gpu/drm/radeon/radeon_display.c:radeon_flip_work_func() >> >>> Is it safe to run them concurrently? >> >> Concurrently with what exactly? > > The flip and unpin work items. Yes, it's safe to run those concurrently. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 02.07.2016 22:46, Tejun Heo wrote: > On Sat, Jul 02, 2016 at 04:33:50PM +0530, Bhaktipriya Shridhar wrote: >> alloc_workqueue replaces deprecated create_singlethread_workqueue(). >> >> A dedicated workqueue has been used since work items need to be flushed >> as a group rather than individually. >> >> Since the flip_queue workqueue is involved in page-flipping and is not >> being used on a memory reclaim path, WQ_MEM_RECLAIM has not been set. >> >> Since there are only a fixed number of work items, explicit concurrency >> limit is unnecessary here. > > What are the involved work items? drivers/gpu/drm/radeon/radeon_display.c:radeon_flip_work_func() > Is it safe to run them concurrently? Concurrently with what exactly? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH] drm/radeon: Remove deprecated create_singlethread_workqueue
On 02.07.2016 22:46, Tejun Heo wrote: > On Sat, Jul 02, 2016 at 04:33:50PM +0530, Bhaktipriya Shridhar wrote: >> alloc_workqueue replaces deprecated create_singlethread_workqueue(). >> >> A dedicated workqueue has been used since work items need to be flushed >> as a group rather than individually. >> >> Since the flip_queue workqueue is involved in page-flipping and is not >> being used on a memory reclaim path, WQ_MEM_RECLAIM has not been set. >> >> Since there are only a fixed number of work items, explicit concurrency >> limit is unnecessary here. > > What are the involved work items? drivers/gpu/drm/radeon/radeon_display.c:radeon_flip_work_func() > Is it safe to run them concurrently? Concurrently with what exactly? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 06/10] drm/amdgpu: use drm_crtc_vblank_{on,off}()
On 07.06.2016 23:07, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
>
> Replace the legacy drm_vblank_{on,off}() with the new helper functions.
>
> Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
Patches 6 & 8-10 are
Reviewed-by: Michel Dänzer <michel.daen...@amd.com>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 06/10] drm/amdgpu: use drm_crtc_vblank_{on,off}()
On 07.06.2016 23:07, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Replace the legacy drm_vblank_{on,off}() with the new helper functions.
>
> Signed-off-by: Gustavo Padovan
Patches 6 & 8-10 are
Reviewed-by: Michel Dänzer
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 08/14] drm/amdgpu: use drm_crtc_vblank_{get,put}()
On 06.06.2016 23:41, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
>
> Replace the legacy drm_vblank_{get,put}() with the new helper functions.
>
> Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
[...]
> @@ -268,7 +268,7 @@ int amdgpu_crtc_page_flip(struct drm_crtc *crtc,
> return 0;
>
> vblank_cleanup:
> - drm_vblank_put(crtc->dev, amdgpu_crtc->crtc_id);
> + drm_crtc_vblank_put(_crtc->base);
Can just use crtc here instead of _crtc->base. Same for the
radeon patch.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 08/14] drm/amdgpu: use drm_crtc_vblank_{get,put}()
On 06.06.2016 23:41, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Replace the legacy drm_vblank_{get,put}() with the new helper functions.
>
> Signed-off-by: Gustavo Padovan
[...]
> @@ -268,7 +268,7 @@ int amdgpu_crtc_page_flip(struct drm_crtc *crtc,
> return 0;
>
> vblank_cleanup:
> - drm_vblank_put(crtc->dev, amdgpu_crtc->crtc_id);
> + drm_crtc_vblank_put(_crtc->base);
Can just use crtc here instead of _crtc->base. Same for the
radeon patch.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 01/14] drm/amdgpu: use drm_crtc_send_vblank_event()
On 15.04.2016 02:48, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.pado...@collabora.co.uk> > > Replace the legacy drm_send_vblank_event() with the new helper function. > > Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk> > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 6de2ce53..92c5a71 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -3370,7 +3370,7 @@ static int dce_v10_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index e9ccc6b..2f784f2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -3366,7 +3366,7 @@ static int dce_v11_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if(works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index e56b55d..9155e3b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -3379,7 +3379,7 @@ static int dce_v8_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > This patch and patch 8 are Reviewed-by: Michel Dänzer <michel.daen...@amd.com> -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 01/14] drm/amdgpu: use drm_crtc_send_vblank_event()
On 15.04.2016 02:48, Gustavo Padovan wrote: > From: Gustavo Padovan > > Replace the legacy drm_send_vblank_event() with the new helper function. > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 6de2ce53..92c5a71 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -3370,7 +3370,7 @@ static int dce_v10_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index e9ccc6b..2f784f2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -3366,7 +3366,7 @@ static int dce_v11_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if(works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index e56b55d..9155e3b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -3379,7 +3379,7 @@ static int dce_v8_0_pageflip_irq(struct amdgpu_device > *adev, > > /* wakeup usersapce */ > if (works->event) > - drm_send_vblank_event(adev->ddev, crtc_id, works->event); > + drm_crtc_send_vblank_event(_crtc->base, works->event); > > spin_unlock_irqrestore(>ddev->event_lock, flags); > > This patch and patch 8 are Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 12.04.2016 23:16, Greg Kroah-Hartman wrote: > On Mon, Apr 11, 2016 at 11:26:00AM +0900, Michel Dänzer wrote: >> On 11.04.2016 03:36, Greg Kroah-Hartman wrote: >>> 4.4-stable review patch. If anyone has any objections, please let me know. >> >> A regression was bisected to this commit: >> >> https://bugzilla.kernel.org/show_bug.cgi?id=115321 FYI, the backport to 4.5 triggered another bug report: https://bugzilla.kernel.org/show_bug.cgi?id=116251 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 12.04.2016 23:16, Greg Kroah-Hartman wrote: > On Mon, Apr 11, 2016 at 11:26:00AM +0900, Michel Dänzer wrote: >> On 11.04.2016 03:36, Greg Kroah-Hartman wrote: >>> 4.4-stable review patch. If anyone has any objections, please let me know. >> >> A regression was bisected to this commit: >> >> https://bugzilla.kernel.org/show_bug.cgi?id=115321 FYI, the backport to 4.5 triggered another bug report: https://bugzilla.kernel.org/show_bug.cgi?id=116251 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 12.04.2016 23:16, Greg Kroah-Hartman wrote:
> On Mon, Apr 11, 2016 at 11:26:00AM +0900, Michel Dänzer wrote:
>> On 11.04.2016 03:36, Greg Kroah-Hartman wrote:
>>> 4.4-stable review patch. If anyone has any objections, please let me know.
>>
>> A regression was bisected to this commit:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=115321
>
> Why not just provide stable with the bugfix as well?
What bugfix are you referring to? I'm not aware of any fix for this
regression.
>> So it's probably better to hold off on backporting this commit (and
>> possibly the corresponding amdgpu commit as well).
>
> What specific commit are you referring to here?
bedf2a65c1aa8fb29ba8527fd00c0f68ec1f55f1 ("drm/amdgpu: disable runtime
pm on PX laptops without dGPU power control")
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 12.04.2016 23:16, Greg Kroah-Hartman wrote:
> On Mon, Apr 11, 2016 at 11:26:00AM +0900, Michel Dänzer wrote:
>> On 11.04.2016 03:36, Greg Kroah-Hartman wrote:
>>> 4.4-stable review patch. If anyone has any objections, please let me know.
>>
>> A regression was bisected to this commit:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=115321
>
> Why not just provide stable with the bugfix as well?
What bugfix are you referring to? I'm not aware of any fix for this
regression.
>> So it's probably better to hold off on backporting this commit (and
>> possibly the corresponding amdgpu commit as well).
>
> What specific commit are you referring to here?
bedf2a65c1aa8fb29ba8527fd00c0f68ec1f55f1 ("drm/amdgpu: disable runtime
pm on PX laptops without dGPU power control")
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 11.04.2016 03:36, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. A regression was bisected to this commit: https://bugzilla.kernel.org/show_bug.cgi?id=115321 So it's probably better to hold off on backporting this commit (and possibly the corresponding amdgpu commit as well). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 4.4 159/210] drm/radeon: disable runtime pm on PX laptops without dGPU power control
On 11.04.2016 03:36, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. A regression was bisected to this commit: https://bugzilla.kernel.org/show_bug.cgi?id=115321 So it's probably better to hold off on backporting this commit (and possibly the corresponding amdgpu commit as well). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: BUG caused by "Use new drm_fb_helper functions" series
On 03.02.2016 03:38, Peter Hurley wrote: > On 02/01/2016 09:20 PM, Archit Taneja wrote: >> Hi Peter, >> >> On 02/02/2016 02:07 AM, Peter Hurley wrote: >>> Hi Archit, >>> >>> Just booting 4.4-rc5+, I got this splat [1] >>> At first glance, this appears to be a simple fix. >> >> Thanks for sharing this. >> >>> >>> However, I'm concerned that fbcon functions, which may be called with >>> interrupts disabled, are now hooked up to fbdev functions which may assume >>> interrupts are not disabled (as is the case with cfb_imageblit()). >> >> In the case when CONFIG_FB is enabled, drm_fb_helper_cfb_imageblit >> helper simply wraps around cfg_imageblit, so I don't see how we'd have >> any difference in behaviour. > > > Sorry; terrible attribution on my part. > This bug clearly has nothing to do with this series. > > But a better look has me wondering how all these gpus are syncing > the framebuffer for direct access via cfb_imageblit and friends. I only see > nouveau and intel gma even trying. Probably no other DRM driver uses hardware acceleration for fbcon. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: BUG caused by "Use new drm_fb_helper functions" series
On 03.02.2016 03:38, Peter Hurley wrote: > On 02/01/2016 09:20 PM, Archit Taneja wrote: >> Hi Peter, >> >> On 02/02/2016 02:07 AM, Peter Hurley wrote: >>> Hi Archit, >>> >>> Just booting 4.4-rc5+, I got this splat [1] >>> At first glance, this appears to be a simple fix. >> >> Thanks for sharing this. >> >>> >>> However, I'm concerned that fbcon functions, which may be called with >>> interrupts disabled, are now hooked up to fbdev functions which may assume >>> interrupts are not disabled (as is the case with cfb_imageblit()). >> >> In the case when CONFIG_FB is enabled, drm_fb_helper_cfb_imageblit >> helper simply wraps around cfg_imageblit, so I don't see how we'd have >> any difference in behaviour. > > > Sorry; terrible attribution on my part. > This bug clearly has nothing to do with this series. > > But a better look has me wondering how all these gpus are syncing > the framebuffer for direct access via cfb_imageblit and friends. I only see > nouveau and intel gma even trying. Probably no other DRM driver uses hardware acceleration for fbcon. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 23.01.2016 00:18, Ville Syrjälä wrote: > On Fri, Jan 22, 2016 at 12:06:00PM +0900, Michel Dänzer wrote: >> >> [ Trimming KDE folks from Cc ] >> >> On 21.01.2016 19:09, Daniel Vetter wrote: >>> On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote: >>>> On 21.01.2016 16:58, Daniel Vetter wrote: >>>>> >>>>> Can you please point me at the vblank on/off jump bug please? >>>> >>>> AFAIR I originally reported it in response to >>>> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html >>>> , but I can't find that in the archives, so maybe that was just on IRC. >>>> See >>>> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html >>>> . Basically, I ran into the bug fixed by your patch because the counter >>>> jumped forward on every DPMS off, so it hit the 32-bit boundary after >>>> just a few days. >>> >>> Ok, so just uncovered the overflow bug. >> >> Not sure what you mean by "just", but to be clear: The drm_vblank_on/off >> counter jumping bug (similar to the bug this thread is about), which >> exposed the overflow bug, is still alive and kicking in 4.5. It seems >> to happen when turning off the CRTC: >> >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=218104694, diff=0, hw=916 hw_last=916 >> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 >> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ >> 7304.307354 -> 7304.308006 [e 0 us, 0 rep] >> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=218104694, diff=16776301, hw=1 hw_last=916 > > Not sure what bug we're talking about here, but here the hw counter > clearly jumps backwards. > >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 >> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ >> 7304.317140 -> 7304.317140 [e 0 us, 0 rep] >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=234880995, diff=16777215, hw=0 hw_last=1 > > Same here. At least one of the jumps is expected, because this is around turning off the CRTC for DPMS off. Don't know yet why there are two jumps back though. > These things just don't happen on i915 because drm_vblank_off() and > drm_vblank_on() are always called around the times when the hw counter > might get reset. Or at least that's how it should be. Which is of course the idea of Daniel's patch (which is what I'm getting the above with) or Mario's patch as well, but clearly something's still wrong. It's certainly possible that it's something in the driver, but since calling drm_vblank_pre/post_modeset from the same places seems to work fine (ignoring the regression discussed in this thread)... Do drm_vblank_on/off require something else to handle this correctly? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 23.01.2016 00:18, Ville Syrjälä wrote: > On Fri, Jan 22, 2016 at 12:06:00PM +0900, Michel Dänzer wrote: >> >> [ Trimming KDE folks from Cc ] >> >> On 21.01.2016 19:09, Daniel Vetter wrote: >>> On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote: >>>> On 21.01.2016 16:58, Daniel Vetter wrote: >>>>> >>>>> Can you please point me at the vblank on/off jump bug please? >>>> >>>> AFAIR I originally reported it in response to >>>> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html >>>> , but I can't find that in the archives, so maybe that was just on IRC. >>>> See >>>> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html >>>> . Basically, I ran into the bug fixed by your patch because the counter >>>> jumped forward on every DPMS off, so it hit the 32-bit boundary after >>>> just a few days. >>> >>> Ok, so just uncovered the overflow bug. >> >> Not sure what you mean by "just", but to be clear: The drm_vblank_on/off >> counter jumping bug (similar to the bug this thread is about), which >> exposed the overflow bug, is still alive and kicking in 4.5. It seems >> to happen when turning off the CRTC: >> >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=218104694, diff=0, hw=916 hw_last=916 >> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 >> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ >> 7304.307354 -> 7304.308006 [e 0 us, 0 rep] >> [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=218104694, diff=16776301, hw=1 hw_last=916 > > Not sure what bug we're talking about here, but here the hw counter > clearly jumps backwards. > >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 >> [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, >> diff=0, hw=0 hw_last=0 >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 >> [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ >> 7304.317140 -> 7304.317140 [e 0 us, 0 rep] >> [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 >> [drm:drm_update_vblank_count] updating vblank count on crtc 0: >> current=234880995, diff=16777215, hw=0 hw_last=1 > > Same here. At least one of the jumps is expected, because this is around turning off the CRTC for DPMS off. Don't know yet why there are two jumps back though. > These things just don't happen on i915 because drm_vblank_off() and > drm_vblank_on() are always called around the times when the hw counter > might get reset. Or at least that's how it should be. Which is of course the idea of Daniel's patch (which is what I'm getting the above with) or Mario's patch as well, but clearly something's still wrong. It's certainly possible that it's something in the driver, but since calling drm_vblank_pre/post_modeset from the same places seems to work fine (ignoring the regression discussed in this thread)... Do drm_vblank_on/off require something else to handle this correctly? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
[ Trimming KDE folks from Cc ] On 21.01.2016 19:09, Daniel Vetter wrote: > On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote: >> On 21.01.2016 16:58, Daniel Vetter wrote: >>> >>> Can you please point me at the vblank on/off jump bug please? >> >> AFAIR I originally reported it in response to >> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html >> , but I can't find that in the archives, so maybe that was just on IRC. >> See >> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html >> . Basically, I ran into the bug fixed by your patch because the counter >> jumped forward on every DPMS off, so it hit the 32-bit boundary after >> just a few days. > > Ok, so just uncovered the overflow bug. Not sure what you mean by "just", but to be clear: The drm_vblank_on/off counter jumping bug (similar to the bug this thread is about), which exposed the overflow bug, is still alive and kicking in 4.5. It seems to happen when turning off the CRTC: [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=0, hw=916 hw_last=916 [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ 7304.307354 -> 7304.308006 [e 0 us, 0 rep] [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=16776301, hw=1 hw_last=916 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ 7304.317140 -> 7304.317140 [e 0 us, 0 rep] [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=234880995, diff=16777215, hw=0 hw_last=1 I suspect this may not be evident with current Intel hardware because dev->max_vblank_count = 0x, which makes the wraparound code in drm_update_vblank_count a no-op. Maybe you can reproduce it if you artificially set a lower max_vblank_count in the driver. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 16:58, Daniel Vetter wrote:
> On Thu, Jan 21, 2016 at 03:41:27PM +0900, Michel Dänzer wrote:
>> On 21.01.2016 15:38, Michel Dänzer wrote:
>>> On 21.01.2016 14:31, Mario Kleiner wrote:
>>>> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>>>>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>>>>
>>>>>> So the problem is that AMDs hardware frame counters reset to
>>>>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>>>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>>>>> count by one during each vblank irq, i think that's what
>>>>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>>>>
>>>>> Right, looks like there's been a regression breaking this. I suspect the
>>>>> problem is that vblank->last isn't getting updated from
>>>>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>>>>> to fix it. Ville?
>>>>>
>>>>
>>>> The whole logic has changed and the software counter updates are now
>>>> driven all the time by the hw counter.
>>>>
>>>>>
>>>>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>>>>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>>>>> vblank counters"). I've been meaning to track that down since then; one
>>>>> of these days hopefully, but if anybody has any ideas offhand...
>>>>
>>>> I spent the last few hours reading through the drm and radeon code and i
>>>> think what should probably work is to replace the
>>>> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
>>>> calls. These are apparently meant for drivers whose hw counters reset
>>>> during modeset, [...]
>>>
>>> ... just like drm_vblank_pre/post_modeset. That those were broken is a
>>> regression which needs to be fixed anyway. I don't think switching to
>>> drm_vblank_on/off is suitable for stable trees.
>>
>> Even more so since as I mentioned, there is (has been since at least
>> about half a year ago) a counter jumping bug with drm_vblank_on/off as well.
>
> Hm, never noticed you reported that. I thought the reason for not picking
> up my drm_vblank_on/off patches was that there's a bug in amdgpu userspace
> where it tried to use vblank waits on a disabled pipe?
http://lists.freedesktop.org/archives/dri-devel/2015-July/086451.html
I don't know why it didn't get picked up.
> Can you please point me at the vblank on/off jump bug please?
AFAIR I originally reported it in response to
http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html
, but I can't find that in the archives, so maybe that was just on IRC.
See
http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html
. Basically, I ran into the bug fixed by your patch because the counter
jumped forward on every DPMS off, so it hit the 32-bit boundary after
just a few days.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 16:58, Daniel Vetter wrote:
> On Thu, Jan 21, 2016 at 03:41:27PM +0900, Michel Dänzer wrote:
>> On 21.01.2016 15:38, Michel Dänzer wrote:
>>> On 21.01.2016 14:31, Mario Kleiner wrote:
>>>> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>>>>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>>>>
>>>>>> So the problem is that AMDs hardware frame counters reset to
>>>>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>>>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>>>>> count by one during each vblank irq, i think that's what
>>>>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>>>>
>>>>> Right, looks like there's been a regression breaking this. I suspect the
>>>>> problem is that vblank->last isn't getting updated from
>>>>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>>>>> to fix it. Ville?
>>>>>
>>>>
>>>> The whole logic has changed and the software counter updates are now
>>>> driven all the time by the hw counter.
>>>>
>>>>>
>>>>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>>>>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>>>>> vblank counters"). I've been meaning to track that down since then; one
>>>>> of these days hopefully, but if anybody has any ideas offhand...
>>>>
>>>> I spent the last few hours reading through the drm and radeon code and i
>>>> think what should probably work is to replace the
>>>> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
>>>> calls. These are apparently meant for drivers whose hw counters reset
>>>> during modeset, [...]
>>>
>>> ... just like drm_vblank_pre/post_modeset. That those were broken is a
>>> regression which needs to be fixed anyway. I don't think switching to
>>> drm_vblank_on/off is suitable for stable trees.
>>
>> Even more so since as I mentioned, there is (has been since at least
>> about half a year ago) a counter jumping bug with drm_vblank_on/off as well.
>
> Hm, never noticed you reported that. I thought the reason for not picking
> up my drm_vblank_on/off patches was that there's a bug in amdgpu userspace
> where it tried to use vblank waits on a disabled pipe?
http://lists.freedesktop.org/archives/dri-devel/2015-July/086451.html
I don't know why it didn't get picked up.
> Can you please point me at the vblank on/off jump bug please?
AFAIR I originally reported it in response to
http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html
, but I can't find that in the archives, so maybe that was just on IRC.
See
http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html
. Basically, I ran into the bug fixed by your patch because the counter
jumped forward on every DPMS off, so it hit the 32-bit boundary after
just a few days.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
[ Trimming KDE folks from Cc ] On 21.01.2016 19:09, Daniel Vetter wrote: > On Thu, Jan 21, 2016 at 05:36:46PM +0900, Michel Dänzer wrote: >> On 21.01.2016 16:58, Daniel Vetter wrote: >>> >>> Can you please point me at the vblank on/off jump bug please? >> >> AFAIR I originally reported it in response to >> http://lists.freedesktop.org/archives/dri-devel/2015-August/087841.html >> , but I can't find that in the archives, so maybe that was just on IRC. >> See >> http://lists.freedesktop.org/archives/dri-devel/2016-January/099122.html >> . Basically, I ran into the bug fixed by your patch because the counter >> jumped forward on every DPMS off, so it hit the 32-bit boundary after >> just a few days. > > Ok, so just uncovered the overflow bug. Not sure what you mean by "just", but to be clear: The drm_vblank_on/off counter jumping bug (similar to the bug this thread is about), which exposed the overflow bug, is still alive and kicking in 4.5. It seems to happen when turning off the CRTC: [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=0, hw=916 hw_last=916 [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x7 p(2199,-45)@ 7304.307354 -> 7304.308006 [e 0 us, 0 rep] [drm:radeon_get_vblank_counter_kms] crtc 0: dist from vblank start 3 [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=218104694, diff=16776301, hw=1 hw_last=916 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 1: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 2: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:radeon_get_vblank_counter_kms] Query failed! stat 3 [drm:drm_update_vblank_count] updating vblank count on crtc 3: current=0, diff=0, hw=0 hw_last=0 [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 [drm:drm_calc_vbltimestamp_from_scanoutpos] crtc 0 : v 0x1 p(0,0)@ 7304.317140 -> 7304.317140 [e 0 us, 0 rep] [drm:radeon_get_vblank_counter_kms] Query failed! stat 1 [drm:drm_update_vblank_count] updating vblank count on crtc 0: current=234880995, diff=16777215, hw=0 hw_last=1 I suspect this may not be evident with current Intel hardware because dev->max_vblank_count = 0x, which makes the wraparound code in drm_update_vblank_count a no-op. Maybe you can reproduce it if you artificially set a lower max_vblank_count in the driver. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 15:38, Michel Dänzer wrote:
> On 21.01.2016 14:31, Mario Kleiner wrote:
>> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>>
>>>> So the problem is that AMDs hardware frame counters reset to
>>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>>> count by one during each vblank irq, i think that's what
>>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>>
>>> Right, looks like there's been a regression breaking this. I suspect the
>>> problem is that vblank->last isn't getting updated from
>>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>>> to fix it. Ville?
>>>
>>
>> The whole logic has changed and the software counter updates are now
>> driven all the time by the hw counter.
>>
>>>
>>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>>> vblank counters"). I've been meaning to track that down since then; one
>>> of these days hopefully, but if anybody has any ideas offhand...
>>
>> I spent the last few hours reading through the drm and radeon code and i
>> think what should probably work is to replace the
>> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
>> calls. These are apparently meant for drivers whose hw counters reset
>> during modeset, [...]
>
> ... just like drm_vblank_pre/post_modeset. That those were broken is a
> regression which needs to be fixed anyway. I don't think switching to
> drm_vblank_on/off is suitable for stable trees.
Even more so since as I mentioned, there is (has been since at least
about half a year ago) a counter jumping bug with drm_vblank_on/off as well.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 14:31, Mario Kleiner wrote:
> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>
>>> So the problem is that AMDs hardware frame counters reset to
>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>> count by one during each vblank irq, i think that's what
>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>
>> Right, looks like there's been a regression breaking this. I suspect the
>> problem is that vblank->last isn't getting updated from
>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>> to fix it. Ville?
>>
>
> The whole logic has changed and the software counter updates are now
> driven all the time by the hw counter.
>
>>
>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>> vblank counters"). I've been meaning to track that down since then; one
>> of these days hopefully, but if anybody has any ideas offhand...
>
> I spent the last few hours reading through the drm and radeon code and i
> think what should probably work is to replace the
> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
> calls. These are apparently meant for drivers whose hw counters reset
> during modeset, [...]
... just like drm_vblank_pre/post_modeset. That those were broken is a
regression which needs to be fixed anyway. I don't think switching to
drm_vblank_on/off is suitable for stable trees.
Looking at Vlastimil's original post again, I'd say the most likely
culprit is 4dfd6486 ("drm: Use vblank timestamps to guesstimate how many
vblanks were missed").
> Once drm_vblank_off is called, drm_vblank_get will no-op and return an
> error, so clients can't enable vblank irqs during the modeset - pageflip
> ioctl and waitvblank ioctl would fail while a modeset happens -
> hopefully userspace handles this correctly everywhere.
We've fixed xf86-video-ati for this.
> I'll hack up a patch for demonstration now.
You're a bit late to that party. :)
http://lists.freedesktop.org/archives/dri-devel/2015-May/083614.html
http://lists.freedesktop.org/archives/dri-devel/2015-July/086451.html
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 05:32, Mario Kleiner wrote:
>
> So the problem is that AMDs hardware frame counters reset to
> zero during a modeset. The old DRM code dealt with drivers doing that by
> keeping vblank irqs enabled during modesets and incrementing vblank
> count by one during each vblank irq, i think that's what
> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
Right, looks like there's been a regression breaking this. I suspect the
problem is that vblank->last isn't getting updated from
drm_vblank_post_modeset. Not sure which change broke that though, or how
to fix it. Ville?
BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
vblank counters"). I've been meaning to track that down since then; one
of these days hopefully, but if anybody has any ideas offhand...
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 15:38, Michel Dänzer wrote:
> On 21.01.2016 14:31, Mario Kleiner wrote:
>> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>>
>>>> So the problem is that AMDs hardware frame counters reset to
>>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>>> count by one during each vblank irq, i think that's what
>>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>>
>>> Right, looks like there's been a regression breaking this. I suspect the
>>> problem is that vblank->last isn't getting updated from
>>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>>> to fix it. Ville?
>>>
>>
>> The whole logic has changed and the software counter updates are now
>> driven all the time by the hw counter.
>>
>>>
>>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>>> vblank counters"). I've been meaning to track that down since then; one
>>> of these days hopefully, but if anybody has any ideas offhand...
>>
>> I spent the last few hours reading through the drm and radeon code and i
>> think what should probably work is to replace the
>> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
>> calls. These are apparently meant for drivers whose hw counters reset
>> during modeset, [...]
>
> ... just like drm_vblank_pre/post_modeset. That those were broken is a
> regression which needs to be fixed anyway. I don't think switching to
> drm_vblank_on/off is suitable for stable trees.
Even more so since as I mentioned, there is (has been since at least
about half a year ago) a counter jumping bug with drm_vblank_on/off as well.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 05:32, Mario Kleiner wrote:
>
> So the problem is that AMDs hardware frame counters reset to
> zero during a modeset. The old DRM code dealt with drivers doing that by
> keeping vblank irqs enabled during modesets and incrementing vblank
> count by one during each vblank irq, i think that's what
> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
Right, looks like there's been a regression breaking this. I suspect the
problem is that vblank->last isn't getting updated from
drm_vblank_post_modeset. Not sure which change broke that though, or how
to fix it. Ville?
BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
vblank counters"). I've been meaning to track that down since then; one
of these days hopefully, but if anybody has any ideas offhand...
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: linux-4.4 bisected: kwin5 stuck on kde5 loading screen with radeon
On 21.01.2016 14:31, Mario Kleiner wrote:
> On 01/21/2016 04:43 AM, Michel Dänzer wrote:
>> On 21.01.2016 05:32, Mario Kleiner wrote:
>>>
>>> So the problem is that AMDs hardware frame counters reset to
>>> zero during a modeset. The old DRM code dealt with drivers doing that by
>>> keeping vblank irqs enabled during modesets and incrementing vblank
>>> count by one during each vblank irq, i think that's what
>>> drm_vblank_pre_modeset() and drm_vblank_post_modeset() were meant for.
>>
>> Right, looks like there's been a regression breaking this. I suspect the
>> problem is that vblank->last isn't getting updated from
>> drm_vblank_post_modeset. Not sure which change broke that though, or how
>> to fix it. Ville?
>>
>
> The whole logic has changed and the software counter updates are now
> driven all the time by the hw counter.
>
>>
>> BTW, I'm seeing a similar issue with drm_vblank_on/off as well, which
>> exposed the bug fixed by 209e4dbc ("drm/vblank: Use u32 consistently for
>> vblank counters"). I've been meaning to track that down since then; one
>> of these days hopefully, but if anybody has any ideas offhand...
>
> I spent the last few hours reading through the drm and radeon code and i
> think what should probably work is to replace the
> drm_vblank_pre/post_modeset calls in radeon/amdgpu by drm_vblank_off/on
> calls. These are apparently meant for drivers whose hw counters reset
> during modeset, [...]
... just like drm_vblank_pre/post_modeset. That those were broken is a
regression which needs to be fixed anyway. I don't think switching to
drm_vblank_on/off is suitable for stable trees.
Looking at Vlastimil's original post again, I'd say the most likely
culprit is 4dfd6486 ("drm: Use vblank timestamps to guesstimate how many
vblanks were missed").
> Once drm_vblank_off is called, drm_vblank_get will no-op and return an
> error, so clients can't enable vblank irqs during the modeset - pageflip
> ioctl and waitvblank ioctl would fail while a modeset happens -
> hopefully userspace handles this correctly everywhere.
We've fixed xf86-video-ati for this.
> I'll hack up a patch for demonstration now.
You're a bit late to that party. :)
http://lists.freedesktop.org/archives/dri-devel/2015-May/083614.html
http://lists.freedesktop.org/archives/dri-devel/2015-July/086451.html
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
Re: [PATCH 3/9] drm/vc4: Add create and map BO ioctls.
On 08.12.2015 10:16, Eric Anholt wrote: > Emil Velikov writes: > >> Hi Eric, >> >> A couple of suggestions/requests on the UAPI header side >> >> On 1 December 2015 at 20:35, Eric Anholt wrote: >> >>> --- /dev/null >>> +++ b/include/uapi/drm/vc4_drm.h >> >>> +#include >>> + >> Can we make this a "drm.h" ? > > Nope. > > include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or > directory What happened to include/uapi/drm/drm.h in that tree? > and none of the drivers do, either. daenzer@kaveri|11:55:31> grep '#include "drm.h"' include/uapi/drm/* include/uapi/drm/amdgpu_drm.h:#include "drm.h" include/uapi/drm/radeon_drm.h:#include "drm.h" Patches to convert the rest are pending. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/9] drm/vc4: Add create and map BO ioctls.
On 08.12.2015 10:16, Eric Anholt wrote: > Emil Velikov <emil.l.veli...@gmail.com> writes: > >> Hi Eric, >> >> A couple of suggestions/requests on the UAPI header side >> >> On 1 December 2015 at 20:35, Eric Anholt <e...@anholt.net> wrote: >> >>> --- /dev/null >>> +++ b/include/uapi/drm/vc4_drm.h >> >>> +#include >>> + >> Can we make this a "drm.h" ? > > Nope. > > include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or > directory What happened to include/uapi/drm/drm.h in that tree? > and none of the drivers do, either. daenzer@kaveri|11:55:31> grep '#include "drm.h"' include/uapi/drm/* include/uapi/drm/amdgpu_drm.h:#include "drm.h" include/uapi/drm/radeon_drm.h:#include "drm.h" Patches to convert the rest are pending. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [radeon r100] when ring test fails, provide users with option to test
On 01.12.2015 19:01, Pavel Machek wrote: > On Mon 2015-11-30 09:39:54, Christian König wrote: >> On 29.11.2015 23:22, Pavel Machek wrote: >>> On Sun 2015-11-29 20:48:53, Christian König wrote: >>>> On 28.11.2015 21:58, Pavel Machek wrote: >>>>> Ring test failure is often caused by too high agpmode. Tell the user >>>>> what to try. >>>>> >>>>> Signed-off-by: Pavel Machek >>>> NAK, the ring test can fail for any number of reasons and the agpmode is >>>> actually rather unlikely to be the cause. >>> Well, when I asked on the list "why this is happened" I got "umm, >>> noone knows" response that was not exactly helpful. And then someone >>> told me about agpmode. >>> >>> If you know about the reasons it can fail, could you list them near >>> the DRM_ERROR, at least as a comment? >> >> Well as I said, that could be any number of reasons. Some of them even >> completely unrelated to the driver itself. >> >> E.g. BIOS setting, faulty hardware, problems with the writeback etc... There >> is really not a list you could give here. >> >> Lowering the agpmode usually helps more to prevent random corruptions and >> problems under load. > > Take a look at > > http://www.gossamer-threads.com/lists/linux/kernel/2197183 > > . I had a problem, you did not know how to debug it, but it already > happened to pebolle at tiscali ... and yes, it was agpmode. That > problem is clearly more common then you realize... So this should go > in. I agree with Christian, but at the very least, agpmode must not be mentioned if AGP isn't being used in the first place, i.e. either the GPU isn't AGP or is being forced to PCI(e) mode. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [radeon r100] when ring test fails, provide users with option to test
On 01.12.2015 19:01, Pavel Machek wrote: > On Mon 2015-11-30 09:39:54, Christian König wrote: >> On 29.11.2015 23:22, Pavel Machek wrote: >>> On Sun 2015-11-29 20:48:53, Christian König wrote: >>>> On 28.11.2015 21:58, Pavel Machek wrote: >>>>> Ring test failure is often caused by too high agpmode. Tell the user >>>>> what to try. >>>>> >>>>> Signed-off-by: Pavel Machek <pa...@ucw.cz> >>>> NAK, the ring test can fail for any number of reasons and the agpmode is >>>> actually rather unlikely to be the cause. >>> Well, when I asked on the list "why this is happened" I got "umm, >>> noone knows" response that was not exactly helpful. And then someone >>> told me about agpmode. >>> >>> If you know about the reasons it can fail, could you list them near >>> the DRM_ERROR, at least as a comment? >> >> Well as I said, that could be any number of reasons. Some of them even >> completely unrelated to the driver itself. >> >> E.g. BIOS setting, faulty hardware, problems with the writeback etc... There >> is really not a list you could give here. >> >> Lowering the agpmode usually helps more to prevent random corruptions and >> problems under load. > > Take a look at > > http://www.gossamer-threads.com/lists/linux/kernel/2197183 > > . I had a problem, you did not know how to debug it, but it already > happened to pebolle at tiscali ... and yes, it was agpmode. That > problem is clearly more common then you realize... So this should go > in. I agree with Christian, but at the very least, agpmode must not be mentioned if AGP isn't being used in the first place, i.e. either the GPU isn't AGP or is being forced to PCI(e) mode. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] drm: support hotspot for universal plane cursors
On 18.11.2015 17:51, Daniel Vetter wrote: > On Wed, Nov 18, 2015 at 05:39:39PM +0900, Michel Dänzer wrote: >> On 18.11.2015 01:29, Daniel Vetter wrote: >>> >>> And no, I have absolutely no idea why radeon is pulling some tricks here, >>> which have been added in >>> >>> commit 78b1a6010b46a69bcd47b723a80f92693f26d17b >>> Author: Michel Dänzer >>> Date: Tue Nov 18 18:00:08 2014 +0900 >>> >>> drm/radeon: Use cursor_set2 hook for enabling / disabling the HW cursor >>> >>> Michel/Alex, can you please shed some light onto this? >> >> As described in the rest of the commit log, the intention was to avoid >> the cursor intermittently appearing in the wrong location with existing >> userspace which sets the cursor BO in one ioctl call and the new >> position in another ioctl call. >> >> >>> radeon is the only driver doing this, making this interface inconsistent. >> >> It's only inconsistent in the case that userspace updates the cursor >> position to account for the new hotspot position in one ioctl call >> first, and only then sets the new BO in another ioctl call. In all other >> cases, the cursor position passed in by userspace is preserved. >> >> Anyway, in the meantime it has become apparent that this change didn't >> fully fix the problem, so feel free to revert it. > > Yeah I read the commit message but didn't understand what it's doing. > After some discussion with Alex on irc I realized that the fixup is only > applied in when updating the cursor bo and changing the hotspot to avoid > that kind of flickering. That problem is solved though on the kernel side > with universal planes (where we don't artificially split up the cursor > update into a move + bo-update for the driver interface any more). And > it's fixable in userspace even with legacy cursor interfaces since the > ioctl allows you to move + update at the same time too. It's just that X > doesn't provide that interface to the driver in a useful way. Well, the legacy cursor interfaces currently don't allow the driver to prevent the hardware from updating the cursor between the cursor_set / cursor_move calls. Anyway, I tried adding a cursor_lock hook for that purpose and adapting userspace accordingly, but it still doesn't seem to fully fix the problem. So I'm leaving it to somebody else / another day. :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] drm: support hotspot for universal plane cursors
On 18.11.2015 01:29, Daniel Vetter wrote:
> On Tue, Nov 17, 2015 at 03:59:43PM +, John Keeping wrote:
>> On Tue, 17 Nov 2015 17:39:32 +0200, Ville Syrjälä wrote:
>>
>>> On Tue, Nov 17, 2015 at 03:05:34PM +, John Keeping wrote:
>>>> The request's hot_x and hot_y are set correctly for both
>>>> DRM_IOCTL_MODE_CURSOR and DRM_IOCTL_MODE_CURSOR2 so we just need to
>>>> save the values and then apply the offset to the cursor plane when
>>>> the cursor moves.
>>>>
>>>> Signed-off-by: John Keeping
>>>> ---
>>>> v2:
>>>> - add kerneldoc for hot_x and hot_y in struct drm_crtc
>>>>
>>>> drivers/gpu/drm/drm_crtc.c | 11 +++
>>>> include/drm/drm_crtc.h | 6 ++
>>>> 2 files changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>>> index 720a153..40f5b84 100644
>>>> --- a/drivers/gpu/drm/drm_crtc.c
>>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>>> @@ -2831,6 +2831,9 @@ static int drm_mode_cursor_universal(struct
>>>> drm_crtc *crtc, DRM_DEBUG_KMS("failed to wrap cursor buffer in drm
>>>> framebuffer\n"); return PTR_ERR(fb);
>>>>}
>>>> +
>>>> + crtc->hot_x = req->hot_x;
>>>> + crtc->hot_y = req->hot_y;
>>>>} else {
>>>>fb = NULL;
>>>>}
>>>> @@ -2841,11 +2844,11 @@ static int drm_mode_cursor_universal(struct
>>>> drm_crtc *crtc, }
>>>>
>>>>if (req->flags & DRM_MODE_CURSOR_MOVE) {
>>>> - crtc_x = req->x;
>>>> - crtc_y = req->y;
>>>> + crtc_x = req->x - crtc->hot_x;
>>>> + crtc_y = req->y - crtc->hot_y;
>>>>} else {
>>>> - crtc_x = crtc->cursor_x;
>>>> - crtc_y = crtc->cursor_y;
>>>> + crtc_x = crtc->cursor_x - crtc->hot_x;
>>>> + crtc_y = crtc->cursor_y - crtc->hot_y;
>>>
>>> Why does the location of the hotspot affect the plane position?
>>
>> hot_{x,y} specify the location of the active pixel within the cursor
>> plane and cursor_{x,y} specify the location of the active pixel on the
>> display so we need to offset the plane position in order for the active
>> pixel to be in the correct place.
>
> Nope, hot_x/y is just for virtual machines to indicate where the logical
> cursor position is within the cursor plane. It should have 2 effect on how
> something is displayed.
Agreed: Since the DRM_IOCTL_MODE_CURSOR ioctl doesn't contain any
information about the hotspot, the x/y coordinates passed in the
DRM_IOCTL_MODE_CURSOR(2) ioctls can only refer to the position of the
top left corner of the cursor.
> And no, I have absolutely no idea why radeon is pulling some tricks here,
> which have been added in
>
> commit 78b1a6010b46a69bcd47b723a80f92693f26d17b
> Author: Michel Dänzer
> Date: Tue Nov 18 18:00:08 2014 +0900
>
> drm/radeon: Use cursor_set2 hook for enabling / disabling the HW cursor
>
> Michel/Alex, can you please shed some light onto this?
As described in the rest of the commit log, the intention was to avoid
the cursor intermittently appearing in the wrong location with existing
userspace which sets the cursor BO in one ioctl call and the new
position in another ioctl call.
> radeon is the only driver doing this, making this interface inconsistent.
It's only inconsistent in the case that userspace updates the cursor
position to account for the new hotspot position in one ioctl call
first, and only then sets the new BO in another ioctl call. In all other
cases, the cursor position passed in by userspace is preserved.
Anyway, in the meantime it has become apparent that this change didn't
fully fix the problem, so feel free to revert it.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] drm: support hotspot for universal plane cursors
On 18.11.2015 01:29, Daniel Vetter wrote:
> On Tue, Nov 17, 2015 at 03:59:43PM +, John Keeping wrote:
>> On Tue, 17 Nov 2015 17:39:32 +0200, Ville Syrjälä wrote:
>>
>>> On Tue, Nov 17, 2015 at 03:05:34PM +, John Keeping wrote:
>>>> The request's hot_x and hot_y are set correctly for both
>>>> DRM_IOCTL_MODE_CURSOR and DRM_IOCTL_MODE_CURSOR2 so we just need to
>>>> save the values and then apply the offset to the cursor plane when
>>>> the cursor moves.
>>>>
>>>> Signed-off-by: John Keeping <j...@metanate.com>
>>>> ---
>>>> v2:
>>>> - add kerneldoc for hot_x and hot_y in struct drm_crtc
>>>>
>>>> drivers/gpu/drm/drm_crtc.c | 11 +++
>>>> include/drm/drm_crtc.h | 6 ++
>>>> 2 files changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>>> index 720a153..40f5b84 100644
>>>> --- a/drivers/gpu/drm/drm_crtc.c
>>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>>> @@ -2831,6 +2831,9 @@ static int drm_mode_cursor_universal(struct
>>>> drm_crtc *crtc, DRM_DEBUG_KMS("failed to wrap cursor buffer in drm
>>>> framebuffer\n"); return PTR_ERR(fb);
>>>>}
>>>> +
>>>> + crtc->hot_x = req->hot_x;
>>>> + crtc->hot_y = req->hot_y;
>>>>} else {
>>>>fb = NULL;
>>>>}
>>>> @@ -2841,11 +2844,11 @@ static int drm_mode_cursor_universal(struct
>>>> drm_crtc *crtc, }
>>>>
>>>>if (req->flags & DRM_MODE_CURSOR_MOVE) {
>>>> - crtc_x = req->x;
>>>> - crtc_y = req->y;
>>>> + crtc_x = req->x - crtc->hot_x;
>>>> + crtc_y = req->y - crtc->hot_y;
>>>>} else {
>>>> - crtc_x = crtc->cursor_x;
>>>> - crtc_y = crtc->cursor_y;
>>>> + crtc_x = crtc->cursor_x - crtc->hot_x;
>>>> + crtc_y = crtc->cursor_y - crtc->hot_y;
>>>
>>> Why does the location of the hotspot affect the plane position?
>>
>> hot_{x,y} specify the location of the active pixel within the cursor
>> plane and cursor_{x,y} specify the location of the active pixel on the
>> display so we need to offset the plane position in order for the active
>> pixel to be in the correct place.
>
> Nope, hot_x/y is just for virtual machines to indicate where the logical
> cursor position is within the cursor plane. It should have 2 effect on how
> something is displayed.
Agreed: Since the DRM_IOCTL_MODE_CURSOR ioctl doesn't contain any
information about the hotspot, the x/y coordinates passed in the
DRM_IOCTL_MODE_CURSOR(2) ioctls can only refer to the position of the
top left corner of the cursor.
> And no, I have absolutely no idea why radeon is pulling some tricks here,
> which have been added in
>
> commit 78b1a6010b46a69bcd47b723a80f92693f26d17b
> Author: Michel Dänzer <michel.daen...@amd.com>
> Date: Tue Nov 18 18:00:08 2014 +0900
>
> drm/radeon: Use cursor_set2 hook for enabling / disabling the HW cursor
>
> Michel/Alex, can you please shed some light onto this?
As described in the rest of the commit log, the intention was to avoid
the cursor intermittently appearing in the wrong location with existing
userspace which sets the cursor BO in one ioctl call and the new
position in another ioctl call.
> radeon is the only driver doing this, making this interface inconsistent.
It's only inconsistent in the case that userspace updates the cursor
position to account for the new hotspot position in one ioctl call
first, and only then sets the new BO in another ioctl call. In all other
cases, the cursor position passed in by userspace is preserved.
Anyway, in the meantime it has become apparent that this change didn't
fully fix the problem, so feel free to revert it.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] drm: support hotspot for universal plane cursors
On 18.11.2015 17:51, Daniel Vetter wrote: > On Wed, Nov 18, 2015 at 05:39:39PM +0900, Michel Dänzer wrote: >> On 18.11.2015 01:29, Daniel Vetter wrote: >>> >>> And no, I have absolutely no idea why radeon is pulling some tricks here, >>> which have been added in >>> >>> commit 78b1a6010b46a69bcd47b723a80f92693f26d17b >>> Author: Michel Dänzer <michel.daen...@amd.com> >>> Date: Tue Nov 18 18:00:08 2014 +0900 >>> >>> drm/radeon: Use cursor_set2 hook for enabling / disabling the HW cursor >>> >>> Michel/Alex, can you please shed some light onto this? >> >> As described in the rest of the commit log, the intention was to avoid >> the cursor intermittently appearing in the wrong location with existing >> userspace which sets the cursor BO in one ioctl call and the new >> position in another ioctl call. >> >> >>> radeon is the only driver doing this, making this interface inconsistent. >> >> It's only inconsistent in the case that userspace updates the cursor >> position to account for the new hotspot position in one ioctl call >> first, and only then sets the new BO in another ioctl call. In all other >> cases, the cursor position passed in by userspace is preserved. >> >> Anyway, in the meantime it has become apparent that this change didn't >> fully fix the problem, so feel free to revert it. > > Yeah I read the commit message but didn't understand what it's doing. > After some discussion with Alex on irc I realized that the fixup is only > applied in when updating the cursor bo and changing the hotspot to avoid > that kind of flickering. That problem is solved though on the kernel side > with universal planes (where we don't artificially split up the cursor > update into a move + bo-update for the driver interface any more). And > it's fixable in userspace even with legacy cursor interfaces since the > ioctl allows you to move + update at the same time too. It's just that X > doesn't provide that interface to the driver in a useful way. Well, the legacy cursor interfaces currently don't allow the driver to prevent the hardware from updating the cursor between the cursor_set / cursor_move calls. Anyway, I tried adding a cursor_lock hook for that purpose and adapting userspace accordingly, but it still doesn't seem to fully fix the problem. So I'm leaving it to somebody else / another day. :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Mobility Radeon HD 4530/4570/545v: flicker in 1920x1080
On 06.11.2015 05:23, Pavel Machek wrote: > Hi! > >>>> The flickering would vanish completely if that's the reason for the issue >>>> you are seeing. >>> >>>> Try setting ref_div_min and ref_div_max to 2 in >>>> radeon_compute_pll_avivo(). >>> >>> Ok, I did this, but no luck, still flickers. But the flicker only >>> happens when something changes on screen, like dragging a big >>> window. Is that consistent with wrong PLL timings? >> >> Does it go away with radeon.dpm=0? Sounds more like either memory >> reclocking happening outside of vblank, or underflow to the display >> controllers. > > No, it does not: > > pavel@half:~$ cat /proc/cmdline > BOOT_IMAGE=(hd0,2)/l/linux/arch/x86/boot/bzImage root=/dev/sda4 > resume=/dev/sda1 radeon.dpm=0 > > ..and same issue. And yes, it looks like an underflow to me. How can I > debug reclocking / underflows? Does radeon.disp_priority=2 help? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Mobility Radeon HD 4530/4570/545v: flicker in 1920x1080
On 06.11.2015 05:23, Pavel Machek wrote: > Hi! > >>>> The flickering would vanish completely if that's the reason for the issue >>>> you are seeing. >>> >>>> Try setting ref_div_min and ref_div_max to 2 in >>>> radeon_compute_pll_avivo(). >>> >>> Ok, I did this, but no luck, still flickers. But the flicker only >>> happens when something changes on screen, like dragging a big >>> window. Is that consistent with wrong PLL timings? >> >> Does it go away with radeon.dpm=0? Sounds more like either memory >> reclocking happening outside of vblank, or underflow to the display >> controllers. > > No, it does not: > > pavel@half:~$ cat /proc/cmdline > BOOT_IMAGE=(hd0,2)/l/linux/arch/x86/boot/bzImage root=/dev/sda4 > resume=/dev/sda1 radeon.dpm=0 > > ..and same issue. And yes, it looks like an underflow to me. How can I > debug reclocking / underflows? Does radeon.disp_priority=2 help? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.16.y-ckt 072/104] drm/radeon: Restore LCD backlight level on resume (>= R5xx)
On 28.10.2015 19:45, Luis Henriques wrote: > On Tue, Oct 27, 2015 at 11:10:29AM +0900, Michel Dänzer wrote: >> On 26.10.2015 22:42, Luis Henriques wrote: >>> 3.16.7-ckt19 -stable review patch. If anyone has any objections, please >>> let me know. >>> >>> ------ >>> >>> From: =TF-8?q?Michel Dänzer?= >>> >>> commit 4281f46ef839050d2ef60348f661eb463c21cc2e upstream. >>> >>> Instead of only enabling the backlight (which seems to set it to max >>> brightness), just re-set the current backlight level, which also takes >>> care of enabling the backlight if necessary. >>> >>> Only the radeon_atom_encoder_dpms_dig part tested on a Kaveri laptop, >>> the radeon_atom_encoder_dpms_avivo part is only compile tested. >>> >>> Signed-off-by: Michel Dänzer >>> Signed-off-by: Alex Deucher >>> Signed-off-by: Luis Henriques >> >> We're currently investigating a regression which was bisected to this >> change. Please hold off on backporting this change until we have a >> solution for that. > > Thanks, I'll drop this patch for this 3.16 release, and will wait for a > fix to be available. The regression is fixed in commits 4cee6a9057d5e13911f0cb6e143d11dc1a3245dd and ae93580ee59c02395c1711d3e6b90546b8137b86 . Please only backport 4281f46ef839050d2ef60348f661eb463c21cc2e together with those two. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.16.y-ckt 072/104] drm/radeon: Restore LCD backlight level on resume (>= R5xx)
On 28.10.2015 19:45, Luis Henriques wrote: > On Tue, Oct 27, 2015 at 11:10:29AM +0900, Michel Dänzer wrote: >> On 26.10.2015 22:42, Luis Henriques wrote: >>> 3.16.7-ckt19 -stable review patch. If anyone has any objections, please >>> let me know. >>> >>> ------ >>> >>> From: =TF-8?q?Michel Dänzer?= <michel.daen...@amd.com> >>> >>> commit 4281f46ef839050d2ef60348f661eb463c21cc2e upstream. >>> >>> Instead of only enabling the backlight (which seems to set it to max >>> brightness), just re-set the current backlight level, which also takes >>> care of enabling the backlight if necessary. >>> >>> Only the radeon_atom_encoder_dpms_dig part tested on a Kaveri laptop, >>> the radeon_atom_encoder_dpms_avivo part is only compile tested. >>> >>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com> >>> Signed-off-by: Alex Deucher <alexander.deuc...@amd.com> >>> Signed-off-by: Luis Henriques <luis.henriq...@canonical.com> >> >> We're currently investigating a regression which was bisected to this >> change. Please hold off on backporting this change until we have a >> solution for that. > > Thanks, I'll drop this patch for this 3.16 release, and will wait for a > fix to be available. The regression is fixed in commits 4cee6a9057d5e13911f0cb6e143d11dc1a3245dd and ae93580ee59c02395c1711d3e6b90546b8137b86 . Please only backport 4281f46ef839050d2ef60348f661eb463c21cc2e together with those two. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [Regression] 4.3.0rc4 through rc7: No LCD backlight on Thinkpad T60P
On 29.10.2015 23:26, Simon Wood wrote: > On Thu, October 29, 2015 8:07 am, Simon Wood wrote: >> > >>> Well, I can confirm that the patch stopped the Oops - but unfortunatly >>> the screen is still dark. >>> >>> Also, Oops _does_ happen with rc2 but the screens is OK with that... >>> Back >>> to looking through the logs. >> >> I couldn't see anything of note in the syslog, so attempted to bisect the >> problem some - although building takes considerable time on this machine >> (note 32bit). >> >> >> 4.3.0rc6 - bad >> 4.3.0rc7 - bad >> 4.3.0rc2 - good >> 4.3.0rc4 - bad (049e6dde7e57f0054fdc49102e7ef4830c698b46) >> 4.3.0rc3 - good (9ffecb10283508260936b96022d4ee43a7798b4c) >> >> >> So problem (no LCD backlight) must have been introduced between those. > > Well this looks like it might have something to do with it will > attempt a build just before it. > -- > commit 4281f46ef839050d2ef60348f661eb463c21cc2e > Author: Michel Dänzer > Date: Mon Sep 28 18:16:31 2015 +0900 > > drm/radeon: Restore LCD backlight level on resume (>= R5xx) Please try the patches from http://lists.freedesktop.org/archives/dri-devel/2015-October/093514.html . -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [Regression] 4.3.0rc4 through rc7: No LCD backlight on Thinkpad T60P
On 29.10.2015 23:26, Simon Wood wrote: > On Thu, October 29, 2015 8:07 am, Simon Wood wrote: >> > >>> Well, I can confirm that the patch stopped the Oops - but unfortunatly >>> the screen is still dark. >>> >>> Also, Oops _does_ happen with rc2 but the screens is OK with that... >>> Back >>> to looking through the logs. >> >> I couldn't see anything of note in the syslog, so attempted to bisect the >> problem some - although building takes considerable time on this machine >> (note 32bit). >> >> >> 4.3.0rc6 - bad >> 4.3.0rc7 - bad >> 4.3.0rc2 - good >> 4.3.0rc4 - bad (049e6dde7e57f0054fdc49102e7ef4830c698b46) >> 4.3.0rc3 - good (9ffecb10283508260936b96022d4ee43a7798b4c) >> >> >> So problem (no LCD backlight) must have been introduced between those. > > Well this looks like it might have something to do with it will > attempt a build just before it. > -- > commit 4281f46ef839050d2ef60348f661eb463c21cc2e > Author: Michel Dänzer <michel.daen...@amd.com> > Date: Mon Sep 28 18:16:31 2015 +0900 > > drm/radeon: Restore LCD backlight level on resume (>= R5xx) Please try the patches from http://lists.freedesktop.org/archives/dri-devel/2015-October/093514.html . -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.16.y-ckt 072/104] drm/radeon: Restore LCD backlight level on resume (>= R5xx)
On 26.10.2015 22:42, Luis Henriques wrote: > 3.16.7-ckt19 -stable review patch. If anyone has any objections, please let > me know. > > -- > > From: =TF-8?q?Michel Dänzer?= > > commit 4281f46ef839050d2ef60348f661eb463c21cc2e upstream. > > Instead of only enabling the backlight (which seems to set it to max > brightness), just re-set the current backlight level, which also takes > care of enabling the backlight if necessary. > > Only the radeon_atom_encoder_dpms_dig part tested on a Kaveri laptop, > the radeon_atom_encoder_dpms_avivo part is only compile tested. > > Signed-off-by: Michel Dänzer > Signed-off-by: Alex Deucher > Signed-off-by: Luis Henriques We're currently investigating a regression which was bisected to this change. Please hold off on backporting this change until we have a solution for that. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.16.y-ckt 072/104] drm/radeon: Restore LCD backlight level on resume (>= R5xx)
On 26.10.2015 22:42, Luis Henriques wrote: > 3.16.7-ckt19 -stable review patch. If anyone has any objections, please let > me know. > > -- > > From: =TF-8?q?Michel Dänzer?= <michel.daen...@amd.com> > > commit 4281f46ef839050d2ef60348f661eb463c21cc2e upstream. > > Instead of only enabling the backlight (which seems to set it to max > brightness), just re-set the current backlight level, which also takes > care of enabling the backlight if necessary. > > Only the radeon_atom_encoder_dpms_dig part tested on a Kaveri laptop, > the radeon_atom_encoder_dpms_avivo part is only compile tested. > > Signed-off-by: Michel Dänzer <michel.daen...@amd.com> > Signed-off-by: Alex Deucher <alexander.deuc...@amd.com> > Signed-off-by: Luis Henriques <luis.henriq...@canonical.com> We're currently investigating a regression which was bisected to this change. Please hold off on backporting this change until we have a solution for that. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Regression v4.2] Re: [PATCH 7/9] drm/radeon: add VCE 1.0 support v4
On 13.08.2015 15:03, Lucas Stach wrote: > Hi Christian, > > this commit is causing a boot regression with v4.2-rcX on my Richland > APU (CHIP_ARUBA) based laptop. I didn't have time yet to track down > where exactly it is going wrong, but I bisected it down to this single > commit. > > I don't have the VCE firmware installed on this system, so from a quick > look at the code I would expect it to drop out pretty early and just > leave the VCE unconfigured, but otherwise keep things working as before. > This is unfortunately not the case. If the radeon driver is built into the kernel (or loaded from the initrd?), the attempt to load the firmware might take a long time to time out. Please provide more information about the symptoms, e.g. any dmesg output etc. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Regression v4.2] Re: [PATCH 7/9] drm/radeon: add VCE 1.0 support v4
On 13.08.2015 15:03, Lucas Stach wrote: Hi Christian, this commit is causing a boot regression with v4.2-rcX on my Richland APU (CHIP_ARUBA) based laptop. I didn't have time yet to track down where exactly it is going wrong, but I bisected it down to this single commit. I don't have the VCE firmware installed on this system, so from a quick look at the code I would expect it to drop out pretty early and just leave the VCE unconfigured, but otherwise keep things working as before. This is unfortunately not the case. If the radeon driver is built into the kernel (or loaded from the initrd?), the attempt to load the firmware might take a long time to time out. Please provide more information about the symptoms, e.g. any dmesg output etc. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On 14.07.2015 22:41, Sergey Senozhatsky wrote: > > sometimes `xset dpms force off' just turns off the panel for a second, > sometimes -- until I generate a `wakeup' event (key press, etc.) FWIW, the former case is because releasing the enter key generates an input event, which changes the DPMS state to on again. You can avoid that with something like "sleep 1 && xset dpms force off". -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On 14.07.2015 22:41, Sergey Senozhatsky wrote: sometimes `xset dpms force off' just turns off the panel for a second, sometimes -- until I generate a `wakeup' event (key press, etc.) FWIW, the former case is because releasing the enter key generates an input event, which changes the DPMS state to on again. You can avoid that with something like sleep 1 xset dpms force off. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] gpu/drm: remove unnecessary check before kfree
On 26.06.2015 15:25, Maninder Singh wrote: > kfree(NULL) is safe and this check is probably not required > > Signed-off-by: Maninder Singh > Reviewed-by: Vaneet Narang > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index fec487d..a85cd08 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1575,8 +1575,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) > amdgpu_fence_driver_fini(adev); > amdgpu_fbdev_fini(adev); > r = amdgpu_fini(adev); > - if (adev->ip_block_enabled) > - kfree(adev->ip_block_enabled); > + kfree(adev->ip_block_enabled); > adev->ip_block_enabled = NULL; > adev->accel_working = false; > /* free i2c buses */ > The shortlog prefix of both your patches should be "drm/amdgpu:" instead of "gpu/drm:". With that fixed, both are Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] gpu/drm: remove unnecessary check before kfree
On 26.06.2015 15:25, Maninder Singh wrote: kfree(NULL) is safe and this check is probably not required Signed-off-by: Maninder Singh maninder...@samsung.com Reviewed-by: Vaneet Narang v.nar...@samsung.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index fec487d..a85cd08 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1575,8 +1575,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_fence_driver_fini(adev); amdgpu_fbdev_fini(adev); r = amdgpu_fini(adev); - if (adev-ip_block_enabled) - kfree(adev-ip_block_enabled); + kfree(adev-ip_block_enabled); adev-ip_block_enabled = NULL; adev-accel_working = false; /* free i2c buses */ The shortlog prefix of both your patches should be drm/amdgpu: instead of gpu/drm:. With that fixed, both are Reviewed-by: Michel Dänzer michel.daen...@amd.com -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
