Re: Atomic driver and old remove FB behavior
Hi, Maarten!
On 02/06/2018 05:37 PM, Maarten Lankhorst wrote:
Op 06-02-18 om 15:43 schreef Oleksandr Andrushchenko:
Hello, Maarten!
On 02/01/2018 12:13 PM, Oleksandr Andrushchenko wrote:
On 02/01/2018 12:04 PM, Maarten Lankhorst wrote:
Op 01-02-18 om 08:08 schreef Oleksandr Andrushchenko:
Hi, all!
I am working on a para-virtualized frontend DRM driver for Xen [1]
which implements display device I/O interface for Xen guest OSes [2].
At the moment I am rebasing the existing implementation from 4.9 kernel
to recent and found that when user-space DRM application exits then CRTC's
.set_config callback is not called to reset CRTC's mode to NULL.
I tracked the change down to commit 846c7dfc1193eb2f9866fe2fa0ae7d45c72f95b9
"drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.
This introduces a slight behavioral change to rmfb. Instead of
disabling a crtc when the primary plane is disabled, we try to
preserve it.
If the atomic commit is rejected by the driver then we will still
fall back to the old behavior and turn off the crtc.
"
which per my understanding is ok for real hardware, but in my case I still
need .set_config to be called with NULL: this way I can tell the backend
driver that it can release resources for this connection on its end.
I tried to implement drm_mode_config_funcs's .atomic_commit returning -EINVAL
on tear down, but at best it made CRTC's atomic state change to NOCRTC, but
still no .set_config callback seen.
Can anyone please tell me the right sequence to implement old remove FB
behavior for atomic drivers? Or what could be the problem on my side?
rmfb was never meant to mean that the crtc will be disabled as well. vmwgfx
relied on that
behavior as well for old userspace support because userspace never called
set_config.
If you're an atomic driver, the driver core will never call set_config any
more, even if the
first attempt to disable returns -EINVAL. Even the fbcon helpers now perform
direct
calls to atomic_commit().
understood, thank you for clarification
I started to re-implement the existing code in my driver to comply to the
new atomic behavior in terms of "the driver core will never call set_config any
more"
and the question now is what would be the right place to put a check for mode
change?
Basically, I would like to know where I can get "struct drm_mode_set" as
I used to have in drm_crtc_funcs.set_config callback? Or some other source of
information I can derive (x,y), (width,height), pixel format for the mode
when it changes.
Any validation check needs to be in atomic_check(), the other check
depends on what kind of check you are looking at. atomic_begin() and
atomic_flush() are usually for plane updates.
If you have special needs for atomic updates, you could always implement
your own implementation for commit_tail(), the original is something like this:
{
drm_atomic_helper_commit_modeset_disables() -> can always put all
disables here, including when mode is changed. If you replace this, make sure you
put drm_atomic_helper_update_legacy_modeset_state() after all disables..
Next 2 calls can be swapped, depending on whether planes need to be
programmed before crtc enabling or after:
{
drm_atomic_helper_commit_modeset_enables() -> putting enables
here..
drm_atomic_helper_commit_planes()
}
drm_atomic_helper_commit_hw_done()
drm_atomic_helper_wait_for_flip_done()
drm_atomic_helper_wait_for_vblanks()
drm_atomic_helper_cleanup_planes()
}
You can always trace the flow and see if there's a function call that's useful
for you in there.
Thank you for such a detailed explanation
I would just call set_config if required in 2 places, once in crtc disable,
once in crtc enable.
Well, in the other mail thread [1] Ville Syrjälä gave an idea to switch
from own CRTC implementation to drm_simple_kms_helper which can be
definitely
done now as I'm moving from legacy to fully atomic operation.
So, I did the change and have something like the below now:
static void display_enable(struct drm_simple_display_pipe *pipe,
struct drm_crtc_state *crtc_state)
{
struct drm_plane *plane = >plane;
struct drm_framebuffer *fb = plane->state->fb;
struct drm_crtc *crtc = >crtc;
display_set_config(pipe, fb);
drm_crtc_vblank_on(crtc);
}
static void display_disable(struct drm_simple_display_pipe *pipe)
{
struct drm_crtc *crtc = >crtc;
display_set_config(pipe, NULL);
drm_crtc_vblank_off(crtc);
}
I believe this is what you meant while suggesting "once in crtc
disable, once in crtc enable" WRT drm_simple_kms_helper's
drm_simple_display_pipe_funcs callbacks.
Candidates I am playing with are:
1. drm_crtc_helper_funcs.atomic_flush
2. drm_mode_config_funcs.atomic_commit
During experiments with the above I see that during "on" phase in the
drm_crtc_state I can see that mode, enable and active
Re: Atomic driver and old remove FB behavior
Op 06-02-18 om 15:43 schreef Oleksandr Andrushchenko:
> Hello, Maarten!
>
>
> On 02/01/2018 12:13 PM, Oleksandr Andrushchenko wrote:
>> On 02/01/2018 12:04 PM, Maarten Lankhorst wrote:
>>> Op 01-02-18 om 08:08 schreef Oleksandr Andrushchenko:
Hi, all!
I am working on a para-virtualized frontend DRM driver for Xen [1]
which implements display device I/O interface for Xen guest OSes [2].
At the moment I am rebasing the existing implementation from 4.9 kernel
to recent and found that when user-space DRM application exits then CRTC's
.set_config callback is not called to reset CRTC's mode to NULL.
I tracked the change down to commit
846c7dfc1193eb2f9866fe2fa0ae7d45c72f95b9
"drm/atomic: Try to preserve the crtc enabled state in
drm_atomic_remove_fb, v2.
This introduces a slight behavioral change to rmfb. Instead of
disabling a crtc when the primary plane is disabled, we try to
preserve it.
If the atomic commit is rejected by the driver then we will still
fall back to the old behavior and turn off the crtc.
"
which per my understanding is ok for real hardware, but in my case I still
need .set_config to be called with NULL: this way I can tell the backend
driver that it can release resources for this connection on its end.
I tried to implement drm_mode_config_funcs's .atomic_commit returning
-EINVAL
on tear down, but at best it made CRTC's atomic state change to NOCRTC, but
still no .set_config callback seen.
Can anyone please tell me the right sequence to implement old remove FB
behavior for atomic drivers? Or what could be the problem on my side?
>>> rmfb was never meant to mean that the crtc will be disabled as well. vmwgfx
>>> relied on that
>>> behavior as well for old userspace support because userspace never called
>>> set_config.
>>>
>>> If you're an atomic driver, the driver core will never call set_config any
>>> more, even if the
>>> first attempt to disable returns -EINVAL. Even the fbcon helpers now
>>> perform direct
>>> calls to atomic_commit().
>> understood, thank you for clarification
> I started to re-implement the existing code in my driver to comply to the
> new atomic behavior in terms of "the driver core will never call set_config
> any more"
> and the question now is what would be the right place to put a check for mode
> change?
> Basically, I would like to know where I can get "struct drm_mode_set" as
> I used to have in drm_crtc_funcs.set_config callback? Or some other source of
> information I can derive (x,y), (width,height), pixel format for the mode
> when it changes.
Any validation check needs to be in atomic_check(), the other check
depends on what kind of check you are looking at. atomic_begin() and
atomic_flush() are usually for plane updates.
If you have special needs for atomic updates, you could always implement
your own implementation for commit_tail(), the original is something like this:
{
drm_atomic_helper_commit_modeset_disables() -> can always put all
disables here, including when mode is changed. If you replace this, make sure
you put drm_atomic_helper_update_legacy_modeset_state() after all disables..
Next 2 calls can be swapped, depending on whether planes need to be
programmed before crtc enabling or after:
{
drm_atomic_helper_commit_modeset_enables() -> putting enables
here..
drm_atomic_helper_commit_planes()
}
drm_atomic_helper_commit_hw_done()
drm_atomic_helper_wait_for_flip_done()
drm_atomic_helper_wait_for_vblanks()
drm_atomic_helper_cleanup_planes()
}
You can always trace the flow and see if there's a function call that's useful
for you in there.
I would just call set_config if required in 2 places, once in crtc disable,
once in crtc enable.
> Candidates I am playing with are:
> 1. drm_crtc_helper_funcs.atomic_flush
> 2. drm_mode_config_funcs.atomic_commit
>
> During experiments with the above I see that during "on" phase in the
> drm_crtc_state I can see that mode, enable and active fields of the state
> are changed to 1, but during the "off" phase they remain at 1.
> Then I added the primary plane check from drm_simple_kms_crtc_check
> and at "off" those started to change to 0.
>
> Do you think drm_mode_config_funcs.atomic_commit with the above changes/checks
> will be the right way to get knowledge to emulate old ".set_config"?
> I am trying to be a new atomic driver, but due to the protocol [2]
> requirements
> I still need a way to send .set_config request to the backend.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Atomic driver and old remove FB behavior
Hello, Maarten! On 02/01/2018 12:13 PM, Oleksandr Andrushchenko wrote: On 02/01/2018 12:04 PM, Maarten Lankhorst wrote: Op 01-02-18 om 08:08 schreef Oleksandr Andrushchenko: Hi, all! I am working on a para-virtualized frontend DRM driver for Xen [1] which implements display device I/O interface for Xen guest OSes [2]. At the moment I am rebasing the existing implementation from 4.9 kernel to recent and found that when user-space DRM application exits then CRTC's .set_config callback is not called to reset CRTC's mode to NULL. I tracked the change down to commit 846c7dfc1193eb2f9866fe2fa0ae7d45c72f95b9 "drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2. This introduces a slight behavioral change to rmfb. Instead of disabling a crtc when the primary plane is disabled, we try to preserve it. If the atomic commit is rejected by the driver then we will still fall back to the old behavior and turn off the crtc. " which per my understanding is ok for real hardware, but in my case I still need .set_config to be called with NULL: this way I can tell the backend driver that it can release resources for this connection on its end. I tried to implement drm_mode_config_funcs's .atomic_commit returning -EINVAL on tear down, but at best it made CRTC's atomic state change to NOCRTC, but still no .set_config callback seen. Can anyone please tell me the right sequence to implement old remove FB behavior for atomic drivers? Or what could be the problem on my side? rmfb was never meant to mean that the crtc will be disabled as well. vmwgfx relied on that behavior as well for old userspace support because userspace never called set_config. If you're an atomic driver, the driver core will never call set_config any more, even if the first attempt to disable returns -EINVAL. Even the fbcon helpers now perform direct calls to atomic_commit(). understood, thank you for clarification I started to re-implement the existing code in my driver to comply to the new atomic behavior in terms of "the driver core will never call set_config any more" and the question now is what would be the right place to put a check for mode change? Basically, I would like to know where I can get "struct drm_mode_set" as I used to have in drm_crtc_funcs.set_config callback? Or some other source of information I can derive (x,y), (width,height), pixel format for the mode when it changes. Candidates I am playing with are: 1. drm_crtc_helper_funcs.atomic_flush 2. drm_mode_config_funcs.atomic_commit During experiments with the above I see that during "on" phase in the drm_crtc_state I can see that mode, enable and active fields of the state are changed to 1, but during the "off" phase they remain at 1. Then I added the primary plane check from drm_simple_kms_crtc_check and at "off" those started to change to 0. Do you think drm_mode_config_funcs.atomic_commit with the above changes/checks will be the right way to get knowledge to emulate old ".set_config"? I am trying to be a new atomic driver, but due to the protocol [2] requirements I still need a way to send .set_config request to the backend. If you then still want to revert to old behavior and it doesn't make sense to have the crtc enabled without primary plane enabled, you can copy the check from drm_simple_kms_crtc_check(). will try that, but at the moment this is not a strict requirement anymore, just wanted to make sure I understand the flow correctly ~Maarten Thank you, Oleksandr Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Atomic driver and old remove FB behavior
On 02/01/2018 12:04 PM, Maarten Lankhorst wrote: Op 01-02-18 om 08:08 schreef Oleksandr Andrushchenko: Hi, all! I am working on a para-virtualized frontend DRM driver for Xen [1] which implements display device I/O interface for Xen guest OSes [2]. At the moment I am rebasing the existing implementation from 4.9 kernel to recent and found that when user-space DRM application exits then CRTC's .set_config callback is not called to reset CRTC's mode to NULL. I tracked the change down to commit 846c7dfc1193eb2f9866fe2fa0ae7d45c72f95b9 "drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2. This introduces a slight behavioral change to rmfb. Instead of disabling a crtc when the primary plane is disabled, we try to preserve it. If the atomic commit is rejected by the driver then we will still fall back to the old behavior and turn off the crtc. " which per my understanding is ok for real hardware, but in my case I still need .set_config to be called with NULL: this way I can tell the backend driver that it can release resources for this connection on its end. I tried to implement drm_mode_config_funcs's .atomic_commit returning -EINVAL on tear down, but at best it made CRTC's atomic state change to NOCRTC, but still no .set_config callback seen. Can anyone please tell me the right sequence to implement old remove FB behavior for atomic drivers? Or what could be the problem on my side? rmfb was never meant to mean that the crtc will be disabled as well. vmwgfx relied on that behavior as well for old userspace support because userspace never called set_config. If you're an atomic driver, the driver core will never call set_config any more, even if the first attempt to disable returns -EINVAL. Even the fbcon helpers now perform direct calls to atomic_commit(). understood, thank you for clarification If you then still want to revert to old behavior and it doesn't make sense to have the crtc enabled without primary plane enabled, you can copy the check from drm_simple_kms_crtc_check(). will try that, but at the moment this is not a strict requirement anymore, just wanted to make sure I understand the flow correctly ~Maarten Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Atomic driver and old remove FB behavior
Op 01-02-18 om 08:08 schreef Oleksandr Andrushchenko: > Hi, all! > > I am working on a para-virtualized frontend DRM driver for Xen [1] > which implements display device I/O interface for Xen guest OSes [2]. > > At the moment I am rebasing the existing implementation from 4.9 kernel > to recent and found that when user-space DRM application exits then CRTC's > .set_config callback is not called to reset CRTC's mode to NULL. > > I tracked the change down to commit 846c7dfc1193eb2f9866fe2fa0ae7d45c72f95b9 > "drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, > v2. > > This introduces a slight behavioral change to rmfb. Instead of > disabling a crtc when the primary plane is disabled, we try to > preserve it. > > If the atomic commit is rejected by the driver then we will still > fall back to the old behavior and turn off the crtc. > " > which per my understanding is ok for real hardware, but in my case I still > need .set_config to be called with NULL: this way I can tell the backend > > driver that it can release resources for this connection on its end. > > > I tried to implement drm_mode_config_funcs's .atomic_commit returning -EINVAL > on tear down, but at best it made CRTC's atomic state change to NOCRTC, but > still no .set_config callback seen. > > Can anyone please tell me the right sequence to implement old remove FB > behavior for atomic drivers? Or what could be the problem on my side? rmfb was never meant to mean that the crtc will be disabled as well. vmwgfx relied on that behavior as well for old userspace support because userspace never called set_config. If you're an atomic driver, the driver core will never call set_config any more, even if the first attempt to disable returns -EINVAL. Even the fbcon helpers now perform direct calls to atomic_commit(). If you then still want to revert to old behavior and it doesn't make sense to have the crtc enabled without primary plane enabled, you can copy the check from drm_simple_kms_crtc_check(). ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
