Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata related fixes (Rebased) #107

Closed
wants to merge 1 commit into from
Closed

Conversation

jnzw
Copy link
Contributor

@jnzw jnzw commented May 26, 2022

This PR is basically #94 rebased on top of #105. Conflicts that occur in max9295.c and max9296.c have been resolved.

@jnzw
Copy link
Contributor Author

jnzw commented May 26, 2022

@xzhangxa Could you please review?

@jnzw jnzw marked this pull request as draft May 26, 2022 03:15
kernel/nvidia/0057-Metadata-related-fixes.patch Outdated Show resolved Hide resolved
kernel/nvidia/0057-Metadata-related-fixes.patch Outdated Show resolved Hide resolved
@xzhangxa xzhangxa requested a review from dmipx May 26, 2022 03:16
- Make metadata buffer smaller so copy on it could waste less CPU;
- IR channel changed from 2 SerDes configs (Y8_Y8I, Y12I) to 3 configs
  (Y8, Y8I, Y12I), and Y8/Y8I configs enabled metadata datatype, the 2
  datatypes per channel limitation is still respected while we could
  have metadata on IR Y8 and Y8I;
- Enable metadata for IR, /dev/video5 will be IR metadata node;
- Y12I doesn't have metadata, the metadata node /dev/video5 will output
  no data when the format is set to Y12I.
- Metadata related code refactoring in MC/VI driver: cleanup, move
  related code into one place, get config from dt instead of hardcode.

Signed-off-by: Xin Zhang <xin.x.zhang@intel.com>
Signed-off-by: Shikun Ding <shikun.ding@intel.com>
Signed-off-by: Junze Wu <junze.wu@intel.com>
@jnzw jnzw marked this pull request as ready for review May 30, 2022 02:22
@jnzw jnzw mentioned this pull request Jun 2, 2022
@dmipx dmipx self-requested a review June 2, 2022 12:02
@xzhangxa
Copy link
Collaborator

Hi @dmipx @ev-mp, this was not merged so I guess it's your decision not having metadata for IR (Y8, Y8I) stream. However there were a few changes are useful no matter enabling metadata of IR or not: the VI code change cleanup, reading config from device tree than hardcode, less copy size for metadata etc. It's still nice to have even we turn off metadata for IR.

So is this metadata of IR needed for future release, or not at all? We're doing a lot of device tree changes for the multi-D457 and some changes in this PR are still good to use, so we'd like to know this feature is still needed later? If no we will just move some parts of this.

@dmipx
Copy link
Contributor

dmipx commented Nov 10, 2022

@xzhangxa there is many changes, we will need to do a thorough verification of everything. it will be better to split every orthogonal cchanges to different PR's so we can merge modifications step-by-step.

@xzhangxa
Copy link
Collaborator

@xzhangxa there is many changes, we will need to do a thorough verification of everything. it will be better to split every orthogonal cchanges to different PR's so we can merge modifications step-by-step.

Yes totally agree, we should split this patch, there was history about this patch then and it gradually grew this big. Just a few questions: the metadata for IR feature needed or not? In the following PRs do we need to update for both JetPack 4.6.1 and 5.0.2, or 4.6.1 will be frozen and only 5.0.2 to be updated?

@xzhangxa
Copy link
Collaborator

xzhangxa commented Jan 3, 2023

Close this since the IR MD feature is not needed and other useful code changes are already in JetPack 5 branch and following PRs.

@xzhangxa xzhangxa closed this Jan 3, 2023
@Nir-Az
Copy link
Collaborator

Nir-Az commented Jan 3, 2023

Actually IR metadata is a good feature, we just didn't had the bandwidth to verify and review it..
Is it still relevant with the new JP branch?

@xzhangxa
Copy link
Collaborator

xzhangxa commented Jan 3, 2023

The code was for JetPack 4.6.1 at the time so it's only applicable for 4.6.1.

For current JetPack-5.0.2 branch, a little refactor is needed; for #144 the serdes code is changed a lot but actually it makes it easier to enable IR MD with a few code changes (though we didn't try/test it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants