| Message ID | 20190403193511.4waspuspncu7xrn7@smtp.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Assistance with a problem related to GEM and Atomic Commit inside vkms | expand |
On Wed, Apr 03, 2019 at 04:35:11PM -0300, Rodrigo Siqueira wrote: > Hi, Hi Rodrigo, > > I’m working to add writeback support to vkms; you can see my current > patch at the end of this email (some prints highlight the issue that I’m > asking for help here). I’m using the Liviu Dudau and Brian Starkey IGT > patchset, which can be seen here: > https://patchwork.freedesktop.org/series/39229/. > > I’m stuck with a problem that happens in my vkms_wb_atomic_commit() > function, which is a helper function at drm_connector_helper_funcs. You > can see the full implementation of this function at the end of this > email, but here is the important part: > > struct drm_framebuffer *fb = conn_state->writeback_job->fb; > struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); > struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj); > > A few lines after this variable initialization, I try to make a memory > copy like this: > > memcpy(vkms_conn->writeback_vaddr, vkms_obj->vaddr, vkms_obj->gem.size); > > If I test my implementation with the subtest writeback-fb-id, everything > work as expected; for extra details, this is the dmesg output after this > test and with a debug print: > > => writeback-fb-id > > [Apr 3 11:30] CIFS: Attempting to mount //10.0.2.4/qemu > [ +1.601900] Console: switching to colour dummy device 80x25 > [ +0.000017] [IGT] kms_writeback: executing > [ +0.032532] [IGT] kms_writeback: starting subtest writeback-fb-id > [ +0.001932] VKMS_OBJ (1): Buff (1) - Vaddr (1) - Size = 1228800 - count = 1 > [ +0.050524] [IGT] kms_writeback: exiting, ret=0 > [ +0.005202] Console: switching to colour frame buffer device 100x37 > > In the above log, you can see the print info; notice that Vaddr is the > output of vkms_obj->vaddr (Vaddr -> vkms_obj->vaddr != NULL). On the > other hand, if I try the subtest writeback-check-output, something weird > happens in this function. Here is the dmesg output for this test: > > => writeback-check-output > > [ +29.911185] Console: switching to colour dummy device 80x25 > [ +0.000017] [IGT] kms_writeback: executing > [ +0.021289] [IGT] kms_writeback: starting subtest writeback-check-output > [ +0.016270] VKMS_OBJ (1): Buff (1) - Vaddr (0) - Size = 1228800 - count = 0 > [ +0.000005] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > [ +0.000004] #PF error: [normal kernel read fault] > [ +0.000001] PGD 0 P4D 0 > [ +0.000003] Oops: 0000 [#1] PREEMPT SMP PTI > [ +0.000002] CPU: 3 PID: 426 Comm: kms_writeback Not tainted 5.0.0-VKMS-RULES+ #102 > [ +0.000002] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014 > [ +0.000021] RIP: 0010:__memcpy+0x12/0x20 > [ +0.000002] Code: ff 0f 31 48 c1 e2 20 48 09 c2 48 31 d3 e9 79 ff ff ff 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 f3 a4 > [ +0.000001] RSP: 0018:ffffadaf0098fbd8 EFLAGS: 00010246 > [ +0.000002] RAX: ffff9f1bf5400000 RBX: ffff9f1bfa28df00 RCX: 0000000000025800 > [ +0.000001] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9f1bf5400000 > [ +0.000001] RBP: 0000000000000001 R08: 00000017818788be R09: ffffadaf0098fb60 > [ +0.000001] R10: ffff9f1bfffdc998 R11: ffffffffa25ec401 R12: ffff9f1b777ff430 > [ +0.000001] R13: ffffffffc06558b8 R14: ffffffffc06853c0 R15: ffffffffc0685520 > [ +0.000002] FS: 00007f2862deae80(0000) GS:ffff9f1bfc980000(0000) knlGS:0000000000000000 > [ +0.000001] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ +0.000001] CR2: 0000000000000000 CR3: 00000000baf6c000 CR4: 00000000000006e0 > [ +0.000004] Call Trace: > [ +0.000023] drm_atomic_helper_commit_modeset_enables+0x15d/0x1e0 [drm_kms_helper] > [ +0.000028] drm_atomic_helper_commit_tail+0x31/0x60 [drm_kms_helper] > [ +0.000008] commit_tail+0x58/0x70 [drm_kms_helper] > [ +0.000008] drm_atomic_helper_commit+0x103/0x110 [drm_kms_helper] > [ +0.000040] drm_mode_atomic_ioctl+0x829/0x950 [drm] > [ +0.000012] ? drm_atomic_set_property+0x960/0x960 [drm] > [ +0.000029] drm_ioctl_kernel+0xb2/0xf0 [drm] > [ +0.000011] drm_ioctl+0x25f/0x3f0 [drm] > [ +0.000009] ? drm_atomic_set_property+0x960/0x960 [drm] > [ +0.000004] ? n_tty_open+0xa0/0xa0 > [ +0.000004] do_vfs_ioctl+0xa4/0x630 > [ +0.000003] ksys_ioctl+0x60/0x90 > [ +0.000003] ? ksys_write+0x4f/0xb0 > [ +0.000002] __x64_sys_ioctl+0x16/0x20 > [ +0.000003] do_syscall_64+0x5b/0x170 > [ +0.000003] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ +0.000002] RIP: 0033:0x7f286677580b > [ +0.000001] Code: 0f 1e fa 48 8b 05 55 b6 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff This call stack looks strange! drm_atomic_set_property() (if that is right function being called) should not end up calling drm_atomic_helper_commit(). I would start by trying to get the actual call function trace to see what is going on. Best regards, Liviu > > Notice that Vaddr became NULL for some reason that I cannot understand. > From the userspace perspective it looks like that drmModeAtomicCommit() > is the entry point for this problem. I tried to debug many things > related to GEM and atomic commit, and the only thing that is worth to > highlight here is the fact that vkms_gem_fault() function is invoked > 1500 times when the test writeback-check-output is executed. > > After many attempts to try to find out the root of the problem, I’m out > of ideas. In this sense, I would like to know if anyone can help by > shedding some light on any issues I'm missing or by pointing at any > other direction. > > Best Regards > Rodrigo Siqueira > > --- > drivers/gpu/drm/vkms/vkms_crtc.c | 5 + > drivers/gpu/drm/vkms/vkms_drv.c | 8 ++ > drivers/gpu/drm/vkms/vkms_drv.h | 27 ++++++ > drivers/gpu/drm/vkms/vkms_gem.c | 2 +- > drivers/gpu/drm/vkms/vkms_output.c | 145 ++++++++++++++++++++++++++++- > drivers/gpu/drm/vkms/vkms_plane.c | 2 +- > 6 files changed, 184 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c > index 8a9aeb0a9ea8..e54265a1c67d 100644 > --- a/drivers/gpu/drm/vkms/vkms_crtc.c > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c > @@ -19,6 +19,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) > if (!ret) > DRM_ERROR("vkms failure on handling vblank"); > > + if (output->wb_state == MW_ONESHOT) { > + drm_writeback_signal_completion(&output->wb_connector, 0); > + output->wb_state = MW_STOP; > + } > + > if (state && output->crc_enabled) { > u64 frame = drm_crtc_accurate_vblank_count(crtc); > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 738dd6206d85..e0d7f94e3972 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -29,6 +29,10 @@ bool enable_cursor; > module_param_named(enable_cursor, enable_cursor, bool, 0444); > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support"); > > +int enable_writeback; > +module_param_named(enable_writeback, enable_writeback, int, 0444); > +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector"); > + > static const struct file_operations vkms_driver_fops = { > .owner = THIS_MODULE, > .open = drm_open, > @@ -123,6 +127,10 @@ static int __init vkms_init(void) > goto out_fini; > } > > + vkms_device->output.writeback_enabled = enable_writeback; > + if (enable_writeback) > + DRM_INFO("Writeback connector enabled"); > + > ret = vkms_modeset_init(vkms_device); > if (ret) > goto out_fini; > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 81f1cfbeb936..d11dcf6b5d55 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -8,6 +8,7 @@ > #include <drm/drm_gem.h> > #include <drm/drm_encoder.h> > #include <linux/hrtimer.h> > +#include <drm/drm_writeback.h> > > #define XRES_MIN 20 > #define YRES_MIN 20 > @@ -20,6 +21,11 @@ > > extern bool enable_cursor; > > +enum { > + MW_STOP = 0, > + MW_ONESHOT, > +}; > + > static const u32 vkms_formats[] = { > DRM_FORMAT_XRGB8888, > }; > @@ -46,6 +52,16 @@ struct vkms_plane_state { > struct vkms_crc_data *crc_data; > }; > > +/** > + * vkms_connector_state - Driver connector specific state > + * @base: base connector state > + * @writeback_vaddr: keep track for writeback framebuffer destination > + */ > +struct vkms_connector_state { > + struct drm_connector_state base; > + void *writeback_vaddr; > +}; > + > /** > * vkms_crtc_state - Driver specific CRTC state > * @base: base CRTC state > @@ -64,10 +80,13 @@ struct vkms_output { > struct drm_crtc crtc; > struct drm_encoder encoder; > struct drm_connector connector; > + struct drm_writeback_connector wb_connector; > struct hrtimer vblank_hrtimer; > ktime_t period_ns; > struct drm_pending_vblank_event *event; > bool crc_enabled; > + int writeback_enabled; > + u8 wb_state; > /* ordered wq for crc_work */ > struct workqueue_struct *crc_workq; > /* protects concurrent access to crc_data */ > @@ -93,6 +112,12 @@ struct vkms_gem_object { > #define drm_crtc_to_vkms_output(target) \ > container_of(target, struct vkms_output, crtc) > > +#define drm_connector_to_wb_connector(target) \ > + container_of(target, struct drm_writeback_connector, base) > + > +#define drm_wb_to_vkms_output(target) \ > + container_of(target, struct vkms_output, wb_connector) > + > #define drm_device_to_vkms_device(target) \ > container_of(target, struct vkms_device, drm) > > @@ -105,6 +130,8 @@ struct vkms_gem_object { > #define to_vkms_plane_state(target)\ > container_of(target, struct vkms_plane_state, base) > > +#define to_wb_state(_state) (struct vkms_connector_state *)(_state) > + > /* CRTC */ > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > struct drm_plane *primary, struct drm_plane *cursor); > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c > index 138b0bb325cf..9e3d25de6aba 100644 > --- a/drivers/gpu/drm/vkms/vkms_gem.c > +++ b/drivers/gpu/drm/vkms/vkms_gem.c > @@ -51,7 +51,7 @@ vm_fault_t vkms_gem_fault(struct vm_fault *vmf) > page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT; > num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE); > > - if (page_offset > num_pages) > + if (page_offset >= num_pages) > return VM_FAULT_SIGBUS; > > mutex_lock(&obj->pages_lock); > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > index 3b162b25312e..66b25c380f21 100644 > --- a/drivers/gpu/drm/vkms/vkms_output.c > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -3,6 +3,8 @@ > #include "vkms_drv.h" > #include <drm/drm_atomic_helper.h> > #include <drm/drm_probe_helper.h> > +#include <drm/drm_writeback.h> > +#include <drm/drm_gem_framebuffer_helper.h> > > static void vkms_connector_destroy(struct drm_connector *connector) > { > @@ -10,6 +12,51 @@ static void vkms_connector_destroy(struct drm_connector *connector) > drm_connector_cleanup(connector); > } > > +static void vkms_wb_connector_reset(struct drm_connector *connector) > +{ > + struct vkms_connector_state *wb_state; > + > + wb_state = kzalloc(sizeof(*wb_state), GFP_KERNEL); > + if (!wb_state) { > + DRM_ERROR("Failed to allocate memory for connector state"); > + return; > + } > + > + if (connector->state) > + __drm_atomic_helper_connector_destroy_state(connector->state); > + > + kfree(connector->state); > + __drm_atomic_helper_connector_reset(connector, &wb_state->base); > +} > + > +static struct drm_connector_state * > +vkms_wb_connector_duplicate_state(struct drm_connector *connector) > +{ > + struct vkms_connector_state *wb_state, *wb_current_state; > + > + if (WARN_ON(!connector->state)) > + return NULL; > + > + wb_state = kzalloc(sizeof(*wb_state), GFP_KERNEL); > + if (!wb_state) > + return NULL; > + > + wb_current_state = to_wb_state(connector->state); > + wb_state->writeback_vaddr = wb_current_state->writeback_vaddr; > + > + __drm_atomic_helper_connector_duplicate_state(connector, &wb_state->base); > + > + return &wb_state->base; > +} > + > +static const struct drm_connector_funcs vkms_wb_connector_funcs = { > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = vkms_connector_destroy, > + .reset = vkms_wb_connector_reset, > + .atomic_duplicate_state = vkms_wb_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > static const struct drm_connector_funcs vkms_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > .destroy = vkms_connector_destroy, > @@ -18,8 +65,37 @@ static const struct drm_connector_funcs vkms_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > -static const struct drm_encoder_funcs vkms_encoder_funcs = { > - .destroy = drm_encoder_cleanup, > +static int vkms_wb_encoder_atomic_check(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct drm_framebuffer *fb; > + > + if (!conn_state->writeback_job || !conn_state->writeback_job->fb) > + return 0; > + > + fb = conn_state->writeback_job->fb; > + if (fb->width != crtc_state->mode.hdisplay || > + fb->height != crtc_state->mode.vdisplay) { > + DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n", > + fb->width, fb->height); > + return -EINVAL; > + } > + > + if (fb->format->format != DRM_FORMAT_XRGB8888) { > + struct drm_format_name_buf format_name; > + > + DRM_DEBUG_KMS("Invalid pixel format %s\n", > + drm_get_format_name(fb->format->format, > + &format_name)); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static const struct drm_encoder_helper_funcs vkms_wb_encoder_helper_funcs = { > + .atomic_check = vkms_wb_encoder_atomic_check, > }; > > static int vkms_conn_get_modes(struct drm_connector *connector) > @@ -32,10 +108,66 @@ static int vkms_conn_get_modes(struct drm_connector *connector) > return count; > } > > +static void vkms_wb_atomic_commit(struct drm_connector *conn, > + struct drm_connector_state *state) > +{ > + struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev); > + struct vkms_output *output = &vkmsdev->output; > + struct drm_writeback_connector *wb_conn = &output->wb_connector; > + struct drm_connector_state *conn_state = wb_conn->base.state; > + struct vkms_connector_state *vkms_conn = to_wb_state(conn_state); > + > + if (!conn_state) > + return; > + > + if (conn_state->writeback_job && conn_state->writeback_job->fb) { > + struct drm_framebuffer *fb = conn_state->writeback_job->fb; > + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); > + struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj); > + > + vkms_conn->writeback_vaddr = kzalloc(vkms_obj->gem.size, GFP_KERNEL); > + > + drm_writeback_queue_job(wb_conn, conn_state->writeback_job); > + conn_state->writeback_job = NULL; > + > + output->wb_state = MW_ONESHOT; > +pr_info("VKMS_OBJ (%d): Buff (%d) - Vaddr (%d) - Size = %ld - count = %d", vkms_obj != NULL, > + vkms_conn->writeback_vaddr != NULL, vkms_obj->vaddr != NULL, vkms_obj->gem.size, > + vkms_obj->vmap_count); > + > + memcpy(vkms_conn->writeback_vaddr, vkms_obj->vaddr, vkms_obj->gem.size); > + } else { > + output->wb_state = MW_STOP; > + } > +} > + > static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > .get_modes = vkms_conn_get_modes, > }; > > +static const struct drm_encoder_funcs vkms_encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = { > + .get_modes = vkms_conn_get_modes, > + .atomic_commit = vkms_wb_atomic_commit, > +}; > + > +static int enable_writeback_connector(struct vkms_device *vkmsdev) > +{ > + struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector; > + > + vkmsdev->output.wb_connector.encoder.possible_crtcs = 1; > + drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs); > + > + return drm_writeback_connector_init(&vkmsdev->drm, wb, > + &vkms_wb_connector_funcs, > + &vkms_wb_encoder_helper_funcs, > + vkms_formats, > + ARRAY_SIZE(vkms_formats)); > +} > + > int vkms_output_init(struct vkms_device *vkmsdev) > { > struct vkms_output *output = &vkmsdev->output; > @@ -77,13 +209,14 @@ int vkms_output_init(struct vkms_device *vkmsdev) > goto err_connector_register; > } > > + encoder->possible_crtcs = 1; > + > ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, > DRM_MODE_ENCODER_VIRTUAL, NULL); > if (ret) { > DRM_ERROR("Failed to init encoder\n"); > goto err_encoder; > } > - encoder->possible_crtcs = 1; > > ret = drm_connector_attach_encoder(connector, encoder); > if (ret) { > @@ -91,6 +224,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) > goto err_attach; > } > > + if (vkmsdev->output.writeback_enabled) { > + ret = enable_writeback_connector(vkmsdev); > + if (ret) > + DRM_ERROR("Failed to init writeback connector\n"); > + } > + > drm_mode_config_reset(dev); > > return 0; > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > index 0e67d2d42f0c..14259ffe47d8 100644 > --- a/drivers/gpu/drm/vkms/vkms_plane.c > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > funcs = &vkms_primary_helper_funcs; > } > > - ret = drm_universal_plane_init(dev, plane, 0, > + ret = drm_universal_plane_init(dev, plane, 1, > &vkms_plane_funcs, > formats, nformats, > NULL, type, NULL); > -- > 2.21.0 > > > -- > Rodrigo Siqueira > https://siqueira.tech
On Wed, Apr 03, 2019 at 04:35:11PM -0300, Rodrigo Siqueira wrote: > Hi, > > I’m working to add writeback support to vkms; you can see my current > patch at the end of this email (some prints highlight the issue that I’m > asking for help here). I’m using the Liviu Dudau and Brian Starkey IGT > patchset, which can be seen here: > https://patchwork.freedesktop.org/series/39229/. > > I’m stuck with a problem that happens in my vkms_wb_atomic_commit() > function, which is a helper function at drm_connector_helper_funcs. You > can see the full implementation of this function at the end of this > email, but here is the important part: > > struct drm_framebuffer *fb = conn_state->writeback_job->fb; > struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); > struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj); > > A few lines after this variable initialization, I try to make a memory > copy like this: > > memcpy(vkms_conn->writeback_vaddr, vkms_obj->vaddr, vkms_obj->gem.size); > > If I test my implementation with the subtest writeback-fb-id, everything > work as expected; for extra details, this is the dmesg output after this > test and with a debug print: > > => writeback-fb-id > > [Apr 3 11:30] CIFS: Attempting to mount //10.0.2.4/qemu > [ +1.601900] Console: switching to colour dummy device 80x25 > [ +0.000017] [IGT] kms_writeback: executing > [ +0.032532] [IGT] kms_writeback: starting subtest writeback-fb-id > [ +0.001932] VKMS_OBJ (1): Buff (1) - Vaddr (1) - Size = 1228800 - count = 1 > [ +0.050524] [IGT] kms_writeback: exiting, ret=0 > [ +0.005202] Console: switching to colour frame buffer device 100x37 > > In the above log, you can see the print info; notice that Vaddr is the > output of vkms_obj->vaddr (Vaddr -> vkms_obj->vaddr != NULL). On the > other hand, if I try the subtest writeback-check-output, something weird > happens in this function. Here is the dmesg output for this test: > > => writeback-check-output > > [ +29.911185] Console: switching to colour dummy device 80x25 > [ +0.000017] [IGT] kms_writeback: executing > [ +0.021289] [IGT] kms_writeback: starting subtest writeback-check-output > [ +0.016270] VKMS_OBJ (1): Buff (1) - Vaddr (0) - Size = 1228800 - count = 0 > [ +0.000005] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > [ +0.000004] #PF error: [normal kernel read fault] > [ +0.000001] PGD 0 P4D 0 > [ +0.000003] Oops: 0000 [#1] PREEMPT SMP PTI > [ +0.000002] CPU: 3 PID: 426 Comm: kms_writeback Not tainted 5.0.0-VKMS-RULES+ #102 > [ +0.000002] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014 > [ +0.000021] RIP: 0010:__memcpy+0x12/0x20 > [ +0.000002] Code: ff 0f 31 48 c1 e2 20 48 09 c2 48 31 d3 e9 79 ff ff ff 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 f3 a4 > [ +0.000001] RSP: 0018:ffffadaf0098fbd8 EFLAGS: 00010246 > [ +0.000002] RAX: ffff9f1bf5400000 RBX: ffff9f1bfa28df00 RCX: 0000000000025800 > [ +0.000001] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9f1bf5400000 > [ +0.000001] RBP: 0000000000000001 R08: 00000017818788be R09: ffffadaf0098fb60 > [ +0.000001] R10: ffff9f1bfffdc998 R11: ffffffffa25ec401 R12: ffff9f1b777ff430 > [ +0.000001] R13: ffffffffc06558b8 R14: ffffffffc06853c0 R15: ffffffffc0685520 > [ +0.000002] FS: 00007f2862deae80(0000) GS:ffff9f1bfc980000(0000) knlGS:0000000000000000 > [ +0.000001] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ +0.000001] CR2: 0000000000000000 CR3: 00000000baf6c000 CR4: 00000000000006e0 > [ +0.000004] Call Trace: > [ +0.000023] drm_atomic_helper_commit_modeset_enables+0x15d/0x1e0 [drm_kms_helper] > [ +0.000028] drm_atomic_helper_commit_tail+0x31/0x60 [drm_kms_helper] > [ +0.000008] commit_tail+0x58/0x70 [drm_kms_helper] > [ +0.000008] drm_atomic_helper_commit+0x103/0x110 [drm_kms_helper] > [ +0.000040] drm_mode_atomic_ioctl+0x829/0x950 [drm] > [ +0.000012] ? drm_atomic_set_property+0x960/0x960 [drm] > [ +0.000029] drm_ioctl_kernel+0xb2/0xf0 [drm] > [ +0.000011] drm_ioctl+0x25f/0x3f0 [drm] > [ +0.000009] ? drm_atomic_set_property+0x960/0x960 [drm] > [ +0.000004] ? n_tty_open+0xa0/0xa0 > [ +0.000004] do_vfs_ioctl+0xa4/0x630 > [ +0.000003] ksys_ioctl+0x60/0x90 > [ +0.000003] ? ksys_write+0x4f/0xb0 > [ +0.000002] __x64_sys_ioctl+0x16/0x20 > [ +0.000003] do_syscall_64+0x5b/0x170 > [ +0.000003] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ +0.000002] RIP: 0033:0x7f286677580b > [ +0.000001] Code: 0f 1e fa 48 8b 05 55 b6 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff > > Notice that Vaddr became NULL for some reason that I cannot understand. Just missing the .prepare_writeback_job() hook?
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..e54265a1c67d 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -19,6 +19,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) if (!ret) DRM_ERROR("vkms failure on handling vblank"); + if (output->wb_state == MW_ONESHOT) { + drm_writeback_signal_completion(&output->wb_connector, 0); + output->wb_state = MW_STOP; + } + if (state && output->crc_enabled) { u64 frame = drm_crtc_accurate_vblank_count(crtc); diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 738dd6206d85..e0d7f94e3972 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -29,6 +29,10 @@ bool enable_cursor; module_param_named(enable_cursor, enable_cursor, bool, 0444); MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support"); +int enable_writeback; +module_param_named(enable_writeback, enable_writeback, int, 0444); +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector"); + static const struct file_operations vkms_driver_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -123,6 +127,10 @@ static int __init vkms_init(void) goto out_fini; } + vkms_device->output.writeback_enabled = enable_writeback; + if (enable_writeback) + DRM_INFO("Writeback connector enabled"); + ret = vkms_modeset_init(vkms_device); if (ret) goto out_fini; diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 81f1cfbeb936..d11dcf6b5d55 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -8,6 +8,7 @@ #include <drm/drm_gem.h> #include <drm/drm_encoder.h> #include <linux/hrtimer.h> +#include <drm/drm_writeback.h> #define XRES_MIN 20 #define YRES_MIN 20 @@ -20,6 +21,11 @@ extern bool enable_cursor; +enum { + MW_STOP = 0, + MW_ONESHOT, +}; + static const u32 vkms_formats[] = { DRM_FORMAT_XRGB8888, }; @@ -46,6 +52,16 @@ struct vkms_plane_state { struct vkms_crc_data *crc_data; }; +/** + * vkms_connector_state - Driver connector specific state + * @base: base connector state + * @writeback_vaddr: keep track for writeback framebuffer destination + */ +struct vkms_connector_state { + struct drm_connector_state base; + void *writeback_vaddr; +}; + /** * vkms_crtc_state - Driver specific CRTC state * @base: base CRTC state @@ -64,10 +80,13 @@ struct vkms_output { struct drm_crtc crtc; struct drm_encoder encoder; struct drm_connector connector; + struct drm_writeback_connector wb_connector; struct hrtimer vblank_hrtimer; ktime_t period_ns; struct drm_pending_vblank_event *event; bool crc_enabled; + int writeback_enabled; + u8 wb_state; /* ordered wq for crc_work */ struct workqueue_struct *crc_workq; /* protects concurrent access to crc_data */ @@ -93,6 +112,12 @@ struct vkms_gem_object { #define drm_crtc_to_vkms_output(target) \ container_of(target, struct vkms_output, crtc) +#define drm_connector_to_wb_connector(target) \ + container_of(target, struct drm_writeback_connector, base) + +#define drm_wb_to_vkms_output(target) \ + container_of(target, struct vkms_output, wb_connector) + #define drm_device_to_vkms_device(target) \ container_of(target, struct vkms_device, drm) @@ -105,6 +130,8 @@ struct vkms_gem_object { #define to_vkms_plane_state(target)\ container_of(target, struct vkms_plane_state, base) +#define to_wb_state(_state) (struct vkms_connector_state *)(_state) + /* CRTC */ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, struct drm_plane *cursor); diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c index 138b0bb325cf..9e3d25de6aba 100644 --- a/drivers/gpu/drm/vkms/vkms_gem.c +++ b/drivers/gpu/drm/vkms/vkms_gem.c @@ -51,7 +51,7 @@ vm_fault_t vkms_gem_fault(struct vm_fault *vmf) page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT; num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE); - if (page_offset > num_pages) + if (page_offset >= num_pages) return VM_FAULT_SIGBUS; mutex_lock(&obj->pages_lock); diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 3b162b25312e..66b25c380f21 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -3,6 +3,8 @@ #include "vkms_drv.h" #include <drm/drm_atomic_helper.h> #include <drm/drm_probe_helper.h> +#include <drm/drm_writeback.h> +#include <drm/drm_gem_framebuffer_helper.h> static void vkms_connector_destroy(struct drm_connector *connector) { @@ -10,6 +12,51 @@ static void vkms_connector_destroy(struct drm_connector *connector) drm_connector_cleanup(connector); } +static void vkms_wb_connector_reset(struct drm_connector *connector) +{ + struct vkms_connector_state *wb_state; + + wb_state = kzalloc(sizeof(*wb_state), GFP_KERNEL); + if (!wb_state) { + DRM_ERROR("Failed to allocate memory for connector state"); + return; + } + + if (connector->state) + __drm_atomic_helper_connector_destroy_state(connector->state); + + kfree(connector->state); + __drm_atomic_helper_connector_reset(connector, &wb_state->base); +} + +static struct drm_connector_state * +vkms_wb_connector_duplicate_state(struct drm_connector *connector) +{ + struct vkms_connector_state *wb_state, *wb_current_state; + + if (WARN_ON(!connector->state)) + return NULL; + + wb_state = kzalloc(sizeof(*wb_state), GFP_KERNEL); + if (!wb_state) + return NULL; + + wb_current_state = to_wb_state(connector->state); + wb_state->writeback_vaddr = wb_current_state->writeback_vaddr; + + __drm_atomic_helper_connector_duplicate_state(connector, &wb_state->base); + + return &wb_state->base; +} + +static const struct drm_connector_funcs vkms_wb_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = vkms_connector_destroy, + .reset = vkms_wb_connector_reset, + .atomic_duplicate_state = vkms_wb_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + static const struct drm_connector_funcs vkms_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vkms_connector_destroy, @@ -18,8 +65,37 @@ static const struct drm_connector_funcs vkms_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -static const struct drm_encoder_funcs vkms_encoder_funcs = { - .destroy = drm_encoder_cleanup, +static int vkms_wb_encoder_atomic_check(struct drm_encoder *encoder, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_framebuffer *fb; + + if (!conn_state->writeback_job || !conn_state->writeback_job->fb) + return 0; + + fb = conn_state->writeback_job->fb; + if (fb->width != crtc_state->mode.hdisplay || + fb->height != crtc_state->mode.vdisplay) { + DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n", + fb->width, fb->height); + return -EINVAL; + } + + if (fb->format->format != DRM_FORMAT_XRGB8888) { + struct drm_format_name_buf format_name; + + DRM_DEBUG_KMS("Invalid pixel format %s\n", + drm_get_format_name(fb->format->format, + &format_name)); + return -EINVAL; + } + + return 0; +} + +static const struct drm_encoder_helper_funcs vkms_wb_encoder_helper_funcs = { + .atomic_check = vkms_wb_encoder_atomic_check, }; static int vkms_conn_get_modes(struct drm_connector *connector) @@ -32,10 +108,66 @@ static int vkms_conn_get_modes(struct drm_connector *connector) return count; } +static void vkms_wb_atomic_commit(struct drm_connector *conn, + struct drm_connector_state *state) +{ + struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev); + struct vkms_output *output = &vkmsdev->output; + struct drm_writeback_connector *wb_conn = &output->wb_connector; + struct drm_connector_state *conn_state = wb_conn->base.state; + struct vkms_connector_state *vkms_conn = to_wb_state(conn_state); + + if (!conn_state) + return; + + if (conn_state->writeback_job && conn_state->writeback_job->fb) { + struct drm_framebuffer *fb = conn_state->writeback_job->fb; + struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0); + struct vkms_gem_object *vkms_obj = drm_gem_to_vkms_gem(gem_obj); + + vkms_conn->writeback_vaddr = kzalloc(vkms_obj->gem.size, GFP_KERNEL); + + drm_writeback_queue_job(wb_conn, conn_state->writeback_job); + conn_state->writeback_job = NULL; + + output->wb_state = MW_ONESHOT; +pr_info("VKMS_OBJ (%d): Buff (%d) - Vaddr (%d) - Size = %ld - count = %d", vkms_obj != NULL, + vkms_conn->writeback_vaddr != NULL, vkms_obj->vaddr != NULL, vkms_obj->gem.size, + vkms_obj->vmap_count); + + memcpy(vkms_conn->writeback_vaddr, vkms_obj->vaddr, vkms_obj->gem.size); + } else { + output->wb_state = MW_STOP; + } +} + static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { .get_modes = vkms_conn_get_modes, }; +static const struct drm_encoder_funcs vkms_encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = { + .get_modes = vkms_conn_get_modes, + .atomic_commit = vkms_wb_atomic_commit, +}; + +static int enable_writeback_connector(struct vkms_device *vkmsdev) +{ + struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector; + + vkmsdev->output.wb_connector.encoder.possible_crtcs = 1; + drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs); + + return drm_writeback_connector_init(&vkmsdev->drm, wb, + &vkms_wb_connector_funcs, + &vkms_wb_encoder_helper_funcs, + vkms_formats, + ARRAY_SIZE(vkms_formats)); +} + int vkms_output_init(struct vkms_device *vkmsdev) { struct vkms_output *output = &vkmsdev->output; @@ -77,13 +209,14 @@ int vkms_output_init(struct vkms_device *vkmsdev) goto err_connector_register; } + encoder->possible_crtcs = 1; + ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) { DRM_ERROR("Failed to init encoder\n"); goto err_encoder; } - encoder->possible_crtcs = 1; ret = drm_connector_attach_encoder(connector, encoder); if (ret) { @@ -91,6 +224,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) goto err_attach; } + if (vkmsdev->output.writeback_enabled) { + ret = enable_writeback_connector(vkmsdev); + if (ret) + DRM_ERROR("Failed to init writeback connector\n"); + } + drm_mode_config_reset(dev); return 0; diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 0e67d2d42f0c..14259ffe47d8 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, funcs = &vkms_primary_helper_funcs; } - ret = drm_universal_plane_init(dev, plane, 0, + ret = drm_universal_plane_init(dev, plane, 1, &vkms_plane_funcs, formats, nformats, NULL, type, NULL);