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

Set subslice mask according to the per-context sseu value. #271

Merged
merged 2 commits into from
Feb 26, 2019

Conversation

zxye
Copy link
Contributor

@zxye zxye commented Aug 13, 2018

This will shut down the non-VME subslice on ICLLP when VME is used.
Fixes issue #267.

@zxye
Copy link
Contributor Author

zxye commented Sep 27, 2018

This PR requires the slice shutdown i915 uAPI supported by this branch https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=sseu

@zxye
Copy link
Contributor Author

zxye commented Oct 10, 2018

This PR works with this i915 branch: https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=media

@smrowe4
Copy link

smrowe4 commented Oct 18, 2018

I tried this commit along with https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=media. Works for AVC VME encode. Can this commit be upstreamed?

@zxye
Copy link
Contributor Author

zxye commented Oct 29, 2018

I tried this commit along with https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=media. Works for AVC VME encode. Can this commit be upstreamed?

Will upstream the commit after KMD patch is upstreamed.

@ykku16github
Copy link

FYI. In https://cgit.freedesktop.org/~tursulin/drm-intel/tree/include/uapi/drm/i915_drm.h?h=media, I915_CONTEXT_PARAM_SSEU is defined to be 0x8. But in https://patchwork.freedesktop.org/series/48194/ (rev 6), it has been changed to 0x7.

@zxye, your patch may need an update depending on which one gets merged in kernel space.

@zxye
Copy link
Contributor Author

zxye commented Dec 11, 2018

FYI. In https://cgit.freedesktop.org/~tursulin/drm-intel/tree/include/uapi/drm/i915_drm.h?h=media, I915_CONTEXT_PARAM_SSEU is defined to be 0x8. But in https://patchwork.freedesktop.org/series/48194/ (rev 6), it has been changed to 0x7.

@zxye, your patch may need an update depending on which one gets merged in kernel space.

Yes, currently this PR uses the number from https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=media header file. Once the i915 patches are upstreamed, I will update the PR to align with it before merging the PR into master branch. Thanks.

@zxye zxye changed the title Set SSEU configuration according to the per-context createOption. Set subslice mask according to the per-context sseu value. Dec 11, 2018
@dvrogozh
Copy link
Contributor

Please, rebase to fix conflict.

@@ -1069,8 +1069,17 @@ struct drm_i915_gem_execbuffer2 {
* dma fences to wait upon or signal.
*/
#define I915_EXEC_FENCE_ARRAY (1<<19)
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit should be substituted with the official pull of the updated i915 uAPI (once it will land to drm-next) according to the instruction: https://github.com/intel/media-driver/blob/master/media_driver/linux/common/os/i915/include/uapi/README.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can update the PR with "$ make headers_install INSTALL_HDR_PATH=/path/to/install" once the uAPI changes landed drm-next.

}

int
mos_get_subslice_mask(int fd, unsigned int *subslice_mask)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see you use this function. Do we really need it? Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was used at the beginning and then removed later. But it is a thin wrapper of the uAPI, maybe it can be useful in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point to keep the code which we don't exercise. This will give us a burden to support. I would drop.

}

int
mos_get_slice_mask(int fd, unsigned int *slice_mask)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. This function also looks unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the beginning, this PR included the changes to upgrade the SKL slice shutdown with this upstream SSEU uAPI. This function was used for that purpose. If we do not need SKL slice shutdown anymore on master branch, it can be removed. Again, it's also a thin wrapper of the uAPI and could be useful in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point to keep the code which we don't exercise. This will give us a burden to support. I would drop.

And if it is needed by someone - let him bring the whole bunch of code to support SKL slice shutdown rather than few functions for it.

@dvrogozh
Copy link
Contributor

@zxye : I believe SSEU patch series landed upstream and in drm-next. Can you, please, update this PR w/ update 915 uAPI per instruction here: https://github.com/intel/media-driver/blob/master/media_driver/linux/common/os/i915/include/uapi/README? I belive we need to proceed with merging this in.

@zxye
Copy link
Contributor Author

zxye commented Feb 13, 2019

@dvrogozh @XinfengZhang I synced the i915 header with drm_next and rebased the PR to latest master. Please help review and merge if no issue. Thank you.

@XinfengZhang XinfengZhang added the verifying PR: fix ready and verifying with build/test label Feb 14, 2019
@XinfengZhang
Copy link
Contributor

@hanlong1 , test for this PR should also upgrade kernel

Copy link
Contributor

@dvrogozh dvrogozh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, strictly follow instruction here (if not): https://github.com/intel/media-driver/blob/master/media_driver/linux/common/os/i915/include/uapi/README. Once done, update README and list commit which you've picked in drm-next.

}

int
mos_get_subslice_mask(int fd, unsigned int *subslice_mask)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point to keep the code which we don't exercise. This will give us a burden to support. I would drop.

}

int
mos_get_slice_mask(int fd, unsigned int *slice_mask)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point to keep the code which we don't exercise. This will give us a burden to support. I would drop.

And if it is needed by someone - let him bring the whole bunch of code to support SKL slice shutdown rather than few functions for it.

@dvrogozh
Copy link
Contributor

@zxye : I see the change 39ce64a which looks to be this PR. Why we did not merge in #271 and got some change thru other channel?

@Sherry-Lin Sherry-Lin merged commit 5027716 into intel:master Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verifying PR: fix ready and verifying with build/test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants