Re: [PATCH 2/2] drm/amdgpu: fix cursor setting of dce6/dce8

[Michel Dänzer]

On 14/12/16 03:45 PM, Flora Cui wrote:
> Change-Id: Ic900051ece1bf7aaf01b3811dde4cbe426be8b42
> Signed-off-by: Flora Cui 
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 --
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index b3846c2..ca1bcf7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1944,9 +1944,7 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc 
> *crtc,
>  
>   dce_v6_0_lock_cursor(crtc, true);
>  
> - if (width != amdgpu_crtc->cursor_width ||
> - height != amdgpu_crtc->cursor_height ||
> - hot_x != amdgpu_crtc->cursor_hot_x ||
> + if (hot_x != amdgpu_crtc->cursor_hot_x ||
>   hot_y != amdgpu_crtc->cursor_hot_y) {
>   int x, y;
>  
> @@ -1955,8 +1953,6 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc 
> *crtc,
>  
>   dce_v6_0_cursor_move_locked(crtc, x, y);
>  
> - amdgpu_crtc->cursor_width = width;
> - amdgpu_crtc->cursor_height = height;
>   amdgpu_crtc->cursor_hot_x = hot_x;
>   amdgpu_crtc->cursor_hot_y = hot_y;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index c0abadf..fb7c0e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2437,8 +2437,6 @@ static int dce_v8_0_crtc_cursor_set2(struct drm_crtc 
> *crtc,
>  
>   dce_v8_0_cursor_move_locked(crtc, x, y);
>  
> - amdgpu_crtc->cursor_width = width;
> - amdgpu_crtc->cursor_height = height;
>   amdgpu_crtc->cursor_hot_x = hot_x;
>   amdgpu_crtc->cursor_hot_y = hot_y;
>   }
> 

Good catch, thanks Flora! Please add

Fixes: 7c83d7abc999 ("drm/amdgpu: Only update the CUR_SIZE register when
necessary")

to get

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: fix cursor setting of dce6/dce8

[Flora Cui]

Change-Id: Ic900051ece1bf7aaf01b3811dde4cbe426be8b42
Signed-off-by: Flora Cui 
---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 --
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index b3846c2..ca1bcf7 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1944,9 +1944,7 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc 
*crtc,
 
dce_v6_0_lock_cursor(crtc, true);
 
-   if (width != amdgpu_crtc->cursor_width ||
-   height != amdgpu_crtc->cursor_height ||
-   hot_x != amdgpu_crtc->cursor_hot_x ||
+   if (hot_x != amdgpu_crtc->cursor_hot_x ||
hot_y != amdgpu_crtc->cursor_hot_y) {
int x, y;
 
@@ -1955,8 +1953,6 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc 
*crtc,
 
dce_v6_0_cursor_move_locked(crtc, x, y);
 
-   amdgpu_crtc->cursor_width = width;
-   amdgpu_crtc->cursor_height = height;
amdgpu_crtc->cursor_hot_x = hot_x;
amdgpu_crtc->cursor_hot_y = hot_y;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index c0abadf..fb7c0e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2437,8 +2437,6 @@ static int dce_v8_0_crtc_cursor_set2(struct drm_crtc 
*crtc,
 
dce_v8_0_cursor_move_locked(crtc, x, y);
 
-   amdgpu_crtc->cursor_width = width;
-   amdgpu_crtc->cursor_height = height;
amdgpu_crtc->cursor_hot_x = hot_x;
amdgpu_crtc->cursor_hot_y = hot_y;
}
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: [PATCH] udev_monitor_receive_device() will block when hotplug monitor

[Qu, Jim]

Got it!

Thanks
JimQu


发件人: Michel Dänzer 
发送时间: 2016年12月14日 11:03
收件人: Qu, Jim
抄送: amd-gfx@lists.freedesktop.org
主题: Re: [PATCH] udev_monitor_receive_device() will block when hotplug monitor

On 13/12/16 05:33 PM, jimqu wrote:
> udev_monitor_receive_device() will block and wait for the event of udev
> use select() to ensure that this will not block.
>
> Change-Id: I4e8f4629580222e94f55a4c03070741bf5f4024c
> Signed-off-by: JimQu 

Reviewed and pushed, thanks!


P.S. Please run

git config --global user.name "Jim Qu"
git config email.prefix "PATCH $(basename $PWD)"

in your Git tree.

--
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] udev_monitor_receive_device() will block when hotplug monitor

[Michel Dänzer]

On 13/12/16 05:33 PM, jimqu wrote:
> udev_monitor_receive_device() will block and wait for the event of udev
> use select() to ensure that this will not block.
> 
> Change-Id: I4e8f4629580222e94f55a4c03070741bf5f4024c
> Signed-off-by: JimQu 

Reviewed and pushed, thanks!


P.S. Please run

git config --global user.name "Jim Qu"
git config email.prefix "PATCH $(basename $PWD)"

in your Git tree.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Bridgman, John]

>>If the Linux community contributes to DC, I guess those contributions
can generally be assumed to be GPLv2 licensed.  Yet a future version
of the macOS driver would incorporate those contributions in the same
binary as their closed source OS-specific portion.


My understanding of the "general rule" was that contributions are normally 
assumed to be made under the "local license", ie GPLv2 for kernel changes in 
general, but the appropriate lower-level license when made to a specific 
subsystem with a more permissive license (eg the X11 license aka MIT aka "GPL 
plus additional rights" license we use for almost all of the graphics 
subsystem. If DC is not X11 licensed today it should be (but I'm pretty sure it 
already is).


We need to keep the graphics subsystem permissively licensed in general to 
allow uptake by other free OS projects such as *BSD, not just closed source.


Either way, driver-level maintainers are going to have to make sure that 
contributions have clear licensing.


Thanks,

John


From: dri-devel  on behalf of Lukas 
Wunner 
Sent: December 13, 2016 4:40 AM
To: Cheng, Tony
Cc: Grodzovsky, Andrey; dri-devel; amd-gfx mailing list; Deucher, Alexander
Subject: Re: [RFC] Using DC in amdgpu for upcoming GPU

On Mon, Dec 12, 2016 at 09:52:08PM -0500, Cheng, Tony wrote:
> With DC the display hardware programming, resource optimization, power
> management and interaction with rest of system will be fully validated
> across multiple OSs.

Do I understand DAL3.jpg correctly that the macOS driver builds on top
of DAL Core?  I'm asking because the graphics drivers shipping with
macOS as well as on Apple's EFI Firmware Volume are closed source.

If the Linux community contributes to DC, I guess those contributions
can generally be assumed to be GPLv2 licensed.  Yet a future version
of the macOS driver would incorporate those contributions in the same
binary as their closed source OS-specific portion.

I don't quite see how that would be legal but maybe I'm missing
something.

Presumably the situation with the Windows driver is the same.

I guess you could maintain a separate branch sans community contributions
which would serve as a basis for closed source drivers, but not sure if
that is feasible given your resource constraints.

Thanks,

Lukas
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Lukas Wunner]

On Tue, Dec 13, 2016 at 10:03:58AM -0500, Cheng, Tony wrote:
> On 12/13/2016 4:40 AM, Lukas Wunner wrote:
> > If the Linux community contributes to DC, I guess those contributions
> > can generally be assumed to be GPLv2 licensed.  Yet a future version
> > of the macOS driver would incorporate those contributions in the same
> > binary as their closed source OS-specific portion.
> 
> I am struggling with that these comminty contributions to DC would be.
> 
> Us AMD developer has access to HW docs and designer and we are still
> spending 50% of our time figuring out why our HW doesn't work right.
> I can't image community doing much of this heavy lifting.

True, but past experience with radeon/amdgpu is that the community has
use cases that AMD developers don't specifically cater to, e.g. due to
lack of the required hardware or resource constraints.

E.g. Mario Kleiner has contributed lots of patches for proper vsync
handling which are needed for his neuroscience software.  I've
contributed DDC switching support for MacBook Pros to radeon.
Your driver becomes more useful, you get more customers, everyone wins.


> > Do I understand DAL3.jpg correctly that the macOS driver builds on top
> > of DAL Core?  I'm asking because the graphics drivers shipping with
> > macOS as well as on Apple's EFI Firmware Volume are closed source.
> 
> macOS currently ship with their own driver.  I can't really comment on what
> macOS do without getting into trouble.

The Intel Israel folks working on Thunderbolt are similarly between
the rock that is the community's expectation of openness and the hard
place that is Apple's secrecy.  So I sympathize with your situation,
kudos for trying to do the right thing.


> I guess we can nak all changes and "rewrite" our
> own version of clean up patch community want to see?

I don't think that would be workable honestly.

One way out of this conundrum might be to use a permissive license such
as BSD for DC.  Then whenever you merge a community patch, in addition
to informing the contributor thereof, send them a boilerplate one-liner
that community contributions are assumed to be under the same license
and if the contributor disagrees they should send a short notice to
have their contribution removed.

But IANAL.

Best regards,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Cheng, Tony]

Only DM that’s open source is amdgpu_dm.the rest will remain closed 
source.I remember we had discussion around legal issues with our grand 
plan of unifying everything, and I remember maybe it was John who 
assured us that it's okay.John can you chime in how it would work with 
GPLv2 licsense?



On 12/13/2016 4:40 AM, Lukas Wunner wrote:

On Mon, Dec 12, 2016 at 09:52:08PM -0500, Cheng, Tony wrote:

With DC the display hardware programming, resource optimization, power
management and interaction with rest of system will be fully validated
across multiple OSs.

Do I understand DAL3.jpg correctly that the macOS driver builds on top
of DAL Core?  I'm asking because the graphics drivers shipping with
macOS as well as on Apple's EFI Firmware Volume are closed source.
macOS currently ship with their own driver.  I can't really comment on 
what macOS do without getting into trouble.

If the Linux community contributes to DC, I guess those contributions
can generally be assumed to be GPLv2 licensed.  Yet a future version
of the macOS driver would incorporate those contributions in the same
binary as their closed source OS-specific portion.

I am struggling with that these comminty contributions to DC would be.

Us AMD developer has access to HW docs and designer and we are still 
spending 50% of our time figuring out why our HW doesn't work right. I 
can't image community doing much of this heavy lifting.


I don't quite see how that would be legal but maybe I'm missing
something.

Presumably the situation with the Windows driver is the same.

I guess you could maintain a separate branch sans community contributions
which would serve as a basis for closed source drivers, but not sure if
that is feasible given your resource constraints.
Dave sent us series of patch to show how it would look like if someone 
were to change DC.  These changes are more removing code that DRM 
already has and deleting/clean up stuff.  I guess we can nak all changes 
and "rewrite" our own version of clean up patch community want to see?

Thanks,

Lukas


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Rob Clark]

On Mon, Dec 12, 2016 at 11:10 PM, Cheng, Tony  wrote:
> We need to treat most of resource that don't map well as global. One example
> is pixel pll.  We have 6 display pipes but only 2 or 3 plls in CI/VI, as a
> result we are limited in number of HDMI or DVI we can drive at the same
> time.  Also the pixel pll can be used to drive DP as well, so there is
> another layer of HW specific but we can't really contain it in crtc or
> encoder by itself.  Doing this resource allocation require knowlege of the
> whole system, and knowning which pixel pll is already used, and what can we
> support with remaining pll.
>
> Another ask is lets say we are driving 2 displays, we would always want
> instance 0 and instance 1 of scaler, timing generator etc getting used.  We
> want to avoid possiblity of due to different user mode commit sequence we
> end up with driving the 2 display with 0 and 2nd instance of HW.  Not only
> this configuration isn't really validated in the lab, we will be less
> effective in power gating as instance 0 and 1 are one the same tile.
> instead of having 2/3 of processing pipeline silicon power gated we can only
> power gate 1/3. And if we power gate wrong the you will have 1 of the 2
> display not lighting up.

Note that as of 4.10, drm/msm/mdp5 is dynamically assigning hwpipes to
planes tracked as part of the driver's global atomic state.  (And for
future hw we will need to dynamically assign layermixers to crtc's).
I'm also using global state for allocating SMP (basically fifo)
blocks.  And drm/i915 is also using global atomic state for shared
resources.

Dynamic assignment of hw resources to kms objects is not a problem,
and the locking model in atomic allows for this.  (I introduced one
new global modeset_lock to protect the global state, so only multiple
parallel updates which both touch shared state will serialize)

BR,
-R
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Daniel Vetter]

On Tue, Dec 13, 2016 at 12:22:59PM +, Daniel Stone wrote:
> Hi Harry,
> I've been loathe to jump in here, not least because both cop roles
> seem to be taken, but ...
> 
> On 13 December 2016 at 01:49, Harry Wentland  wrote:
> > On 2016-12-11 09:57 PM, Dave Airlie wrote:
> >> On 8 December 2016 at 12:02, Harry Wentland  wrote:
> >> Sharing code is a laudable goal and I appreciate the resourcing
> >> constraints that led us to the point at which we find ourselves, but
> >> the way forward involves finding resources to upstream this code,
> >> dedicated people (even one person) who can spend time on a day by day
> >> basis talking to people in the open and working upstream, improving
> >> other pieces of the drm as they go, reading atomic patches and
> >> reviewing them, and can incrementally build the DC experience on top
> >> of the Linux kernel infrastructure. Then having the corresponding
> >> changes in the DC codebase happen internally to correspond to how the
> >> kernel code ends up looking. Lots of this code overlaps with stuff the
> >> drm already does, lots of is stuff the drm should be doing, so patches
> >> to the drm should be sent instead.
> >
> > Personally I'm with you on this and hope to get us there. I'm learning...
> > we're learning. I agree that changes on atomic, removing abstractions, etc.
> > should happen on dri-devel.
> >
> > When it comes to brand-new technologies (MST, Freesync), though, we're often
> > the first which means that we're spending a considerable amount of time to
> > get things right, working with HW teams, receiver vendors and other partners
> > internal and external to AMD. By the time we do get it right it's time to
> > hit the market. This gives us fairly little leeway to work with the
> > community on patches that won't land in distros for another half a year.
> > We're definitely hoping to improve some of this but it's not easy and in
> > some case impossible ahead of time (though definitely possibly after initial
> > release).
> 
> Speaking with my Wayland hat on, I think these need to be very
> carefully considered. Both MST and FreeSync have _significant_ UABI
> implications, which may not be immediately obvious when working with a
> single implementation. Having them working and validated with a
> vertical stack of amdgpu-DC/KMS + xf86-video-amdgpu +
> Mesa-amdgpu/AMDGPU-Pro is one thing, but looking outside the X11 world
> we now have Weston, Mutter and KWin all directly driving KMS, plus
> whatever Mir/Unity ends up doing (presumably the same), and that's
> just on the desktop. Beyond the desktop, there's also CrOS/Freon and
> Android/HWC. For better or worse, outside of Xorg and HWC, we no
> longer have a vendor-provided userspace component driving KMS.
> 
> It was also easy to get away with loose semantics before with X11
> imposing little to no structure on rendering, but we now have the twin
> requirements of an atomic and timing-precise ABI - see Mario Kleiner's
> unending quest for accuracy - and also a vendor-independent ABI. So a
> good part of the (not insignificant) pain incurred in the atomic
> transition for drivers, was in fact making those drivers conform to
> the expectations of the KMS UABI contract, which just happened to not
> have been tripped over previously.
> 
> Speaking with my Collabora hat on now: we did do a substantial amount
> of demidlayering on the Exynos driver, including an atomic conversion,
> on Google's behalf. The original Exynos driver happened to work with
> the Tizen stack, but ChromeOS exposed a huge amount of subtle
> behaviour differences between that and other drivers when using Freon.
> We'd also hit the same issues when attempting to use Weston on Exynos
> in embedded devices for OEMs we worked with, so took on the project to
> remove the midlayer and have as much as possible driven from generic
> code.
> 
> How the hardware is programmed is of course ultimately up to you, and
> in this regard AMD will be very different from Intel is very different
> from Nouveau is very different from Rockchip. But especially for new
> features like FreeSync, I think we need to be very conscious of
> walking the line between getting those features in early, and setting
> unworkable UABI in stone. It would be unfortunate if later on down the
> line, you had to choose between breaking older xf86-video-amdgpu
> userspace which depended on specific behaviours of the amdgpu kernel
> driver, or breaking the expectations of generic userspace such as
> Weston/Mutter/etc.
> 
> One good way to make sure you don't get into that position, is to have
> core KMS code driving as much of the machinery as possible, with a
> very clear separation of concerns between actual hardware programming,
> versus things which may be visible to userspace. This I think is
> DanielV's point expressed at much greater length. ;)
> 
> I should be clear though that this isn't unique to AMD, nor a problem
> 

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Daniel Stone]

Hi Harry,
I've been loathe to jump in here, not least because both cop roles
seem to be taken, but ...

On 13 December 2016 at 01:49, Harry Wentland  wrote:
> On 2016-12-11 09:57 PM, Dave Airlie wrote:
>> On 8 December 2016 at 12:02, Harry Wentland  wrote:
>> Sharing code is a laudable goal and I appreciate the resourcing
>> constraints that led us to the point at which we find ourselves, but
>> the way forward involves finding resources to upstream this code,
>> dedicated people (even one person) who can spend time on a day by day
>> basis talking to people in the open and working upstream, improving
>> other pieces of the drm as they go, reading atomic patches and
>> reviewing them, and can incrementally build the DC experience on top
>> of the Linux kernel infrastructure. Then having the corresponding
>> changes in the DC codebase happen internally to correspond to how the
>> kernel code ends up looking. Lots of this code overlaps with stuff the
>> drm already does, lots of is stuff the drm should be doing, so patches
>> to the drm should be sent instead.
>
> Personally I'm with you on this and hope to get us there. I'm learning...
> we're learning. I agree that changes on atomic, removing abstractions, etc.
> should happen on dri-devel.
>
> When it comes to brand-new technologies (MST, Freesync), though, we're often
> the first which means that we're spending a considerable amount of time to
> get things right, working with HW teams, receiver vendors and other partners
> internal and external to AMD. By the time we do get it right it's time to
> hit the market. This gives us fairly little leeway to work with the
> community on patches that won't land in distros for another half a year.
> We're definitely hoping to improve some of this but it's not easy and in
> some case impossible ahead of time (though definitely possibly after initial
> release).

Speaking with my Wayland hat on, I think these need to be very
carefully considered. Both MST and FreeSync have _significant_ UABI
implications, which may not be immediately obvious when working with a
single implementation. Having them working and validated with a
vertical stack of amdgpu-DC/KMS + xf86-video-amdgpu +
Mesa-amdgpu/AMDGPU-Pro is one thing, but looking outside the X11 world
we now have Weston, Mutter and KWin all directly driving KMS, plus
whatever Mir/Unity ends up doing (presumably the same), and that's
just on the desktop. Beyond the desktop, there's also CrOS/Freon and
Android/HWC. For better or worse, outside of Xorg and HWC, we no
longer have a vendor-provided userspace component driving KMS.

It was also easy to get away with loose semantics before with X11
imposing little to no structure on rendering, but we now have the twin
requirements of an atomic and timing-precise ABI - see Mario Kleiner's
unending quest for accuracy - and also a vendor-independent ABI. So a
good part of the (not insignificant) pain incurred in the atomic
transition for drivers, was in fact making those drivers conform to
the expectations of the KMS UABI contract, which just happened to not
have been tripped over previously.

Speaking with my Collabora hat on now: we did do a substantial amount
of demidlayering on the Exynos driver, including an atomic conversion,
on Google's behalf. The original Exynos driver happened to work with
the Tizen stack, but ChromeOS exposed a huge amount of subtle
behaviour differences between that and other drivers when using Freon.
We'd also hit the same issues when attempting to use Weston on Exynos
in embedded devices for OEMs we worked with, so took on the project to
remove the midlayer and have as much as possible driven from generic
code.

How the hardware is programmed is of course ultimately up to you, and
in this regard AMD will be very different from Intel is very different
from Nouveau is very different from Rockchip. But especially for new
features like FreeSync, I think we need to be very conscious of
walking the line between getting those features in early, and setting
unworkable UABI in stone. It would be unfortunate if later on down the
line, you had to choose between breaking older xf86-video-amdgpu
userspace which depended on specific behaviours of the amdgpu kernel
driver, or breaking the expectations of generic userspace such as
Weston/Mutter/etc.

One good way to make sure you don't get into that position, is to have
core KMS code driving as much of the machinery as possible, with a
very clear separation of concerns between actual hardware programming,
versus things which may be visible to userspace. This I think is
DanielV's point expressed at much greater length. ;)

I should be clear though that this isn't unique to AMD, nor a problem
of your creation. For example, I'm currently looking at a flip-timing
issue in Rockchip - a fairly small, recent, atomic-native, and
generally exemplary driver - which I'm pretty sure is going to be
resolved by deleting more driver 

[PATCH] udev_monitor_receive_device() will block when hotplug monitor

[jimqu]

udev_monitor_receive_device() will block and wait for the event of udev
use select() to ensure that this will not block.

Change-Id: I4e8f4629580222e94f55a4c03070741bf5f4024c
Signed-off-by: JimQu 
---
 src/drmmode_display.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 79c9390..a5b4345 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2528,10 +2528,26 @@ static void drmmode_handle_uevents(int fd, void 
*closure)
ScrnInfoPtr scrn = drmmode->scrn;
struct udev_device *dev;
Bool received = FALSE;
+   struct timeval tv;
+   fd_set readfd;
+   int ret;
+
+   FD_ZERO();
+   FD_SET(fd, );
+   tv.tv_sec = 0;
+   tv.tv_usec = 0;
 
-   while ((dev = udev_monitor_receive_device(drmmode->uevent_monitor))) {
-   udev_device_unref(dev);
-   received = TRUE;
+   while (1) {
+   ret = select(fd + 1, , NULL, NULL, );
+   /* Check if our file descriptor has received data. */
+   if (!(ret > 0 && FD_ISSET(fd, )))
+   break;
+   /* Make the call to receive device. select() ensured that this 
will not be blocked. */
+   dev = udev_monitor_receive_device(drmmode->uevent_monitor);
+   if (dev) {
+   udev_device_unref(dev);
+   received = TRUE;
+   }
}
 
if (received)
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Raciness with page table shadows being swapped out

[Nicolai Hähnle]

On 13.12.2016 10:48, Christian König wrote:

The attached patch has fixed these crashes for me so far, but it's
very heavy-handed: it collects all page table shadows and the page
directory shadow and adds them all to the reservations for the callers
of amdgpu_vm_update_page_directory.


That is most likely just a timing change, cause the shadows should end
up in the duplicates list anyway. So the patch shouldn't have any
effect.


Okay, so the reason for the remaining crash is still unclear at least
for me.


Yeah, that's a really good question. Can you share the call stack of the
problem once more?


Attaching the dmesg again.

amdgpu_gtt_mgr_alloc+0x23 resolves to the check

   if (node->start != AMDGPU_BO_INVALID_OFFSET)

amdgpu_vm_update_page_directory+0x23f is

r = amdgpu_ttm_bind(_shadow->tbo,
_shadow->tbo.mem);

Nicolai
[  545.477646] BUG: unable to handle kernel NULL pointer dereference at 
0048
[  545.477689] IP: [] amdgpu_gtt_mgr_alloc+0x23/0x150 [amdgpu]
[  545.477764] PGD 7e384a067
[  545.45] PUD 7f4a84067
[  545.477786] PMD 0

[  545.477797] Oops:  [#1] SMP
[  545.477810] Modules linked in: binfmt_misc nls_iso8859_1 eeepc_wmi asus_wmi 
video sparse_keymap mxm_wmi joydev input_leds edac_mce_amd edac_core kvm_amd 
kvm irqbypass snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi 
snd_hda_intel serio_raw snd_hda_codec snd_hda_core snd_hwdep fam15h_power 
k10temp snd_pcm snd_seq_midi i2c_piix4 snd_seq_midi_event snd_rawmidi snd_seq 
snd_seq_device snd_timer tpm_infineon snd soundcore wmi mac_hid shpchp 
parport_pc ppdev lp parport autofs4 algif_skcipher af_alg hid_generic usbhid 
hid dm_crypt amdkfd amd_iommu_v2 amdgpu crct10dif_pclmul crc32_pclmul 
ghash_clmulni_intel aesni_intel i2c_algo_bit aes_x86_64 drm_kms_helper 
glue_helper lrw syscopyarea gf128mul sysfillrect ablk_helper sysimgblt cryptd 
fb_sys_fops ttm psmouse drm ahci r8169 libahci mii fjes
[  545.478165] CPU: 5 PID: 29619 Comm: glcts Not tainted 
4.9.0-rc6-tip+drm-next-2 #104
[  545.478191] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A97 LE R2.0, BIOS 2601 03/24/2015
[  545.478225] task: 8be896f4d580 task.stack: b7af4c3f4000
[  545.478246] RIP: 0010:[]  [] 
amdgpu_gtt_mgr_alloc+0x23/0x150 [amdgpu]
[  545.478301] RSP: 0018:b7af4c3f7a28  EFLAGS: 00010296
[  545.478320] RAX: 7fff RBX: 8be8967e6180 RCX: 8be82806ec90
[  545.478343] RDX:  RSI: 8be82806ec58 RDI: 8be8957c9980
[  545.478367] RBP: b7af4c3f7a88 R08: 8be8bed5c540 R09: 8be89e003900
[  545.478390] R10: 8be896af4cc0 R11: 8be8957c1900 R12: 
[  545.478412] R13:  R14: 8be8967e6228 R15: 8be82806fc00
[  545.478437] FS:  7ff4415f2740() GS:8be8bed4() 
knlGS:
[  545.478462] CS:  0010 DS:  ES:  CR0: 80050033
[  545.478481] CR2: 0048 CR3: 0007c031a000 CR4: 000406e0
[  545.478506] Stack:
[  545.478513]   d3451c09 8be78308a9d8 

[  545.478544]  8be8957c8000 1a00 8be863b86000 
8be8967e6180
[  545.478575]  8be82806ec90  8be8967e6228 
8be82806fc00
[  545.478604] Call Trace:
[  545.478632]  [] amdgpu_ttm_bind+0x61/0x160 [amdgpu]
[  545.478672]  [] 
amdgpu_vm_update_page_directory+0x23f/0x4c0 [amdgpu]
[  545.478717]  [] amdgpu_cs_ioctl+0xd8a/0x1400 [amdgpu]
[  545.478759]  [] drm_ioctl+0x1f6/0x4a0 [drm]
[  545.478794]  [] ? amdgpu_cs_find_mapping+0xa0/0xa0 [amdgpu]
[  545.478823]  [] ? update_load_avg+0x75/0x390
[  545.478858]  [] amdgpu_drm_ioctl+0x4c/0x80 [amdgpu]
[  545.478882]  [] do_vfs_ioctl+0xa1/0x5d0
[  545.478902]  [] ? __schedule+0x23a/0x6f0
[  545.478923]  [] SyS_ioctl+0x79/0x90
[  545.478942]  [] entry_SYSCALL_64_fastpath+0x1e/0xad
[  545.478965] Code: 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 b8 ff ff ff 
ff ff ff ff 7f 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 38 4c 8b 21 <49> 39 
44 24 48 74 11 31 c0 48 83 c4 38 5b 41 5c 41 5d 41 5e 41
[  545.479133] RIP  [] amdgpu_gtt_mgr_alloc+0x23/0x150 
[amdgpu]
[  545.479179]  RSP 
[  545.479192] CR2: 0048
[  545.485015] ---[ end trace 390c3d6250a76506 ]---
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Raciness with page table shadows being swapped out

[Christian König]

Am 13.12.2016 um 00:35 schrieb Nicolai Hähnle:

On 12.12.2016 19:22, Christian König wrote:

Am 12.12.2016 um 16:20 schrieb Nicolai Hähnle:

Hi all,

I just sent out two patches that hopefully make the kernel module more
robust in the face of page table shadows being swapped out.

However, even with those patches, I can still fairly reliably
reproduce crashes with a backtrace of the shape

amdgpu_cs_ioctl
 -> amdgpu_vm_update_page_directory
 -> amdgpu_ttm_bind
 -> amdgpu_gtt_mgr_alloc

The plausible reason for these crashes is that nothing seems to
prevent the shadow BOs from being moved between the calls to
amdgpu_cs_validate in amdgpu_cs_parser_bos and the calls to
amdgpu_ttm_bind.


The shadow BOs use the same reservation object than the real BOs. So as
long as the real BOs can't be evicted the shadows can't be evicted 
either.


Ah okay. I don't see where the page table BOs are added to the buffer 
list. Do they also share the reservation object with the page directory?


Yes, exactly. All use the same reservation object, so when you hold the 
lock on the PD nothing in the VM will be evicted any more.






The attached patch has fixed these crashes for me so far, but it's
very heavy-handed: it collects all page table shadows and the page
directory shadow and adds them all to the reservations for the callers
of amdgpu_vm_update_page_directory.


That is most likely just a timing change, cause the shadows should end
up in the duplicates list anyway. So the patch shouldn't have any 
effect.


Okay, so the reason for the remaining crash is still unclear at least 
for me.


Yeah, that's a really good question. Can you share the call stack of the 
problem once more?


Thanks,
Christian.



Cheers,
Nicolai





I feel like there should be a better way. In part, I wonder why the
shadows are needed in the first place. I vaguely recall the
discussions about GPU reset and such, but I don't remember why the
page tables can't just be rebuilt in some other way.


It's just the simplest and fastest way to keep a copy of the page tables
around.

The problem with rebuilding the page tables from the mappings is that
the housekeeping structures already have the future state when a reset
happens, not the state we need to rebuild the tables.

We could obviously change the housekeeping a bit to keep both states,
but that would complicate mapping and unmapping of BOs significantly.

Regards,
Christian.



Cheers,
Nicolai


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Cheng, Tony]

On 12/13/2016 2:30 AM, Dave Airlie wrote:

(hit send too early)

We would love to upstream DC for all supported asic!  We made enough change
to make Sea Island work but it's really not validate to the extend we
validate Polaris on linux and no where close to what we do for 2017 ASICs.
With DC the display hardware programming, resource optimization, power
management and interaction with rest of system will be fully validated
across multiple OSs.  Therefore we have high confidence that the quality is
going to better than what we have upstreammed today.

I don't have a baseline to say if DC is in good enough quality for older
generation compare to upstream.  For example we don't have HW generate
bandwidth_calc for DCE 8/10 (Sea/Vocanic island family) but our code is
structured in a way that we assume bandwidth_calc is there.  None of us feel
like go untangle the formulas in windows driver at this point to create our
own version of bandwidth_calc.  It sort of work with HW default values but
some mode / config is likely to underflows.  If community is okay with
uncertain quality, sure we would love to upstream everything to reduce our
maintaince overhead.  You do get audio with DC on DCE8 though.

If we get any of this upstream, we should get all of the hw supported with it.

If it regresses we just need someone to debug why.

great will do.



Maybe let me share what we are doing and see if we can come up with
something to make DC work for both upstream and our internal need.  We are
sharing code not just on Linux and we will do our best to make our code
upstream friendly.  Last year we focussed on having enough code to prove
that our DAL rewrite works and get more people contributing to it.  We rush
a bit as a result we had a few legacy component we port from Windows driver
and we know it's bloat that needed to go.

We designed DC so HW can contribute bandwidth_calc magic and psuedo code to
program the HW blocks.  The HW blocks on the bottom of DC.JPG in models our
HW blocks and the programming sequence are provided by HW engineers.  If a
piece of HW need a bit toggled 7 times during power up I rather have HW
engineer put that in their psedo code rather than me trying to find that
sequence in some document.  Afterall they did simulate the HW with the
toggle sequence.  I guess these are back-end code Daniel talked about.  Can
we agree that DRM core is not interested in how things are done in that
layer and we can upstream these as it?

The next is dce_hwseq.c to program the HW blocks in correct sequence.  Some
HW block can be programmed in any sequence, but some requires strict
sequence to be followed.  For example Display CLK and PHY CLK need to be up
before we enable timing generator.  I would like these sequence to remain in
DC as it's really not DRM's business to know how to program the HW.  In a
way you can consider hwseq as a helper to commit state to HW.

Above hwseq is the dce*_resource.c.  It's job is to come up with the HW
state required to realize given config.  For example we would use the exact
same HW resources with same optimization setting to drive any same given
config.  If 4 x 4k@60 is supported with resource setting A on HW diagnositc
suite during bring up setting B on Linux then we have a problem.  It know
which HW block work with which block and their capability and limitations.
I hope you are not asking this stuff to move up to core because in reality
we should probably hide this in some FW, as HW expose the register to config
them differently that doesn't mean all combination of HW usage is validated.
To me resource is more of a helper to put together functional pipeline and
does not make any decision that any OS might be interested in.

These yellow boxes in DC.JPG are really specific to each generation of HW
and changes frequently.  These are things that HW has consider hiding it in
FW before.  Can we agree on those code (under /dc/dce*) can stay?

I think most of these things are fine to be part of the solution we end up at,
but I can't say for certain they won't require interface changes. I think the
most useful code is probably the stuff in the dce subdirectories.
okay as long as we can agree on this piece stay I am sure we can make it 
work.



Is this about demonstration how basic functionality work and add more
features with series of patches to make review eaiser?  If so I don't think
we are staff to do this kind of rewrite.  For example it make no sense to
hooking up bandwidth_calc to calculate HW magic if we don't have mem_input
to program the memory settings.  We need portion of hw_seq to ensure these
blocks are programming in correct sequence.  We will need to feed
bandwidth_calc it's required inputs, which is basically the whole system
state tracked in validate_context today, which means we basically need big
bulk of resource.c.  This effort might have benefit in reviewing the code,
but we will end up with pretty much similar if not the same as what we
already have.

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Daniel Vetter]

On Mon, Dec 12, 2016 at 09:05:15PM -0500, Harry Wentland wrote:
> 
> On 2016-12-12 02:22 AM, Daniel Vetter wrote:
> > On Wed, Dec 07, 2016 at 09:02:13PM -0500, Harry Wentland wrote:
> > > Current version of DC:
> > > 
> > >  * 
> > > https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7
> > > 
> > > Once Alex pulls in the latest patches:
> > > 
> > >  * 
> > > https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7
> > 
> > One more: That 4.7 here is going to be unbelievable amounts of pain for
> > you. Yes it's a totally sensible idea to just freeze your baseline kernel
> > because then linux looks a lot more like Windows where the driver abi is
> > frozen. But it makes following upstream entirely impossible, because
> > rebasing is always a pain and hence postponed. Which means you can't just
> > use the latest stuff in upstream drm, which means collaboration with
> > others and sharing bugfixes in core is a lot more pain, which then means
> > you do more than necessary in your own code and results in HALs like DAL,
> > perpetuating the entire mess.
> > 
> > So I think you don't just need to demidlayer DAL/DC, you also need to
> > demidlayer your development process. In our experience here at Intel that
> > needs continuous integration testing (in drm-tip), because even 1 month of
> > not resyncing with drm-next is sometimes way too long. See e.g. the
> > controlD regression we just had. And DAL is stuck on a 1 year old kernel,
> > so pretty much only of historical significance and otherwise dead code.
> > 
> > And then for any stuff which isn't upstream yet (like your internal
> > enabling, or DAL here, or our own internal enabling) you need continuous
> > rebasing When we started doing this years ago it was still
> > manually, but we still rebased like every few days to keep the pain down
> > and adjust continuously to upstream evolution. But then going to a
> > continous rebase bot that sends you mail when something goes wrong was
> > again a massive improvement.
> > 
> 
> I think we've seen that pain already but haven't quite realized how much of
> it is due to a mismatch in kernel trees. We're trying to move onto a tree
> following drm-next much more closely. I'd love to help automate some of that
> (time permitting). Would the drm-misc scripts be of any use with that? I
> only had a very cursory glance at those.

I've offered to Alex that we could include the amd trees (only stuff ready
for pull request) into drm-tip for continuous integration testing at
least. Would mean that Alex needs to use dim when updating those branches,
and you CI needs to test drm-tip (and do that everytime it changes, i.e.
really continuously).

For continues rebasing there's no ready-made thing public, but I highly
recommend you use one of the patch pile tools. In intel we have a glue of
quilt + tracking quilt state with git, implemented in the qf script in
maintainer-tools. That one has a lot more sharp edges than dim, but it
gets the job done. And the combination of git track + raw patch file for
seding is very powerful for rebasing.

Long term I'm hopefully that git series will become the new shiny, since
Josh Tripplet really understands the use-cases of having long-term
rebasing trees which are collaboratively maintainer. It's a lot nicer than
qf, but can't yet do everything we need (and likely what you'll need to be
able to rebase DC without going crazy).

> > I guess in the end Conway's law that your software architecture
> > necessarily reflects how you organize your teams applies again. Fix your
> > process and it'll become glaringly obvious to everyone involved that
> > DC-the-design as-is is entirely unworkeable and how it needs to be fixed.
> > 
> > From my own experience over the past few years: Doing that is a fun
> > journey ;-)
> > 
> 
> Absolutely. We're only at the start of this but have learned a lot from the
> community (maybe others in the DC team disagree with me somewhat).
> 
> Not sure if I fully agree that this means that DC-the-design-as-is will
> become apparent as unworkable... There are definitely pieces to be cleaned
> here and lessons learned from the DRM community but on the other hand we
> feel there are some good reasons behind our approach that we'd like to share
> with the community (some of which I'm learning myself).

Tony asking what the difference between a midlayer and a helper library is
is imo a good indicator that there's still learning to do in the team ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx