[igt-dev] [PATCH V6 i-g-t 1/6] lib/igt_kms: Add writeback support

Rodrigo Siqueira rodrigosiqueiramelo at gmail.com
Tue Jul 9 15:07:56 UTC 2019


On Tue, Jul 9, 2019 at 11:42 AM Ser, Simon <simon.ser at intel.com> wrote:
>
> On Tue, 2019-07-09 at 11:32 -0300, Rodrigo Siqueira wrote:
> > On Wed, Jul 3, 2019 at 9:15 AM Ser, Simon <simon.ser at intel.com> wrote:
> > > On Tue, 2019-06-18 at 18:56 -0300, Rodrigo Siqueira wrote:
> > > > On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau at arm.com> wrote:
> > > > > On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > > > > > Add support in igt_kms for writeback connectors, with the ability
> > > > > > to attach framebuffers.
> > > > > >
> > > > > > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > > > > > drmModeGetResources()
> > > > >
> > > > > Reviewed-by: Liviu Dudau <liviu.dudau at arm.com>
> > > > >
> > > > > Thanks for updating this! Given that I have done the last changes and this is
> > > > > mostly a refresh, not sure if I should add more Reviewed-by's to the other
> > > > > patches.
> > > >
> > > > Thanks Liviu!
> > > >
> > > > I just forgot to add my SoB, and for some reason, gmail does not allow
> > > > me to send an email on someone behalf.
> > >
> > > FWIW, that's a good thing, and it's required to pass DMARC checks.
> > >
> > > Instead, git-send-email should add a From line at the beginning of the
> > > message when sending a patch on behalf of someone else. I wonder what
> > > happened here.
> >
> > Thank you for your help.
> >
> > I’m using neomutt for sending patches, and I’ll take a look at
> > git-send-email. Additionally, it looks like Gmail requires that I add
> > a new account in order to allow me to send patches on someone behalf.
> > I’ll take some time to read about this issue, and I will try to resend
> > the patchset again.
>
> When sending a patch on someone else's behalf, you shouldn't send the
> e-mail with the sender (From header) set to someone else. You should
> use your normal address. The From line in the body of the message will
> make Git understand the author is someone else.
>
> So no new account setup required.
>
> I really recommend using git-send-email for patches. There are too many
> ways to make a mistake if you try sending emails manually. Here is a
> tutorial:
> https://git-send-email.io/

Thank you very much for the tutorial! I'll read it and prepare to
migrate from 'neomutt -H' to git-send-mail.

> > Btw, if you have time, could you take a look in this series?
>
> Sure, I've already began looking at the series. I'll continue soon.

Thank you again for your help.

> Thanks for your work!
>
> > Best regards
> >
> > > > Btw, I can fix it after
> > > > everybody agrees that the kms_writeback is ready for landing.
> > > >
> > > >
> > > > > Best regards,
> > > > > Liviu
> > > > >
> > > > > > Signed-off-by: Brian Starkey <brian.starkey at arm.com>
> > > > > > [rebased and updated to the latest igt style]
> > > > > > Signed-off-by: Liviu Dudau <liviu.dudau at arm.com>
> > > > > > ---
> > > > > >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  lib/igt_kms.h |  6 ++++++
> > > > > >  2 files changed, 63 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > > > index da188a39..140db346 100644
> > > > > > --- a/lib/igt_kms.c
> > > > > > +++ b/lib/igt_kms.c
> > > > > > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > > > >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> > > > > >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> > > > > >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> > > > > >  };
> > > > > >
> > > > > >  /*
> > > > > > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> > > > > >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> > > > > >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> > > > > >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > > > > > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> > > > > >       {}
> > > > > >  };
> > > > > >
> > > > > > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> > > > > >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> > > > > >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> > > > > >                                         BROADCAST_RGB_FULL);
> > > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > > > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > > > > > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /**
> > > > > > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> > > > > >   * For outputs:
> > > > > >   * - %IGT_CONNECTOR_CRTC_ID
> > > > > >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > > > > > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > > > > > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> > > > > >   * - igt_output_override_mode() to default.
> > > > > >   *
> > > > > >   * For pipes:
> > > > > > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > > > > >
> > > > > >       display->drm_fd = drm_fd;
> > > > > >
> > > > > > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > > > > +
> > > > > >       resources = drmModeGetResources(display->drm_fd);
> > > > > >       if (!resources)
> > > > > >               goto out;
> > > > > > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> > > > > >       kmstest_free_connector_config(&output->config);
> > > > > >       free(output->name);
> > > > > >       output->name = NULL;
> > > > > > +
> > > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > > +             close(output->writeback_out_fence_fd);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /**
> > > > > > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> > > > > >                                         output->props[i],
> > > > > >                                         output->values[i]));
> > > > > >       }
> > > > > > +
> > > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > > +             close(output->writeback_out_fence_fd);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /*
> > > > > > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > > > > >               else
> > > > > >                       /* no modeset in universal commit, no change to crtc. */
> > > > > >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > > > > > +
> > > > > > +             if (s == COMMIT_ATOMIC) {
> > > > > > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > > > > > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > > > > > +
> > > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > > +             }
> > > > > >       }
> > > > > >
> > > > > >       if (display->first_commit) {
> > > > > > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> > > > > >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> > > > > >  }
> > > > > >
> > > > > > +/**
> > > > > > + * 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);
> > > > > > +}
> > > > > > +
> > > > > >  /**
> > > > > >   * igt_wait_for_vblank_count:
> > > > > >   * @drm_fd: A drm file descriptor
> > > > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > > > index a448a003..cacc6b90 100644
> > > > > > --- a/lib/igt_kms.h
> > > > > > +++ b/lib/igt_kms.h
> > > > > > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> > > > > >         IGT_CONNECTOR_BROADCAST_RGB,
> > > > > >         IGT_CONNECTOR_CONTENT_PROTECTION,
> > > > > >         IGT_CONNECTOR_VRR_CAPABLE,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > >         IGT_NUM_CONNECTOR_PROPS
> > > > > >  };
> > > > > >
> > > > > > @@ -364,6 +367,8 @@ typedef struct {
> > > > > >       bool use_override_mode;
> > > > > >       drmModeModeInfo override_mode;
> > > > > >
> > > > > > +     int32_t writeback_out_fence_fd;
> > > > > > +
> > > > > >       /* bitmask of changed properties */
> > > > > >       uint64_t changed;
> > > > > >
> > > > > > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> > > > > >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> > > > > >      drmModeConnector *connector);
> > > > > >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> > > > > >
> > > > > >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > > >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > > > --
> > > > > > 2.21.0
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Rodrigo Siqueira
> > > > > > https://siqueira.tech
> > > > >
> > > > > --
> > > > > ====================
> > > > > > I would like to |
> > > > > > fix the world,  |
> > > > > > but they're not |
> > > > > > giving me the   |
> > > > >  \ source code!  /
> > > > >   ---------------
> > > > >     ¯\_(ツ)_/¯
> >
> >



-- 

Rodrigo Siqueira
https://siqueira.tech


More information about the igt-dev mailing list