Re: [PATCH v5 00/21] Host1x sync point UAPI should not be used for tracking DRM jobs

Dmitry Osipenko
2021-02-27T11:20:40Z

03.02.2021 14:18, Mikko Perttunen пишет:
...
>> I'll need more time to think about it.
>>
>
> How about something like this:
>
> Turn the syncpt_incr field back into an array of structs like
>
> #define DRM_TEGRA_SUBMIT_SYNCPT_INCR_REPLACE_SYNCOBJ        (1<<0)
> #define DRM_TEGRA_SUBMIT_SYNCPT_INCR_PATCH_DYNAMIC_SYNCPT    (1<<1)
>
> struct drm_tegra_submit_syncpt_incr {
>     /* can be left as zero if using dynamic syncpt */
>     __u32 syncpt_id;
>     __u32 flags;
>
>     struct {
>         __u32 syncobj;
>         __u32 value;
>     } fence;
>
>     /* patch word as such:
>          * *word = *word | (syncpt_id << shift)
>          */
>     struct {
>         __u32 gather_offset_words;
>         __u32 shift;
>     } patch;
> };
>
> So this will work similarly to the buffer reloc system; the kernel
> driver will allocate a job syncpoint and patch in the syncpoint ID if
> requested, and allows outputting syncobjs for each increment.

I haven't got any great ideas so far, but it feels that will be easier
and cleaner if we could have separate job paths (and job IOCTLS) based
on hardware generation since the workloads a too different. The needs of
a newer h/w are too obscure for me and absence of userspace code,
firmware sources and full h/w documentation do not help.

There still should be quite a lot to share, but things like
mapping-to-channel and VM sync points are too far away from older h/w,
IMO. This means that code path before drm-sched and path for job-timeout
handling should be separate.

Maybe later on it will become cleaner that we actually could unify it
all nicely, but for now it doesn't look like a good idea to me.

Mikko, do you have any objections to trying out variant with the
separate paths?