On Mon, Mar 18, 2019 at 07:05:29PM -0300, Rodrigo Siqueira wrote: > On 03/18, Liviu Dudau wrote: > > On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote: > > > Hi Liviu, > > > > > > I’m using your patchset to guide my implementation of writeback in the > > > VKMS, so, first of all, thanks :) > > > > > > During my work, I noticed that you’re setting the drmSetClientCap() > > > before drmModeGetResources() which make the writeback capability > > > > before? I believe your patchset moves it to be before, which makes > > sense, as the writeback applies to connectors, not planes (sorry for the > > confusion, I think I wrote the code when we were toying with the idea > > that writeback was going to be a plane property). > > > > > ‘invisible’ for drmModeGetResources(). I made the following change, and > > > I could pass the igt_display_require(): > > > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > > > index f4ff3a96..bb2d8e06 100644 > > > --- a/lib/igt_kms.c > > > +++ b/lib/igt_kms.c > > > @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd) > > > > > > display->drm_fd = drm_fd; > > > > > > + drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); > > > + if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) > > > + display->is_atomic = 1; > > > + > > > + drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); > > > + > > > resources = drmModeGetResources(display->drm_fd); > > > if (!resources) > > > goto out; > > > @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd) > > > display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes); > > > igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes); > > > > > > - drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1); > > > - if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) > > > - display->is_atomic = 1; > > > - > > > - drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); > > > - > > > plane_resources = drmModeGetPlaneResources(display->drm_fd); > > > igt_assert(plane_resources); > > > > > > I'm not 100% confident about this issue, because of this I will send an > > > RFC and see if I can get more details about this issue. > > > > It actually looks correct to me, so > > > > Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx> > > > > > > > > Additionally, if you need any help with this patchset I will be glad to > > > help since I’m using it \o/ > > > > Well, I need to send an updated version and if you could review that > > then I can merge it into igt. > > Sure! I will be glad to take some time to review your code :) > > I can also try to test it on my current implementation of VKMS. > One question, I have a NanoPc-T1 device [1] which has a Mali GPU, can I > test the writeback feature in this device? All kind of Mali GPU has > support for the writeback feature? > > 1. http://wiki.friendlyarm.com/wiki/index.php/NanoPC-T1 (sorry, email fell through the cracks) The writeback is a Mali Display Processor feature, not GPU. As such you need a device that has Mali DP in it. Commercially there will be a platform later this year, but until it is announced I cannot say more about it. Best regards, Liviu > > Thanks! > Best Regards > > > Best regards, > > Liviu > > > > > > > > Best Regards > > > Rodrigo Siqueira > > > > > > On 02/04, Brian Starkey wrote: > > > > Hi Liviu, > > > > > > > > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote: > > > > > From: Brian Starkey <brian.starkey@xxxxxxx> > > > > > > > > [snip] > > > > > > > > > > > > > > +/** > > > > > + * igt_output_set_writeback_fb: > > > > > + * @output: Target output > > > > > + * @fb: Target framebuffer > > > > > + * > > > > > + * This function sets the given @fb to be used as the target framebuffer for the > > > > > + * writeback engine at the next atomic commit. It will also request a writeback > > > > > + * out fence that will contain the fd number of the out fence created by KMS if > > > > > + * the given @fb is valid. > > > > > + */ > > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb) > > > > > +{ > > > > > + igt_display_t *display = output->display; > > > > > + > > > > > + LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0); > > > > > + > > > > > + igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0); > > > > > + /* only request a writeback out fence if the framebuffer is valid */ > > > > > + if (fb) > > > > > + igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR, > > > > > + (ptrdiff_t)&output->writeback_out_fence_fd); > > > > > > > > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t) > > > > seems better. > > > > > > > > Thanks, > > > > -Brian > > > > _______________________________________________ > > > > igt-dev mailing list > > > > igt-dev@xxxxxxxxxxxxxxxxxxxxx > > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev > > > > > > -- > > > Rodrigo Siqueira > > > https://siqueira.tech > > > Graduate Student > > > Department of Computer Science > > > University of São Paulo > > > > > > > > -- > > ==================== > > | I would like to | > > | fix the world, | > > | but they're not | > > | giving me the | > > \ source code! / > > --------------- > > ¯\_(ツ)_/¯ > > -- > Rodrigo Siqueira > https://siqueira.tech > Graduate Student > Department of Computer Science > University of São Paulo -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
