Re: [Intel-gfx] [PATCH v5 00/13] ICELAKE DSI DRIVER
On Wed, 12 Sep 2018, Madhav Chauhan wrote: > On 9/12/2018 1:05 AM, Jani Nikula wrote: >> On Tue, 10 Jul 2018, Madhav Chauhan wrote: >>> From ICELAKE platform onwards, new MIPI DSI IP controller is integrated to >>> GPU/Display Engine and same could be extended for future Intel platforms as >>> well. >>> DSI IP controller supports MIPI DSI 1.3 and DPHY 1.2 specification. >>> >>> So, a new DSI driver has been added inside I915. >>> >>> Given below patches are the part of new DSI driver which implements BSPEC >>> sequence till transcoder configuration. Rest of the patches published to >>> GITHUB >>> and latest snapshot can be downloaded using: >>> #git clone https://github.com/madhavchauhan/Intel-DSI-Driver.git >>> >>> v2: Addressed review comments from Jani N for Patches 1-6 and rebase for >>> some >>> other few patches. >>> v3: Renamed intel_dsi_new.c to gen11_dsi.c as per discussion with Jani, >>> Daniel, >>> Ville. Also addressed review comments for couple of patches. >>> v4: Rename gen11_dsi.c to icl_dsi.c (Ville). No functional changes. >>> v5: Rebase on drm-tip after initial 7 patches got merged. >> Hi Madhav, I think there's enough review here to warrant a revised >> set. I regret I haven't been able to review this earlier, and I'm now >> throwing the ball back in your court... with the added pressure that I'd >> really like to get this merged for v4.20. Which means the deadline for >> merging is about 1½ weeks away. Is there any chance? > > Agree, i will publish next series soon. > Do you mean to merge these 13 patches to 4.20 or the complete > implementation of 65 patches?? Let's start with these. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ringbuffer: Reload PDs harder on byt/bcs
Quoting Chris Wilson (2018-09-11 08:43:32) > Quoting Chris Wilson (2018-09-10 14:08:08) > > Baytrail takes a little more convincing that it needs to actually reload > > its Page Directoy (ppGTT) before the context switch, so repeat it until > > it gets the message. Once again the arbitrary values here are > > empirically derived. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861 > > Testcase: igt/gem_exec_parallel/fds > > Signed-off-by: Chris Wilson > > On drm-tip, this testcase fails within 15 iterations (30s). It has now > survived 24 hours. I think it've proved itself against this one testcase > and deserves a shot at the rest. 48 hours. Please kindly ack. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active
On 11/09/2018 21:11, Lionel Landwerlin wrote: On 10/09/2018 14:44, Tvrtko Ursulin wrote: On 07/09/2018 10:55, Lionel Landwerlin wrote: On 07/09/2018 10:39, Tvrtko Ursulin wrote: On 07/09/2018 10:23, Lionel Landwerlin wrote: On 07/09/2018 09:26, Tvrtko Ursulin wrote: On 06/09/2018 11:36, Lionel Landwerlin wrote: On 06/09/2018 11:22, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-09-06 11:18:01) On 06/09/2018 11:10, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-09-06 10:57:47) On 05/09/2018 15:22, Tvrtko Ursulin wrote: From: Lionel Landwerlin If some of the contexts submitting workloads to the GPU have been configured to shutdown slices/subslices, we might loose the NOA configurations written in the NOA muxes. One possible solution to this problem is to reprogram the NOA muxes when we switch to a new context. We initially tried this in the workaround batchbuffer but some concerns where raised about the cost of reprogramming at every context switch. This solution is also not without consequences from the userspace point of view. Reprogramming of the muxes can only happen once the powergating configuration has changed (which happens after context switch). This means for a window of time during the recording, counters recorded by the OA unit might be invalid. This requires userspace dealing with OA reports to discard the invalid values. Minimizing the reprogramming could be implemented by tracking of the last programmed configuration somewhere in GGTT and use MI_PREDICATE to discard some of the programming commands, but the command streamer would still have to parse all the MI_LRI instructions in the workaround batchbuffer. Another solution, which this change implements, is to simply disregard the user requested configuration for the period of time when i915/perf is active. There is no known issue with this apart from a performance penality for some media workloads that benefit from running on a partially powergated GPU. We already prevent RC6 from affecting the programming so it doesn't sound completely unreasonable to hold on powergating for the same reason. v2: Leave RPCS programming in intel_lrc.c (Lionel) v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) More to_intel_context() (Tvrtko) s/dev_priv/i915/ (Tvrtko) Tvrtko Ursulin: v4: * Rebase for make_rpcs changes. v5: * Apply OA restriction from make_rpcs directly. v6: * Rebase for context image setup changes. Signed-off-by: Lionel Landwerlin Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_perf.c | 5 + drivers/gpu/drm/i915/intel_lrc.c | 30 -- drivers/gpu/drm/i915/intel_lrc.h | 3 +++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index ccb20230df2c..dd65b72bddd4 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, CTX_REG(reg_state, state_offset, flex_regs[i], value); } + + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, + gen8_make_rpcs(dev_priv, + &to_intel_context(ctx, + dev_priv->engine[RCS])->sseu)); I think there is one issue I missed on the previous iterations of this patch. This gen8_update_reg_state_unlocked() is called when the GPU is parked on the kernel context. It's supposed to update all contexts, but I think we might not be able to update the kernel context image while the GPU is using it. The kernel context is only ever taken in extremis (you are either parking or stalling userspace) so I don't care. The patch exposing the RPCS configuration to userspace will make use of the kernel context while OA/perf is enabled. Even if it reprograms the locked value that will break the power configuration stability on Gen11 (because the locked configuration will be different from the kernel context configuration). Sure, but as you point out that's only on changing configuration. What's missing in the patch is that we only bail early if the new sseu matches the ce->sseu, but that doesn't necessarily match whats in the context due to OA. (Or maybe I missed the conversion to rpcs value and checking.) -Chris Yep, because the gen8_make_rpcs() post processes the values store at the gem context level, we risk rerunning the kernel context to write the exiting value. Sorry this is all so messy :( Lets see if I managed to follow here. The current code indeed bails out at the set ctx param level if the requested state matches the ce->state. My thinking was that ce->state is the master state and whatever happens in "post processing" via gen8_make_rpcs should be hidden from it since the design is that the i915_perf.c will re-configure all contexts when the OA active status changes (to either direc
Re: [Intel-gfx] [PATCH] drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB
Quoting Ville Syrjala (2018-09-11 17:54:57)
> From: Ville Syrjälä
>
> If we have framebuffers that are >= 4GiB in size we will overflow
> the fb size check in intel_fill_fb_info().
>
> Currently that is only possible with NV12 and CCS as offsets[1]
> may be anything between 0 and 0x. ofsets[0] is currently
> required to be 0 so we can't hit the overflow with any single
> plane format (thanks to max fb size of 8kx8k and max stride of
> 32 KiB).
>
> In the future we may allow almost any framebuffer to exceed 4GiB
> in size so we really should fix the overflow. Not that the overflow
> is particularly dangerous. It's mostly just a sanity check against
> insane userspace. The display engine can't write to memory anyway
> so I suppose in the worst case we might anger the hw by attempting
> scanout past the end of the ggtt, or we might scan out some data
> that we're not supposed to see from other parts of the ggtt.
>
> Note that triggering this overflow depends on the driver
> aligning the fb height to the next tile boundary to push the
> calculated size above 4GiB. With linear buffers the effective
> tile height is one so that never happens, and the core already
> has a check for 32bit overflow of offsets[]+pitches[]*height.
>
> Testcase: igt/kms_big_fb/x-tiled-addfb-size-offset-overflow
> Testcase: igt/kms_big_fb/y-tiled-addfb-size-offset-overflow
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2b77d9350a3a..2b474d049074 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2636,9 +2636,10 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> max_size = max(max_size, offset + size);
> }
>
> - if (max_size * tile_size > obj->base.size) {
> - DRM_DEBUG_KMS("fb too big for bo (need %u bytes, have %zu
> bytes)\n",
> - max_size * tile_size, obj->base.size);
> + if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
> + DRM_DEBUG_KMS("fb too big for bo (need %llu bytes, have %zu
> bytes)\n",
> + (unsigned long long) mul_u32_u32(max_size,
> tile_size),
mul_u32_u32 returns u64 i.e. unsigned long long; %llu is the one true
format specifier for u64 (Linus decree #103789)
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
If the ioctl is not supported on a particular piece of HW/driver
combination, report ENODEV so that it can be easily distinguished from
both the lack of the ioctl and from a regular invalid parameter.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Ville Syrjälä
---
drivers/gpu/drm/drm_framebuffer.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 6eaacd4eb8cc..eed6ad0fe84a 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -113,6 +113,9 @@ int drm_mode_addfb(struct drm_device *dev, struct
drm_mode_fb_cmd *or,
struct drm_mode_fb_cmd2 r = {};
int ret;
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -ENODEV;
+
r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
if (r.pixel_format == DRM_FORMAT_INVALID) {
DRM_DEBUG("bad {bpp:%d, depth:%d}\n", or->bpp, or->depth);
@@ -352,7 +355,7 @@ int drm_mode_addfb2(struct drm_device *dev,
struct drm_framebuffer *fb;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
- return -EINVAL;
+ return -ENODEV;
fb = drm_internal_framebuffer_create(dev, r, file_priv);
if (IS_ERR(fb))
@@ -387,7 +390,7 @@ int drm_mode_addfb2_ioctl(struct drm_device *dev,
* ADDFB.
*/
DRM_DEBUG_KMS("addfb2 broken on bigendian");
- return -EINVAL;
+ return -ENODEV;
}
#endif
return drm_mode_addfb2(dev, data, file_priv);
@@ -432,7 +435,7 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id,
int found = 0;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
- return -EINVAL;
+ return -ENODEV;
fb = drm_framebuffer_lookup(dev, file_priv, fb_id);
if (!fb)
@@ -509,7 +512,7 @@ int drm_mode_getfb(struct drm_device *dev,
int ret;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
- return -EINVAL;
+ return -ENODEV;
fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
if (!fb)
@@ -582,7 +585,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
int ret;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
- return -EINVAL;
+ return -ENODEV;
fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
if (!fb)
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson wrote:
> If the ioctl is not supported on a particular piece of HW/driver
> combination, report ENODEV so that it can be easily distinguished from
> both the lack of the ioctl and from a regular invalid parameter.
>
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: Ville Syrjälä
Hm, I thought the canonical errno for "ioctl doesn't apply to this
device" is ENOTTY? And ENODEV means that it applies, but atm nothing
plugged in, or device gone already.
I found a few more modeset ioctl:
- drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl
- drm_mode_getconnector
- drm_mode_getcrtc, drm_mode_setcrtc
- drm_mode_getencoder
- drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ...
Ok now I stop looking through the grep thing, looks like we've been
using EINVAL consistently. I'm all for switching, it makes sense, but
I think we should at least try to be consistent across all kms ioctl.
I'm happy to r-b such a series.
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_framebuffer.c | 13 -
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 6eaacd4eb8cc..eed6ad0fe84a 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -113,6 +113,9 @@ int drm_mode_addfb(struct drm_device *dev, struct
> drm_mode_fb_cmd *or,
> struct drm_mode_fb_cmd2 r = {};
> int ret;
>
> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> + return -ENODEV;
> +
> r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
> if (r.pixel_format == DRM_FORMAT_INVALID) {
> DRM_DEBUG("bad {bpp:%d, depth:%d}\n", or->bpp, or->depth);
> @@ -352,7 +355,7 @@ int drm_mode_addfb2(struct drm_device *dev,
> struct drm_framebuffer *fb;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> + return -ENODEV;
>
> fb = drm_internal_framebuffer_create(dev, r, file_priv);
> if (IS_ERR(fb))
> @@ -387,7 +390,7 @@ int drm_mode_addfb2_ioctl(struct drm_device *dev,
> * ADDFB.
> */
> DRM_DEBUG_KMS("addfb2 broken on bigendian");
> - return -EINVAL;
> + return -ENODEV;
> }
> #endif
> return drm_mode_addfb2(dev, data, file_priv);
> @@ -432,7 +435,7 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id,
> int found = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> + return -ENODEV;
>
> fb = drm_framebuffer_lookup(dev, file_priv, fb_id);
> if (!fb)
> @@ -509,7 +512,7 @@ int drm_mode_getfb(struct drm_device *dev,
> int ret;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> + return -ENODEV;
>
> fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
> if (!fb)
> @@ -582,7 +585,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> int ret;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - return -EINVAL;
> + return -ENODEV;
>
> fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
> if (!fb)
> --
> 2.19.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATH i-g-t] igt: Test tagging support
On 07/09/2018 12:43, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-07 12:14:20) From: Tvrtko Ursulin Proposal to add test tags as a replacement for separate test list which can be difficult to maintain and get out of date. Putting this maintanenace inline with tests makes it easier to remember to update the (now implicit) lists, and also enables richer test selection possibilities for the test runner. Current method of implying tags from test/subtest names has a couple of problems one of which is where some names can use the same token for different meanings. (One example is the "default" token.) It also creates a name clash between naming and tagging. Test tags are strings of tokens separated by spaces, meaning of which we decide by creating a convetion and helped by the suitable helper macros. For example tags can be things like: gem, kms, basic, guc, flip, semaphore, bz12345678, gt4e, reset, etc.. At runtime this would look something like this (abbreviated for readability): @ tests/gem_sync --list-subtests-with-tags ... forked-each TAGS="gem " forked-store-each TAGS="gem " basic-all TAGS="gem basic " basic-store-all TAGS="gem basic " @ tests/gem_concurrent_blit --list-subtests-with-tags ... 16MiB-swap-gpuX-render-write-read-bcs-bomb TAGS="gem stress " 16MiB-swap-gpuX-render-write-read-rcs-bomb TAGS="gem stress " 16MiB-swap-gpuX-render-gpu-read-after-write-bomb TAGS="gem stress " @ tests/kms_flip --list-subtests-with-tags | grep basic basic-plain-flip TAGS="kms basic " basic-flip-vs-dpms TAGS="kms basic " Test runner could then enable usages like: ./run-tests --include gem --exclude stress Minor comment, I like some basic boolean algebra here --include "gem AND smoketest NOT stress" :) That's what my hypothetical examples showed just with a different syntax! I'd probably tag everything according to which ioctls they use. I feel my endgoal is still to list tests by their kernel functions (which can and should be automatically derived), and their hw function which is the open problem. If we want to do that automatically then tagging in this flavour probably doesn't make sense and it should instead be an external database. If we go on the ioctl granularity it can probably mostly have one version, and it should be static enough to be able to live in the tree, but if we go more granular, like something derived from kcov, then that's out of the window. Since it will be tied both to a platform and kernel version. So I think tagging as proposed here is not appropriate for either, but only if we wanted to tag on coarser functional areas and such. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
Quoting Daniel Vetter (2018-09-12 09:39:30) > On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson > wrote: > > If the ioctl is not supported on a particular piece of HW/driver > > combination, report ENODEV so that it can be easily distinguished from > > both the lack of the ioctl and from a regular invalid parameter. > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: Ville Syrjälä > > Hm, I thought the canonical errno for "ioctl doesn't apply to this > device" is ENOTTY? That's ioctl doesn't exist. Sometimes it's nice to know the kernel knows about the ioctl but it isn't applicable. Either is fine for my purpose, which is to know the ioctl exists, I just need to distinguish it from EINVAL. > And ENODEV means that it applies, but atm nothing > plugged in, or device gone already. > > I found a few more modeset ioctl: > - drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl > - drm_mode_getconnector > - drm_mode_getcrtc, drm_mode_setcrtc > - drm_mode_getencoder > - drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ... > > Ok now I stop looking through the grep thing, looks like we've been > using EINVAL consistently. I'm all for switching, it makes sense, but > I think we should at least try to be consistent across all kms ioctl. Ah, but we've been consistent on the other side and have been using ENODEV for the better part of a decade for this meaning (that the ioctl doesn't apply to this HW) :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Differentiate the lack of an interface from invalid parameter
== Series Details ==
Series: drm: Differentiate the lack of an interface from invalid parameter
URL : https://patchwork.freedesktop.org/series/49536/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4808 -> Patchwork_10148 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10148 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10148, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/49536/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10148:
=== IGT changes ===
Warnings
igt@pm_rpm@module-reload:
fi-hsw-4770r: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_10148 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_module_reload@basic-reload-inject:
fi-hsw-4770r: PASS -> DMESG-WARN (fdo#107425)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
igt@kms_psr@primary_mmap_gtt:
{fi-cnl-u}: NOTRUN -> FAIL (fdo#107383) +3
igt@kms_psr@sprite_plane_onoff:
fi-bdw-samus: NOTRUN -> FAIL (fdo#107360, fdo#107383)
igt@pm_rpm@module-reload:
fi-bdw-samus: NOTRUN -> DMESG-WARN (fdo#107603)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#106685) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-byt-clapper: FAIL (fdo#107362) -> PASS
igt@kms_psr@primary_page_flip:
fi-whl-u: FAIL (fdo#107336) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107360 https://bugs.freedesktop.org/show_bug.cgi?id=107360
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425
fdo#107603 https://bugs.freedesktop.org/show_bug.cgi?id=107603
== Participating hosts (47 -> 44) ==
Additional (3): fi-cnl-u fi-icl-u fi-bdw-samus
Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-ctg-p8600 fi-gdg-551
== Build changes ==
* Linux: CI_DRM_4808 -> Patchwork_10148
CI_DRM_4808: 1a24d0c172a97f513014071eb0e5532a7d1ccbce @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10148: 11a809a46579732852de89dbd38e5dfd66a0bb58 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
11a809a46579 drm: Differentiate the lack of an interface from invalid parameter
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10148/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
On Wed, Sep 12, 2018 at 10:50 AM, Chris Wilson wrote: > Quoting Daniel Vetter (2018-09-12 09:39:30) >> On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson >> wrote: >> > If the ioctl is not supported on a particular piece of HW/driver >> > combination, report ENODEV so that it can be easily distinguished from >> > both the lack of the ioctl and from a regular invalid parameter. >> > >> > Signed-off-by: Chris Wilson >> > Cc: Daniel Vetter >> > Cc: Ville Syrjälä >> >> Hm, I thought the canonical errno for "ioctl doesn't apply to this >> device" is ENOTTY? > > That's ioctl doesn't exist. Sometimes it's nice to know the kernel knows > about the ioctl but it isn't applicable. Either is fine for my purpose, > which is to know the ioctl exists, I just need to distinguish it from > EINVAL. > >> And ENODEV means that it applies, but atm nothing >> plugged in, or device gone already. >> >> I found a few more modeset ioctl: >> - drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl >> - drm_mode_getconnector >> - drm_mode_getcrtc, drm_mode_setcrtc >> - drm_mode_getencoder >> - drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ... >> >> Ok now I stop looking through the grep thing, looks like we've been >> using EINVAL consistently. I'm all for switching, it makes sense, but >> I think we should at least try to be consistent across all kms ioctl. > > Ah, but we've been consistent on the other side and have been using > ENODEV for the better part of a decade for this meaning (that the ioctl > doesn't apply to this HW) :) Hm indeed, we've been using either ENODEV or EINVAL for "this ioctl doesn't exist/doesn't apply". ENODEV is clearly used in many places (also outside of drm) for "hw absent/gone/disappeared". And we have very few uses of ENOTTY. So I'm kinda leaning towards starting a new trend here, and using ENOTTY for "this ioctl doesn't apply". I don't think we need to differentiate this from the case of "this kernel has no idea about this ioctl at all", since from a userspace pov there's no difference really: Either way it can't use it. But I'm also fine if we're just sticking to ENODEV, just feels like wasting a perfectly useful errno (and there's not many) if we don't give ENOTTY some more use. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 02/13] drm/i915/icl: DSI vswing programming sequence
On 9/12/2018 12:20 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch setup voltage swing before enabling
combo PHY DDI (shared with DSI).
Note that DSI voltage swing programming is for
high speed data buffers. HW automatically handles
the voltage swing for the low power data buffers.
v2: Rebase
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/icl_dsi.c | 114 +
1 file changed, 114 insertions(+)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index a571339..dc16c1f 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -27,6 +27,65 @@
#include "intel_dsi.h"
+static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ enum port port;
+ u32 tmp;
+ int lane;
+
+ for_each_dsi_port(port, intel_dsi->ports) {
+
+ /* Bspec: set scaling mode to 0x6 */
Today bspec says 2. Also, please don't duplicate the value in the
comment.
Right..thanks for catching :)
+ tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+ tmp |= SCALING_MODE_SEL(6);
+ I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
Like Ville said, adding a blank line between each read-modify-write
group helps readability. Perhaps add /* DW5 */ etc. comments to group
the, eh, groups.
Ok.
+ tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
+ tmp |= SCALING_MODE_SEL(6);
+ I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
+ tmp |= TAP2_DISABLE | TAP3_DISABLE;
+ I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
+ tmp |= TAP2_DISABLE | TAP3_DISABLE;
+ I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
Are you missing RTERM_SELECT?
Looks this was not earlier and added recently. Will program in next version.
Why do you do two read-modify-writes (RMW) on both GRP and AUX, instead
of doing all the changes at once?
Do you mean for tmp |= TAP2_DISABLE | TAP3_DISABLE ?? If yes, because
GRP and AUX
might contain different values and need to read them explicitly.
The RMW doesn't actually clear the fields before changing them, just ORs
more stuff on top of them, and cursor program or coeff polarity might
contain garbage (at least in theory). The same below.
Yeah, we need to reset those bits using MASK and then do 'OR'.
Or are you suggesting something else??
+
+ /*
+* swing and scaling values are taken from DSI
+* table under vswing programming sequence for
+* combo phy ddi in BSPEC.
+* program swing values
+*/
Please reflow the comment.
Ok.
+ tmp = I915_READ(ICL_PORT_TX_DW2_LN0(port));
+ tmp |= SWING_SEL_UPPER(0x2);
+ tmp |= SWING_SEL_LOWER(0x2);
This would benefit from
+#define SWING_SEL_MASK (SWING_SEL_UPPER_MASK |
SWING_SEL_LOWER_MASK)
+#define SWING_SEL(x) (SWING_SEL_UPPER(x) |
SWING_SEL_LOWER(x))
in i915_reg.h. But I can look the other way and fix it myself later...
+ tmp |= RCOMP_SCALAR(0x98);
+ I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
+ tmp = I915_READ(ICL_PORT_TX_DW2_AUX(port));
+ tmp |= SWING_SEL_UPPER(0x2);
+ tmp |= SWING_SEL_LOWER(0x2);
+ tmp |= RCOMP_SCALAR(0x98);
+ I915_WRITE(ICL_PORT_TX_DW2_AUX(port), tmp);
+
+ /* program scaling values */
+ tmp = I915_READ(ICL_PORT_TX_DW4_AUX(port));
+ tmp |= POST_CURSOR_1(0x0);
+ tmp |= POST_CURSOR_2(0x0);
+ tmp |= CURSOR_COEFF(0x18);
0x3f?
Yes, now its changed to 0x3f.
Again, you need to zero the fields before ORin the new values into them.
Agree.
+ I915_WRITE(ICL_PORT_TX_DW4_AUX(port), tmp);
+
+ for (lane = 0; lane <= 3; lane++) {
+ /* Bspec: must not use GRP register for write */
I'll take your word for it, although I've missed such a requirement.
:-)
+ tmp = I915_READ(ICL_PORT_TX_DW4_LN(port, lane));
+ tmp |= POST_CURSOR_1(0x0);
+ tmp |= POST_CURSOR_2(0x0);
+ tmp |= CURSOR_COEFF(0x18);
0x3f?
Yes.
+ I915_WRITE(ICL_PORT_TX_DW4_LN(port, lane), tmp);
+ }
+ }
+}
+
static void gen11_dsi_program_esc_clk_div(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -140,6 +199,58 @@ static void gen11_dsi_config_phy_lanes_sequenc
Re: [Intel-gfx] [PATCH v5 03/13] drm/i915/icl: Enable DDI Buffer
On 9/12/2018 12:24 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch enables DDI buffer by writing to DDI_BUF_CTL
register and wait for DDI status to be *not idle* for a
port.
v2: Rebase
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/icl_dsi.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index dc16c1f..41faa19 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -251,6 +251,25 @@ static void gen11_dsi_voltage_swing_program_seq(struct
intel_encoder *encoder)
}
}
+static void gen11_dsi_enable_ddi_buffer(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ u32 tmp;
+ enum port port;
+
+ for_each_dsi_port(port, intel_dsi->ports) {
+ tmp = I915_READ(DDI_BUF_CTL(port));
+ tmp |= DDI_BUF_CTL_ENABLE;
+ I915_WRITE(DDI_BUF_CTL(port), tmp);
+
+ if (wait_for_us(!(I915_READ(DDI_BUF_CTL(port)) &
+ DDI_BUF_IS_IDLE),
+ 500))
IMO a "== 0" check reads better in wait_for_us.
+ DRM_ERROR("DDI port:%c buffer idle\n", port_name(port));
+ }
+}
+
static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
{
/* step 4a: power up all lanes of the DDI used by DSI */
@@ -261,6 +280,9 @@ static void gen11_dsi_enable_port_and_phy(struct
intel_encoder *encoder)
/* step 4c: configure voltage swing and skew */
gen11_dsi_voltage_swing_program_seq(encoder);
+
+ /* step 4d: enable DDI buffer */
Alas, this is step 4e now, and you have a new 4d to take care of for
B0+.
Yes. Will remove step hardcoding comments from patch.
Regards,
Madhav
Regardless,
Reviewed-by: Jani Nikula
+ gen11_dsi_enable_ddi_buffer(encoder);
}
static void __attribute__((unused))
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATH i-g-t] igt: Test tagging support
Quoting Tvrtko Ursulin (2018-09-12 09:48:00) > > On 07/09/2018 12:43, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-07 12:14:20) > >> From: Tvrtko Ursulin > >> > >> Proposal to add test tags as a replacement for separate test > >> list which can be difficult to maintain and get out of date. > >> > >> Putting this maintanenace inline with tests makes it easier > >> to remember to update the (now implicit) lists, and also enables > >> richer test selection possibilities for the test runner. > >> > >> Current method of implying tags from test/subtest names has a > >> couple of problems one of which is where some names can use > >> the same token for different meanings. (One example is the > >> "default" token.) It also creates a name clash between naming > >> and tagging. > >> > >> Test tags are strings of tokens separated by spaces, meaning of > >> which we decide by creating a convetion and helped by the > >> suitable helper macros. > >> > >> For example tags can be things like: gem, kms, basic, guc, flip, > >> semaphore, bz12345678, gt4e, reset, etc.. > >> > >> At runtime this would look something like this (abbreviated for > >> readability): > >> > >>@ tests/gem_sync --list-subtests-with-tags > >>... > >>forked-each TAGS="gem " > >>forked-store-each TAGS="gem " > >>basic-all TAGS="gem basic " > >>basic-store-all TAGS="gem basic " > >> > >>@ tests/gem_concurrent_blit --list-subtests-with-tags > >>... > >>16MiB-swap-gpuX-render-write-read-bcs-bomb TAGS="gem stress " > >>16MiB-swap-gpuX-render-write-read-rcs-bomb TAGS="gem stress " > >>16MiB-swap-gpuX-render-gpu-read-after-write-bomb TAGS="gem stress " > >> > >>@ tests/kms_flip --list-subtests-with-tags | grep basic > >>basic-plain-flip TAGS="kms basic " > >>basic-flip-vs-dpms TAGS="kms basic " > >> > >> Test runner could then enable usages like: > >> > >>./run-tests --include gem --exclude stress > > > > Minor comment, I like some basic boolean algebra here > > --include "gem AND smoketest NOT stress" > > :) > > That's what my hypothetical examples showed just with a different syntax! > > > I'd probably tag everything according to which ioctls they use. I feel my > > endgoal is still to list tests by their kernel functions (which can and > > should be automatically derived), and their hw function which is the > > open problem. > > If we want to do that automatically then tagging in this flavour > probably doesn't make sense and it should instead be an external database. > > If we go on the ioctl granularity it can probably mostly have one > version, and it should be static enough to be able to live in the tree, > but if we go more granular, like something derived from kcov, then > that's out of the window. Since it will be tied both to a platform and > kernel version. > > So I think tagging as proposed here is not appropriate for either, but > only if we wanted to tag on coarser functional areas and such. Yeah, I think the same as well, that tags should be "smoke", "stress". (But one man's stress is another's minimal workload :( But both of those are in essence a duration parameter, and I'd prefer if we expressed those as configurable parameters. At which point there is a meta level of test scripts to tag ;) I feel that "gem", "kms" is better expressed in lower level terms, so what's left to tag? Clients? "display", "opencl", "media", "opengl" ? Even using client specs for features (e.g. EGL_IMG_context_priority)? Who and why would use those? From a selfish perspective, I want lcov with tests sorted in order of increasing "stress" (start with the precise X does Y test, finish with can it survive pathological client behaviour for 48 hours). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 02/13] drm/i915/icl: DSI vswing programming sequence
On Wed, 12 Sep 2018, Madhav Chauhan wrote:
> On 9/12/2018 12:20 AM, Jani Nikula wrote:
>> On Tue, 10 Jul 2018, Madhav Chauhan wrote:
>>> This patch setup voltage swing before enabling
>>> combo PHY DDI (shared with DSI).
>>> Note that DSI voltage swing programming is for
>>> high speed data buffers. HW automatically handles
>>> the voltage swing for the low power data buffers.
>>>
>>> v2: Rebase
>>>
>>> Signed-off-by: Madhav Chauhan
>>> ---
>>> drivers/gpu/drm/i915/icl_dsi.c | 114
>>> +
>>> 1 file changed, 114 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
>>> index a571339..dc16c1f 100644
>>> --- a/drivers/gpu/drm/i915/icl_dsi.c
>>> +++ b/drivers/gpu/drm/i915/icl_dsi.c
>>> @@ -27,6 +27,65 @@
>>>
>>> #include "intel_dsi.h"
>>>
>>> +static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>>> +{
>>> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> + enum port port;
>>> + u32 tmp;
>>> + int lane;
>>> +
>>> + for_each_dsi_port(port, intel_dsi->ports) {
>>> +
>>> + /* Bspec: set scaling mode to 0x6 */
>> Today bspec says 2. Also, please don't duplicate the value in the
>> comment.
>
> Right..thanks for catching :)
>
>>
>>> + tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
>>> + tmp |= SCALING_MODE_SEL(6);
>>> + I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
>> Like Ville said, adding a blank line between each read-modify-write
>> group helps readability. Perhaps add /* DW5 */ etc. comments to group
>> the, eh, groups.
>
> Ok.
>
>>
>>> + tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
>>> + tmp |= SCALING_MODE_SEL(6);
>>> + I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
>>> + tmp = I915_READ(ICL_PORT_TX_DW5_LN0(port));
>>> + tmp |= TAP2_DISABLE | TAP3_DISABLE;
>>> + I915_WRITE(ICL_PORT_TX_DW5_GRP(port), tmp);
>>> + tmp = I915_READ(ICL_PORT_TX_DW5_AUX(port));
>>> + tmp |= TAP2_DISABLE | TAP3_DISABLE;
>>> + I915_WRITE(ICL_PORT_TX_DW5_AUX(port), tmp);
>> Are you missing RTERM_SELECT?
>
> Looks this was not earlier and added recently. Will program in next version.
>
>>
>> Why do you do two read-modify-writes (RMW) on both GRP and AUX, instead
>> of doing all the changes at once?
>
> Do you mean for tmp |= TAP2_DISABLE | TAP3_DISABLE ?? If yes, because
> GRP and AUX
> might contain different values and need to read them explicitly.
No, I mean first you RMW scaling mode for GRP and AUX, then you do
TAP2/3 disable for GRP and AUX. Why not scaling mode *and* TAP2/3
disable in one go, for GRP and AUX separately of course.
>
>>
>> The RMW doesn't actually clear the fields before changing them, just ORs
>> more stuff on top of them, and cursor program or coeff polarity might
>> contain garbage (at least in theory). The same below.
>
> Yeah, we need to reset those bits using MASK and then do 'OR'.
Yes.
> Or are you suggesting something else??
No, that's just it.
BR,
Jani.
>
>>
>>> +
>>> + /*
>>> +* swing and scaling values are taken from DSI
>>> +* table under vswing programming sequence for
>>> +* combo phy ddi in BSPEC.
>>> +* program swing values
>>> +*/
>> Please reflow the comment.
>
> Ok.
>
>>
>>> + tmp = I915_READ(ICL_PORT_TX_DW2_LN0(port));
>>> + tmp |= SWING_SEL_UPPER(0x2);
>>> + tmp |= SWING_SEL_LOWER(0x2);
>> This would benefit from
>>
>> +#define SWING_SEL_MASK(SWING_SEL_UPPER_MASK |
>> SWING_SEL_LOWER_MASK)
>> +#define SWING_SEL(x) (SWING_SEL_UPPER(x) |
>> SWING_SEL_LOWER(x))
>>
>> in i915_reg.h. But I can look the other way and fix it myself later...
>>
>>> + tmp |= RCOMP_SCALAR(0x98);
>>> + I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
>>> + tmp = I915_READ(ICL_PORT_TX_DW2_AUX(port));
>>> + tmp |= SWING_SEL_UPPER(0x2);
>>> + tmp |= SWING_SEL_LOWER(0x2);
>>> + tmp |= RCOMP_SCALAR(0x98);
>>> + I915_WRITE(ICL_PORT_TX_DW2_AUX(port), tmp);
>>> +
>>> + /* program scaling values */
>>> + tmp = I915_READ(ICL_PORT_TX_DW4_AUX(port));
>>> + tmp |= POST_CURSOR_1(0x0);
>>> + tmp |= POST_CURSOR_2(0x0);
>>> + tmp |= CURSOR_COEFF(0x18);
>> 0x3f?
>
> Yes, now its changed to 0x3f.
>
>>
>> Again, you need to zero the fields before ORin the new values into them.
>
> Agree.
>
>>
>>> + I915_WRITE(ICL_PORT_TX_DW4_AUX(port), tmp);
>>> +
>>> + for (lane = 0; lane <= 3; lane++) {
>>> + /* Bspec: must not use GRP register for write */
>> I'll take your word for it, although I've missed such a requirement.
>
> :-)
>
>>
>>> + tmp = I915_READ(ICL_PORT_TX_DW4_LN(port, lane));
>>> +
Re: [Intel-gfx] [PATCH v5 06/13] drm/i915/icl: Define data/clock lanes dphy timing registers
On 9/12/2018 12:44 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch defines DSI_CLK_TIMING_PARAM, DPHY_CLK_TIMING_PARAM,
DSI_DATA_TIMING_PARAM, DPHY_DATA_TIMING_PARAM register used in
dphy programming.
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6129372..0dbdd57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10075,6 +10075,46 @@ enum skl_power_gate {
_ICL_DSI_T_INIT_MASTER_0,\
_ICL_DSI_T_INIT_MASTER_1)
+#define _DPHY_CLK_TIMING_PARAM_0 0x162180
+#define _DPHY_CLK_TIMING_PARAM_1 0x6c180
+#define DPHY_CLK_TIMING_PARAM(port)_MMIO_PORT(port,\
+ _DPHY_CLK_TIMING_PARAM_0,\
+ _DPHY_CLK_TIMING_PARAM_1)
+#define _DSI_CLK_TIMING_PARAM_00x6b080
+#define _DSI_CLK_TIMING_PARAM_10x6b880
+#define DSI_CLK_TIMING_PARAM(port) _MMIO_PORT(port,\
+ _DSI_CLK_TIMING_PARAM_0,\
+ _DSI_CLK_TIMING_PARAM_1)
+#define CLK_PREP_OVERRIDE (1 << 31)
+#define CLK_PREP_TIME(x) (x << 28)
+#define CLK_ZERO_OVERRIDE (1 << 27)
+#define CLK_ZERO_TIME(x) (x << 20)
+#define CLK_PRE_OVERRIDE (1 << 19)
+#define CLK_PRE_TIME(x) (x << 16)
+#define CLK_POST_OVERRIDE (1 << 15)
+#define CLK_POST_TIME(x) (x << 8)
+#define CLK_TRAIL_OVERRIDE(1 << 7)
+#define CLK_TRAIL_TIME(x) (x << 0)
I would prefer we stuck to the convention of defining _SHIFT and _MASK
macros for the bitfields. Even if the above style has started to creep
in without proper discussion. (I approve of the function-like macros for
things that aren't straight shifts; stuff with split bitfields or
calculations.)
No matter what, you need to wrap the macro arguments in parens!
Also, please don't do your own abbreviations or renames of the field
names when the bspec name is short/good enough.
Ok. I will add the mask and wrap the arguments in parens.
Also will recheck the names as per BSPEC.
Regards,
Madhav
+
+#define _DPHY_DATA_TIMING_PARAM_0 0x162184
+#define _DPHY_DATA_TIMING_PARAM_1 0x6c184
+#define DPHY_DATA_TIMING_PARAM(port) _MMIO_PORT(port,\
+ _DPHY_DATA_TIMING_PARAM_0,\
+ _DPHY_DATA_TIMING_PARAM_1)
+#define _DSI_DATA_TIMING_PARAM_0 0x6B084
+#define _DSI_DATA_TIMING_PARAM_1 0x6B884
+#define DSI_DATA_TIMING_PARAM(port)_MMIO_PORT(port,\
+ _DSI_DATA_TIMING_PARAM_0,\
+ _DSI_DATA_TIMING_PARAM_1)
+#define HS_PREP_OVERRIDE (1 << 31)
+#define HS_PREP_TIME(x) (x << 24)
+#define HS_ZERO_OVERRIDE (1 << 23)
+#define HS_ZERO_TIME(x) (x << 16)
+#define HS_TRAIL_OVERRIDE (1 << 15)
+#define HS_TRAIL_TIME(x) (x << 8)
+#define HS_EXIT_OVERRIDE (1 << 7)
+#define HS_EXIT_TIME(x) (x << 0)
Same as above.
The register offsets and shifts etc. look ok.
BR,
Jani.
+
/* bits 31:0 */
#define _MIPIA_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb084)
#define _MIPIC_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb884)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 03/13] drm/i915/icl: Enable DDI Buffer
On Wed, 12 Sep 2018, Madhav Chauhan wrote:
> On 9/12/2018 12:24 AM, Jani Nikula wrote:
>> On Tue, 10 Jul 2018, Madhav Chauhan wrote:
>>> This patch enables DDI buffer by writing to DDI_BUF_CTL
>>> register and wait for DDI status to be *not idle* for a
>>> port.
>>>
>>> v2: Rebase
>>>
>>> Signed-off-by: Madhav Chauhan
>>> ---
>>> drivers/gpu/drm/i915/icl_dsi.c | 22 ++
>>> 1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
>>> index dc16c1f..41faa19 100644
>>> --- a/drivers/gpu/drm/i915/icl_dsi.c
>>> +++ b/drivers/gpu/drm/i915/icl_dsi.c
>>> @@ -251,6 +251,25 @@ static void gen11_dsi_voltage_swing_program_seq(struct
>>> intel_encoder *encoder)
>>> }
>>> }
>>>
>>> +static void gen11_dsi_enable_ddi_buffer(struct intel_encoder *encoder)
>>> +{
>>> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> + u32 tmp;
>>> + enum port port;
>>> +
>>> + for_each_dsi_port(port, intel_dsi->ports) {
>>> + tmp = I915_READ(DDI_BUF_CTL(port));
>>> + tmp |= DDI_BUF_CTL_ENABLE;
>>> + I915_WRITE(DDI_BUF_CTL(port), tmp);
>>> +
>>> + if (wait_for_us(!(I915_READ(DDI_BUF_CTL(port)) &
>>> + DDI_BUF_IS_IDLE),
>>> + 500))
>> IMO a "== 0" check reads better in wait_for_us.
>>
>>> + DRM_ERROR("DDI port:%c buffer idle\n", port_name(port));
>>> + }
>>> +}
>>> +
>>> static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
>>> {
>>> /* step 4a: power up all lanes of the DDI used by DSI */
>>> @@ -261,6 +280,9 @@ static void gen11_dsi_enable_port_and_phy(struct
>>> intel_encoder *encoder)
>>>
>>> /* step 4c: configure voltage swing and skew */
>>> gen11_dsi_voltage_swing_program_seq(encoder);
>>> +
>>> + /* step 4d: enable DDI buffer */
>> Alas, this is step 4e now, and you have a new 4d to take care of for
>> B0+.
>
> Yes. Will remove step hardcoding comments from patch.
It's unfortunate; the comments do help in review *now* but alas they
might confuse the reader tomorrow. :(
BR,
Jani.
>
> Regards,
> Madhav
>
>>
>> Regardless,
>>
>> Reviewed-by: Jani Nikula
>>
>>
>>> + gen11_dsi_enable_ddi_buffer(encoder);
>>> }
>>>
>>> static void __attribute__((unused))
>
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
Quoting Daniel Vetter (2018-09-12 10:02:47) > On Wed, Sep 12, 2018 at 10:50 AM, Chris Wilson > wrote: > > Quoting Daniel Vetter (2018-09-12 09:39:30) > >> On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson > >> wrote: > >> > If the ioctl is not supported on a particular piece of HW/driver > >> > combination, report ENODEV so that it can be easily distinguished from > >> > both the lack of the ioctl and from a regular invalid parameter. > >> > > >> > Signed-off-by: Chris Wilson > >> > Cc: Daniel Vetter > >> > Cc: Ville Syrjälä > >> > >> Hm, I thought the canonical errno for "ioctl doesn't apply to this > >> device" is ENOTTY? > > > > That's ioctl doesn't exist. Sometimes it's nice to know the kernel knows > > about the ioctl but it isn't applicable. Either is fine for my purpose, > > which is to know the ioctl exists, I just need to distinguish it from > > EINVAL. > > > >> And ENODEV means that it applies, but atm nothing > >> plugged in, or device gone already. > >> > >> I found a few more modeset ioctl: > >> - drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl > >> - drm_mode_getconnector > >> - drm_mode_getcrtc, drm_mode_setcrtc > >> - drm_mode_getencoder > >> - drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ... > >> > >> Ok now I stop looking through the grep thing, looks like we've been > >> using EINVAL consistently. I'm all for switching, it makes sense, but > >> I think we should at least try to be consistent across all kms ioctl. > > > > Ah, but we've been consistent on the other side and have been using > > ENODEV for the better part of a decade for this meaning (that the ioctl > > doesn't apply to this HW) :) > > Hm indeed, we've been using either ENODEV or EINVAL for "this ioctl > doesn't exist/doesn't apply". ENODEV is clearly used in many places > (also outside of drm) for "hw absent/gone/disappeared". And we have > very few uses of ENOTTY. So I'm kinda leaning towards starting a new > trend here, and using ENOTTY for "this ioctl doesn't apply". I don't > think we need to differentiate this from the case of "this kernel has > no idea about this ioctl at all", since from a userspace pov there's > no difference really: Either way it can't use it. > > But I'm also fine if we're just sticking to ENODEV, just feels like > wasting a perfectly useful errno (and there's not many) if we don't > give ENOTTY some more use. In going through the drm_core_checks, I noticed one brave soul used -ENOTSUPP. How about that? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 08/13] drm/i915/icl: Define TA_TIMING_PARAM registers
On 9/12/2018 12:53 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch defines DSI_TA_TIMING_PARAM and
DPHY_TA_TIMING_PARAM registers used in
dphy programming.
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0dbdd57..1d13ba9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10115,6 +10115,20 @@ enum skl_power_gate {
#define HS_EXIT_OVERRIDE (1 << 7)
#define HS_EXIT_TIME(x) (x << 0)
+#define _DPHY_TA_TIMING_PARAM_0 0x162188
+#define _DPHY_TA_TIMING_PARAM_10x6c188
+#define DPHY_TA_TIMING_PARAM(port) _MMIO_PORT(port,\
+ _DPHY_TA_TIMING_PARAM_0,\
+ _DPHY_TA_TIMING_PARAM_1)
+#define _DSI_TA_TIMING_PARAM_0 0x6b098
+#define _DSI_TA_TIMING_PARAM_1 0x6b898
+#define DSI_TA_TIMING_PARAM(port) _MMIO_PORT(port,\
+ _DSI_TA_TIMING_PARAM_0,\
+ _DSI_TA_TIMING_PARAM_1)
+#define TA_SURE_OVERRIDE (1 << 31)
+#define TA_SURE_TIME(x) (x << 16)
+#define TA_SURE_TIME_MASK (0x1f << 16)
Please stick to _SHIFT. And in any case macro arguments need parens
around them.
Please add all the fields for the registers in one go.
Ok. Thanks!!
Regards,
Madhav
BR,
Jani.
+
/* bits 31:0 */
#define _MIPIA_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb084)
#define _MIPIC_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb884)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 09/13] drm/i915/icl: Program TA_TIMING_PARAM registers
On 9/12/2018 12:56 AM, Jani Nikula wrote:
On Fri, 20 Jul 2018, "Chauhan, Madhav" wrote:
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Thursday, July 19, 2018 9:51 PM
To: Chauhan, Madhav
Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani ;
Zanoni, Paulo R ; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH v5 09/13] drm/i915/icl: Program
TA_TIMING_PARAM registers
On Tue, Jul 10, 2018 at 03:10:10PM +0530, Madhav Chauhan wrote:
This patch programs D-PHY timing parameters for the bus turn around
flow(in escape clocks) only if dsi link frequency <=800 MHz using
DPHY_TA_TIMING_PARAM and its identical register
DSI_TA_TIMING_PARAM
(inside DSI Controller within the Display Core).
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/icl_dsi.c | 21 +
drivers/gpu/drm/i915/intel_dsi.h | 1 +
drivers/gpu/drm/i915/intel_dsi_vbt.c | 1 +
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/icl_dsi.c
b/drivers/gpu/drm/i915/icl_dsi.c index 832772d..8fd5284 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -302,6 +302,27 @@ static void gen11_dsi_setup_dphy_timings(struct
intel_encoder *encoder)
I915_WRITE(DSI_DATA_TIMING_PARAM(port),
intel_dsi->dphy_data_lane_reg);
}
+
+ /*
+* If DSI link operating at or below an 800 MHz,
+* TA_SURE should be override and programmed to
+* a value '0' inside TA_PARAM_REGISTERS otherwise
+* leave all fields at HW default values.
+*/
+ if (intel_dsi->bitrate_khz <= KHz(800)) {
The KHz(800) confuses me. My brain thinks this value is 800 kHz when it's
not. So I'd write it without the KHz() macro.
Ok. Initially I wrote without using KHz macro, but got comment to use KHz macro
:)
Did I? Oh well. Go with 80.
Ok :)
Please don't add additional state with intel_dsi->bitrate_khz when you
can calculate the bitrate at any time. Add a function to do it if you
like, and use it in both places.
Ok. But we will reusing this bitrate(same value) in multiple place.
Shouldn't
we cache it rather than calculating everytime??
Regards,
Madhav
BR,
Jani.
Regards,
Madhav
+ for_each_dsi_port(port, intel_dsi->ports) {
+ tmp = I915_READ(DPHY_TA_TIMING_PARAM(port));
+ tmp &= ~TA_SURE_TIME_MASK;
+ tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0));
+ I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp);
+
+ /* shadow register inside display core */
+ tmp = I915_READ(DSI_TA_TIMING_PARAM(port));
+ tmp &= ~TA_SURE_TIME_MASK;
+ tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0));
+ I915_WRITE(DSI_TA_TIMING_PARAM(port), tmp);
+ }
+ }
}
static void gen11_dsi_enable_port_and_phy(struct intel_encoder
*encoder) diff --git a/drivers/gpu/drm/i915/intel_dsi.h
b/drivers/gpu/drm/i915/intel_dsi.h
index 9fd8526..25e7396 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -101,6 +101,7 @@ struct intel_dsi {
u16 init_count;
u32 pclk;
+ u32 bitrate_khz;
u16 burst_mode_ratio;
/* all delays in ms */
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 428290d..a9a98a4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -589,6 +589,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi,
u16 panel_id)
intel_dsi->pclk = pclk;
bitrate = (pclk * bpp) / intel_dsi->lane_count;
+ intel_dsi->bitrate_khz = bitrate;
switch (intel_dsi->escape_clk_div) {
case 0:
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm: Differentiate the lack of an interface from invalid parameter
If the ioctl is not supported on a particular piece of HW/driver
combination, report ENOTSUPP so that it can be easily distinguished from
both the lack of the ioctl and from a regular invalid parameter.
v2: Across all the kms ioctls we had a mixture of reporting EINVAL,
ENODEV and a few ENOTSUPP (most where EINVAL) for a failed
drm_core_check_feature(). Update everybody to report ENOTSUPP.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Ville Syrjälä
---
drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
drivers/gpu/drm/drm_bufs.c| 32 +++
drivers/gpu/drm/drm_color_mgmt.c | 4 ++--
drivers/gpu/drm/drm_connector.c | 2 +-
drivers/gpu/drm/drm_context.c | 16
drivers/gpu/drm/drm_crtc.c| 4 ++--
drivers/gpu/drm/drm_encoder.c | 2 +-
drivers/gpu/drm/drm_framebuffer.c | 13 -
drivers/gpu/drm/drm_gem.c | 6 +++---
drivers/gpu/drm/drm_ioctl.c | 2 +-
drivers/gpu/drm/drm_irq.c | 4 ++--
drivers/gpu/drm/drm_lease.c | 8
drivers/gpu/drm/drm_lock.c| 4 ++--
drivers/gpu/drm/drm_mode_config.c | 2 +-
drivers/gpu/drm/drm_mode_object.c | 4 ++--
drivers/gpu/drm/drm_pci.c | 4 ++--
drivers/gpu/drm/drm_plane.c | 10 +-
drivers/gpu/drm/drm_prime.c | 4 ++--
drivers/gpu/drm/drm_property.c| 8
drivers/gpu/drm/drm_scatter.c | 8
drivers/gpu/drm/drm_syncobj.c | 14 +++---
drivers/gpu/drm/drm_vblank.c | 4 ++--
22 files changed, 80 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 26690a664ec6..dc4502464126 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1251,7 +1251,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
/* disallow for drivers not supporting atomic: */
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
- return -EINVAL;
+ return -ENOTSUPP;
/* disallow for userspace that has not enabled atomic cap (even
* though this may be a bit overkill, since legacy userspace
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index ba8cfe65c65b..a07e7a781d64 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -398,7 +398,7 @@ int drm_legacy_addmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
err = drm_addmap_core(dev, map->offset, map->size, map->type,
map->flags, &maplist);
@@ -444,7 +444,7 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
idx = map->offset;
if (idx < 0)
@@ -596,7 +596,7 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
mutex_lock(&dev->struct_mutex);
list_for_each_entry(r_list, &dev->maplist, head) {
@@ -860,7 +860,7 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
struct drm_buf **temp_buflist;
if (!drm_core_check_feature(dev, DRIVER_PCI_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1064,7 +1064,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev,
struct drm_buf **temp_buflist;
if (!drm_core_check_feature(dev, DRIVER_SG))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1221,10 +1221,10 @@ int drm_legacy_addbufs(struct drm_device *dev, void
*data,
int ret;
if (!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
#if IS_ENABLED(CONFIG_AGP)
if (request->flags & _DRM_AGP_BUFFER)
@@ -1267,10 +1267,10 @@ int __drm_legacy_infobufs(struct drm_device *dev,
int count;
if (!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1352,10 +1352,10 @@ int drm_legacy_markbufs(struct drm_device *dev, void
*data,
struct drm_buf_entry *entry;
[Intel-gfx] [PATCH v3] drm: Differentiate the lack of an interface from invalid parameter
If the ioctl is not supported on a particular piece of HW/driver
combination, report ENOTSUPP so that it can be easily distinguished from
both the lack of the ioctl and from a regular invalid parameter.
v2: Across all the kms ioctls we had a mixture of reporting EINVAL,
ENODEV and a few ENOTSUPP (most where EINVAL) for a failed
drm_core_check_feature(). Update everybody to report ENOTSUPP.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Ville Syrjälä
---
drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
drivers/gpu/drm/drm_bufs.c| 32 +++
drivers/gpu/drm/drm_color_mgmt.c | 4 ++--
drivers/gpu/drm/drm_connector.c | 2 +-
drivers/gpu/drm/drm_context.c | 16
drivers/gpu/drm/drm_crtc.c| 4 ++--
drivers/gpu/drm/drm_encoder.c | 2 +-
drivers/gpu/drm/drm_framebuffer.c | 13 -
drivers/gpu/drm/drm_gem.c | 6 +++---
drivers/gpu/drm/drm_ioctl.c | 2 +-
drivers/gpu/drm/drm_irq.c | 4 ++--
drivers/gpu/drm/drm_lease.c | 8
drivers/gpu/drm/drm_lock.c| 4 ++--
drivers/gpu/drm/drm_mode_config.c | 2 +-
drivers/gpu/drm/drm_mode_object.c | 4 ++--
drivers/gpu/drm/drm_pci.c | 4 ++--
drivers/gpu/drm/drm_plane.c | 10 +-
drivers/gpu/drm/drm_prime.c | 4 ++--
drivers/gpu/drm/drm_property.c| 8
drivers/gpu/drm/drm_scatter.c | 8
drivers/gpu/drm/drm_syncobj.c | 14 +++---
drivers/gpu/drm/drm_vblank.c | 4 ++--
22 files changed, 80 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 26690a664ec6..dc4502464126 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1251,7 +1251,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
/* disallow for drivers not supporting atomic: */
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
- return -EINVAL;
+ return -ENOTSUPP;
/* disallow for userspace that has not enabled atomic cap (even
* though this may be a bit overkill, since legacy userspace
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index ba8cfe65c65b..a07e7a781d64 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -398,7 +398,7 @@ int drm_legacy_addmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
err = drm_addmap_core(dev, map->offset, map->size, map->type,
map->flags, &maplist);
@@ -444,7 +444,7 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
idx = map->offset;
if (idx < 0)
@@ -596,7 +596,7 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void
*data,
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
mutex_lock(&dev->struct_mutex);
list_for_each_entry(r_list, &dev->maplist, head) {
@@ -860,7 +860,7 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
struct drm_buf **temp_buflist;
if (!drm_core_check_feature(dev, DRIVER_PCI_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1064,7 +1064,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev,
struct drm_buf **temp_buflist;
if (!drm_core_check_feature(dev, DRIVER_SG))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1221,10 +1221,10 @@ int drm_legacy_addbufs(struct drm_device *dev, void
*data,
int ret;
if (!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
#if IS_ENABLED(CONFIG_AGP)
if (request->flags & _DRM_AGP_BUFFER)
@@ -1267,10 +1267,10 @@ int __drm_legacy_infobufs(struct drm_device *dev,
int count;
if (!drm_core_check_feature(dev, DRIVER_LEGACY))
- return -EINVAL;
+ return -ENOTSUPP;
if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA))
- return -EINVAL;
+ return -ENOTSUPP;
if (!dma)
return -EINVAL;
@@ -1352,10 +1352,10 @@ int drm_legacy_markbufs(struct drm_device *dev, void
*data,
struct drm_buf_entry *entry;
[Intel-gfx] [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
From: Tvrtko Ursulin
Tests which call intel_require_memory currently always skip on simulation,
unless they fail first due insufficient memory. This can create different
outcomes depending on the simulation environment so move the simulation
skip to the start of the function for 100% consistency in results.
Signed-off-by: Tvrtko Ursulin
Cc: Radoslaw Szwichtenberg
---
lib/intel_os.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/intel_os.c b/lib/intel_os.c
index 29a27272e782..e1e31e23069b 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -375,6 +375,8 @@ void intel_require_memory(uint64_t count, uint64_t size,
unsigned mode)
uint64_t required, total;
bool sufficient_memory;
+ igt_skip_on_simulation();
+
sufficient_memory = __intel_check_memory(count, size, mode,
&required, &total);
if (!sufficient_memory) {
@@ -406,8 +408,6 @@ void intel_require_memory(uint64_t count, uint64_t size,
unsigned mode)
mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
mode & CHECK_SWAP ? " + swap": "",
(long long)vfs_file_max());
-
- igt_skip_on_simulation();
}
void intel_purge_vm_caches(int drm_fd)
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATH i-g-t 2/2] core: Show backtrace from igt_skip_on_simulation
From: Tvrtko Ursulin
igt_skip_on_simulation is called both directly from tests but also from
library helpers. In the latter case especially the logged caller name is
useless since it is always the helper itself. What we instead want to know
is who is the caller.
Trivial approach would be to move the helper to a header as static inline,
but due the longjmp in it it can never be inlined. Alternative option is
to print a backtrace from it.
Signed-off-by: Tvrtko Ursulin
Cc: Radoslaw Szwichtenberg
---
lib/igt_core.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 23bb858fd886..990abc5a36b3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2065,14 +2065,26 @@ bool igt_run_in_simulation(void)
*/
void igt_skip_on_simulation(void)
{
+ bool in_simulation;
+
if (igt_only_list_subtests())
return;
+ in_simulation = igt_run_in_simulation();
+
if (!igt_can_fail()) {
- igt_fixture
- igt_require(!igt_run_in_simulation());
- } else
- igt_require(!igt_run_in_simulation());
+ igt_fixture {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
+ } else {
+ if (in_simulation) {
+ print_backtrace();
+ igt_require(!in_simulation);
+ }
+ }
}
/* structured logging */
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
On 9/12/2018 1:00 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch defines transcoder function configuration
registers and its bitfields for both DSI ports.
Used while programming/enabling DSI transcoder.
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 47 +
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 62bc76e..71ce6ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10134,6 +10134,53 @@ enum skl_power_gate {
#define TA_SURE_TIME(x) (x << 16)
#define TA_SURE_TIME_MASK(0x1f << 16)
+/* DSI transcoder configuration */
+#define _DSI_TRANS_FUNC_CONF_0 0x6b030
+#define _DSI_TRANS_FUNC_CONF_1 0x6b830
+#define DSI_TRANS_FUNC_CONF(tc)_MMIO_DSI(tc, \
+ _DSI_TRANS_FUNC_CONF_0,\
+ _DSI_TRANS_FUNC_CONF_1)
+#define OP_MODE(x)(x << 28)
+#define OP_MODE_MASK (0x3 << 28)
+#define CMD_MODE_NO_GATE 0x0
+#define CMD_MODE_TE_GATE 0x1
+#define VIDEO_MODE_SYNC_EVENT 0x2
+#define VIDEO_MODE_SYNC_PULSE 0x3
The convention is to define macros for field values that you can OR
directly in place instead of requiring a shift. Please stick to the
conventions. Use _SHIFT and _MASK.
We can debate the relative merits of both approaches at some point, but
this is not the time.
Just to understand this point correctly,
#define OP_MODE(x) ((x) << 28) is OK
but
#define OP_MODE_MASK (0x3 << 28) is NOT OK
and should be:
#define OP_MODE_MASK0x3
#define OP_MODE_SHIFT 28
Regards,
Madhav
BR,
Jani.
+#define LINK_READY(1 << 20)
+#define PIX_FMT(x)(x << 16)
+#define PIX_FMT_MASK (0x3 << 16)
+#define PIX_FMT_RGB5650x0
+#define PIX_FMT_RGB666_PACKED 0x1
+#define PIX_FMT_RGB666_LOOSE 0x2
+#define PIX_FMT_RGB8880x3
+#define PIX_FMT_RGB101010 0x4
+#define PIX_FMT_RGB121212 0x5
+#define PIX_FMT_COMPRESSED0x6
+#define BGR_TRANSMISSION (1 << 15)
+#define PIX_VIRT_CHAN(x) (x << 12)
+#define PIX_VIRT_CHAN_MASK(0x3 << 12)
+#define PIX_BUF_THRESHOLD(x) ((x & 0x3) << 10)
+#define PIX_BUF_THRESHOLD_MASK(0x3 << 10)
+#define PIX_BUF_THRESHOLD_1_4 0x0
+#define PIX_BUF_THRESHOLD_1_2 0x1
+#define PIX_BUF_THRESHOLD_3_4 0x2
+#define PIX_BUF_THRESHOLD_FULL0x3
+#define CONTINUOUS_CLK(x) (x << 8)
+#define CONTINUOUS_CLK_MASK (0x3 << 8)
+#define CLK_ENTER_LP_AFTER_DATA 0x0
+#define CLK_HS_OR_LP 0x2
+#define CLK_HS_CONTINUOUS 0x3
+#define LINK_CALIBRATION(x) (x << 4)
+#define LINK_CALIBRATION_MASK (0x3 << 4)
+#define CALIBRATION_DISABLED 0x0
+#define CALIBRATION_ENABLED_INITIAL_ONLY 0x2
+#define CALIBRATION_ENABLED_INITIAL_PERIODIC 0x3
+#define S3D_ORIENTATION(x)(x << 1)
+#define S3D_ORIENTATION_MASK (0x1 << 1)
+#define S3D_ORIENTATION_PORTRAIT 0x0
+#define S3D_ORIENTATION_LANDSCAPE 0x1
+#define EOTP_DISABLED (1 << 0)
+
/* bits 31:0 */
#define _MIPIA_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb084)
#define _MIPIC_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb884)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 11/13] drm/i915/icl: Add macros for MMIO of DSI transcoder registers
On 7/19/2018 9:52 PM, Ville Syrjälä wrote:
On Tue, Jul 10, 2018 at 03:10:12PM +0530, Madhav Chauhan wrote:
This patch adds _MMIO_DSI and _DSI_TRANS macros for accessing
DSI transcoder registers.
Credits-to: Jani N
Cc: Jani Nikula
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1d13ba9..62bc76e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9576,6 +9576,11 @@ enum skl_power_gate {
#define _MIPI_PORT(port, a, c)(((port) == PORT_A) ? a : c)/*
ports A and C only */
#define _MMIO_MIPI(port, a, c)_MMIO(_MIPI_PORT(port, a, c))
+/* gen11 DSI */
+#define _DSI_TRANS(tc, dsi0, dsi1) (((tc) == TRANSCODER_DSI_0) ? \
+(dsi0) : (dsi1))
_PIPE() etc. should result in slughtly better code IIRC.
Can you please clarify on this??
Regards,
Madhav
+#define _MMIO_DSI(tc, dsi0, dsi1) _MMIO(_DSI_TRANS(tc, dsi0, dsi1))
+
#define MIPIO_TXESC_CLK_DIV1 _MMIO(0x160004)
#define GLK_TX_ESC_CLK_DIV1_MASK 0x3FF
#define MIPIO_TXESC_CLK_DIV2 _MMIO(0x160008)
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 09/13] drm/i915/icl: Program TA_TIMING_PARAM registers
On Wed, 12 Sep 2018, Madhav Chauhan wrote:
> On 9/12/2018 12:56 AM, Jani Nikula wrote:
>> On Fri, 20 Jul 2018, "Chauhan, Madhav" wrote:
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Thursday, July 19, 2018 9:51 PM
To: Chauhan, Madhav
Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani ;
Zanoni, Paulo R ; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH v5 09/13] drm/i915/icl: Program
TA_TIMING_PARAM registers
On Tue, Jul 10, 2018 at 03:10:10PM +0530, Madhav Chauhan wrote:
> This patch programs D-PHY timing parameters for the bus turn around
> flow(in escape clocks) only if dsi link frequency <=800 MHz using
> DPHY_TA_TIMING_PARAM and its identical register
DSI_TA_TIMING_PARAM
> (inside DSI Controller within the Display Core).
>
> Signed-off-by: Madhav Chauhan
> ---
> drivers/gpu/drm/i915/icl_dsi.c | 21 +
> drivers/gpu/drm/i915/intel_dsi.h | 1 +
> drivers/gpu/drm/i915/intel_dsi_vbt.c | 1 +
> 3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c
> b/drivers/gpu/drm/i915/icl_dsi.c index 832772d..8fd5284 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -302,6 +302,27 @@ static void gen11_dsi_setup_dphy_timings(struct
intel_encoder *encoder)
> I915_WRITE(DSI_DATA_TIMING_PARAM(port),
> intel_dsi->dphy_data_lane_reg);
> }
> +
> + /*
> + * If DSI link operating at or below an 800 MHz,
> + * TA_SURE should be override and programmed to
> + * a value '0' inside TA_PARAM_REGISTERS otherwise
> + * leave all fields at HW default values.
> + */
> + if (intel_dsi->bitrate_khz <= KHz(800)) {
The KHz(800) confuses me. My brain thinks this value is 800 kHz when it's
not. So I'd write it without the KHz() macro.
>>> Ok. Initially I wrote without using KHz macro, but got comment to use KHz
>>> macro :)
>> Did I? Oh well. Go with 80.
>
> Ok :)
>
>>
>> Please don't add additional state with intel_dsi->bitrate_khz when you
>> can calculate the bitrate at any time. Add a function to do it if you
>> like, and use it in both places.
>
> Ok. But we will reusing this bitrate(same value) in multiple place.
> Shouldn't we cache it rather than calculating everytime??
Depends. The performance impact is neglible. Caching has its own
problems, but here I suppose you don't need to invalidate it. In the
end, I think it boils down to code readability. In this case, I guess go
with what you have then.
BR,
Jani.
>
> Regards,
> Madhav
>
>>
>> BR,
>> Jani.
>>
>>
>>> Regards,
>>> Madhav
>>>
> + for_each_dsi_port(port, intel_dsi->ports) {
> + tmp = I915_READ(DPHY_TA_TIMING_PARAM(port));
> + tmp &= ~TA_SURE_TIME_MASK;
> + tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0));
> + I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp);
> +
> + /* shadow register inside display core */
> + tmp = I915_READ(DSI_TA_TIMING_PARAM(port));
> + tmp &= ~TA_SURE_TIME_MASK;
> + tmp |= (TA_SURE_OVERRIDE | TA_SURE_TIME(0));
> + I915_WRITE(DSI_TA_TIMING_PARAM(port), tmp);
> + }
> + }
> }
>
> static void gen11_dsi_enable_port_and_phy(struct intel_encoder
> *encoder) diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 9fd8526..25e7396 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -101,6 +101,7 @@ struct intel_dsi {
>
> u16 init_count;
> u32 pclk;
> + u32 bitrate_khz;
> u16 burst_mode_ratio;
>
> /* all delays in ms */
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 428290d..a9a98a4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -589,6 +589,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi,
u16 panel_id)
> intel_dsi->pclk = pclk;
>
> bitrate = (pclk * bpp) / intel_dsi->lane_count;
> + intel_dsi->bitrate_khz = bitrate;
>
> switch (intel_dsi->escape_clk_div) {
> case 0:
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
>
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lis
Re: [Intel-gfx] [PATCH 01/26] drm/i915/ringbuffer: Reload PDs harder on byt/bcs
Quoting Chris Wilson (2018-09-11 14:57:45)
> Baytrail takes a little more convincing that it needs to actually reload
> its Page Directoy (ppGTT) before the context switch, so repeat it until
> it gets the message. Once again the arbitrary values here are
> empirically derived.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861
> Testcase: igt/gem_exec_parallel/fds
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 472939f5c18f..d0ef50bf930a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1677,9 +1677,26 @@ static int switch_context(struct i915_request *rq)
> GEM_BUG_ON(HAS_EXECLISTS(rq->i915));
>
> if (ppgtt) {
> - ret = load_pd_dir(rq, ppgtt);
> - if (ret)
> - goto err;
> + int loops;
> +
> + /*
> +* Baytail takes a little more convincing that it really needs
> +* to reload the PD between contexts. It is not just a little
> +* longer, as adding more stalls after the load_pd_dir (i.e.
> +* adding a long loop around flush_pd_dir) is not as effective
> +* as reloading the PD umpteen times. 32 is derived from
> +* experimentation (gem_exec_parallel/fds) and has no good
> +* explanation.
> +*/
> + loops = 1;
> + if (engine->id == BCS && IS_VALLEYVIEW(engine->i915))
> + loops = 32;
> +
> + do {
> + ret = load_pd_dir(rq, ppgtt);
> + if (ret)
> + goto err;
> + } while (--loops);
This what they call black magic.
Reviewed-by: Joonas Lahtinen
Regards, Joonas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
On Wed, 12 Sep 2018, Madhav Chauhan wrote: > On 9/12/2018 1:00 AM, Jani Nikula wrote: >> On Tue, 10 Jul 2018, Madhav Chauhan wrote: >> The convention is to define macros for field values that you can OR >> directly in place instead of requiring a shift. Please stick to the >> conventions. Use _SHIFT and _MASK. >> >> We can debate the relative merits of both approaches at some point, but >> this is not the time. > > Just to understand this point correctly, > > #define OP_MODE(x) ((x) << 28) is OK This is acceptable when the values for the field are *not* defined as separate macros. The convention is that the values for fields are defined already shifted in place, and that would conflict. > but > #define OP_MODE_MASK (0x3 << 28) is NOT OK This is okay and recommended. > and should be: > #define OP_MODE_MASK0x3 This is not okay. > #define OP_MODE_SHIFT 28 This is okay. Please read the big comment with examples near the top of i915_reg.h, and let me know which part is not clear from that. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ringbuffer: Reload PDs harder on byt/bcs
Quoting Chris Wilson (2018-09-10 14:08:08) > Baytrail takes a little more convincing that it needs to actually reload > its Page Directoy (ppGTT) before the context switch, so repeat it until > it gets the message. Once again the arbitrary values here are > empirically derived. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107861 > Testcase: igt/gem_exec_parallel/fds > Signed-off-by: Chris Wilson From another thread, Reviewed-by: Joonas Lahtinen Ta, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/26] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
Quoting Chris Wilson (2018-09-11 14:57:46) > If the caller supplies more than 4G of objects and than one that has to > be in the low 4G, it is possible for the low 4G to be full before we > attempt to find room for the last object that must be there. As we don't > reorder the two types, every pass hits the same problem and we fail with > ENOSPC. However, if we impose a little bit of ordering between the two > classes of objects, on the second pass we will be able to fit the > special object as we do it first. For setups that only use !48b objects, > we now reverse the order between passes, hopefully making the subsequent > passes more likely to succeed given that we are trying a different > order (rather than repeating the previous pass!) > > Signed-off-by: Chris Wilson Might be worthy dropping a one-liner comment in the fashion of. /* Avoid congesting 4GB region required for last object. */ Reviewed-by: Joonas Lahtinen Regards, Joonas > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 22b4cb775576..d70d142f5338 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -696,6 +696,8 @@ static int eb_reserve(struct i915_execbuffer *eb) > list_add(&vma->exec_link, &eb->unbound); > else if (flags & __EXEC_OBJECT_NEEDS_MAP) > list_add_tail(&vma->exec_link, &eb->unbound); > + else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) > + list_add(&vma->exec_link, &last); > else > list_add_tail(&vma->exec_link, &last); > } > -- > 2.19.0.rc2 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Differentiate the lack of an interface from invalid parameter (rev3)
== Series Details ==
Series: drm: Differentiate the lack of an interface from invalid parameter
(rev3)
URL : https://patchwork.freedesktop.org/series/49536/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4808 -> Patchwork_10149 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/49536/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_10149 that come from known issues:
=== IGT changes ===
Issues hit
igt@gem_exec_suspend@basic-s3:
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
igt@kms_psr@primary_mmap_gtt:
{fi-cnl-u}: NOTRUN -> FAIL (fdo#107383) +3
igt@kms_psr@sprite_plane_onoff:
fi-bdw-samus: NOTRUN -> FAIL (fdo#107360, fdo#107383)
igt@pm_rpm@module-reload:
fi-hsw-peppy: NOTRUN -> DMESG-WARN (fdo#107603)
fi-bdw-samus: NOTRUN -> DMESG-WARN (fdo#107603)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#106685) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: FAIL (fdo#103167) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-byt-clapper: FAIL (fdo#107362) -> PASS
igt@kms_psr@primary_page_flip:
fi-whl-u: FAIL (fdo#107336) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107360 https://bugs.freedesktop.org/show_bug.cgi?id=107360
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
fdo#107603 https://bugs.freedesktop.org/show_bug.cgi?id=107603
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
== Participating hosts (47 -> 46) ==
Additional (4): fi-hsw-peppy fi-cnl-u fi-icl-u fi-bdw-samus
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4808 -> Patchwork_10149
CI_DRM_4808: 1a24d0c172a97f513014071eb0e5532a7d1ccbce @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10149: 5e5ca8579b38a0a7b1757c92ce4be819af472615 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5e5ca8579b38 drm: Differentiate the lack of an interface from invalid parameter
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10149/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
If the caller supplies more than 4G of objects and than one that has to be in the low 4G, it is possible for the low 4G to be full before we attempt to find room for the last object that must be there. As we don't reorder the two types, every pass hits the same problem and we fail with ENOSPC. However, if we impose a little bit of ordering between the two classes of objects, on the second pass we will be able to fit the special object as we do it first. For setups that only use !48b objects, we now reverse the order between passes, hopefully making the subsequent passes more likely to succeed given that we are trying a different order (rather than repeating the previous pass!) v2: Quick one line explanation for the relative priorities given to reservations. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index aaa8a26aae86..09187286d346 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -693,9 +693,14 @@ static int eb_reserve(struct i915_execbuffer *eb) eb_unreserve_vma(vma, &eb->flags[i]); if (flags & EXEC_OBJECT_PINNED) + /* Pinned must have their slot */ list_add(&vma->exec_link, &eb->unbound); else if (flags & __EXEC_OBJECT_NEEDS_MAP) + /* Map require the lowest 256MiB (aperture) */ list_add_tail(&vma->exec_link, &eb->unbound); + else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) + /* Prioritise 4GiB region for restricted bo */ + list_add(&vma->exec_link, &last); else list_add_tail(&vma->exec_link, &last); } -- 2.19.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/4] drm/i915/icl: Preparations for enabling Y210, Y212, Y216 formats
From: Vidya Srinivas
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
drivers/gpu/drm/i915/intel_sprite.c | 3 +++
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2b77d93..7c68a0d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3512,6 +3512,12 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
case DRM_FORMAT_NV12:
return PLANE_CTL_FORMAT_NV12;
+ case DRM_FORMAT_Y210:
+ return PLANE_CTL_FORMAT_Y210;
+ case DRM_FORMAT_Y212:
+ return PLANE_CTL_FORMAT_Y212;
+ case DRM_FORMAT_Y216:
+ return PLANE_CTL_FORMAT_Y216;
default:
MISSING_CASE(pixel_format);
}
@@ -4960,6 +4966,9 @@ static int skl_update_scaler_plane(struct
intel_crtc_state *crtc_state,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_Y210:
+ case DRM_FORMAT_Y212:
+ case DRM_FORMAT_Y216:
break;
default:
DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format
0x%x\n",
@@ -13422,6 +13431,9 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
+ case DRM_FORMAT_Y210:
+ case DRM_FORMAT_Y212:
+ case DRM_FORMAT_Y216:
case DRM_FORMAT_NV12:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
@@ -14551,6 +14563,9 @@ static int intel_framebuffer_init(struct
intel_framebuffer *intel_fb,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
case DRM_FORMAT_VYUY:
+ case DRM_FORMAT_Y210:
+ case DRM_FORMAT_Y212:
+ case DRM_FORMAT_Y216:
if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
DRM_DEBUG_KMS("unsupported pixel format: %s\n",
drm_get_format_name(mode_cmd->pixel_format, &format_name));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 9600ccf..f7e2768 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1419,6 +1419,9 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
+ case DRM_FORMAT_Y210:
+ case DRM_FORMAT_Y212:
+ case DRM_FORMAT_Y216:
case DRM_FORMAT_NV12:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/4] Enable Y210, Y212, Y216 formats for ICL
These patches enable packed format YUV422-Y210, Y212 and Y216 for 10, 12 and 16 bit respectively for ICL. For user space component IGT:WIP v2: addressed review comments of mahesh and alexandru hdr handling of these 64 bit pixel format not inscope of this series Vidya Srinivas (4): drm: Add Y210, Y212, Y216 format definitions and fourcc drm/i915/icl: Add Y210, Y212, Y216 plane control definitions drm/i915/icl: Preparations for enabling Y210, Y212, Y216 formats drm/i915/icl: Enable Y210, Y212, Y216 format for primary and sprite planes drivers/gpu/drm/drm_fourcc.c | 3 +++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 40 ++-- drivers/gpu/drm/i915/intel_sprite.c | 25 -- include/uapi/drm/drm_fourcc.h| 8 5 files changed, 75 insertions(+), 4 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/4] drm: Add Y210, Y212, Y216 format definitions and fourcc
From: Vidya Srinivas
The following pixel formats are packed format that follows 4:2:2
chroma sampling. For memory represenation each component is
allocated 16 bits each. Thus each pixel occupies a DWORD.
Y210:Valid data occupies MSB 10 bits.
LSB 6 bits are filled with zeroes.
Y212:Valid data occupies MSB 12 bits.
LSB 4 bits are filled with zeroes.
Y216:Valid data occupies 16 bits,
doesn't require any padding bits.
First 16 bits stores the Y value and the next 16 bits stores one
of the chroma samples alternatively. The first luma sample will
be accompanied by first U sample and second luma sample is
accompanied by the first V sample.
v2: is_yuv setted to true (mahesh)
different order of yuv samples (mahesh): still update from
hardware team pending
change in comment (alexandru)
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/drm_fourcc.c | 3 +++
include/uapi/drm/drm_fourcc.h | 8
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index be1d6aa..143c26a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -190,6 +190,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_VYUY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true,
.is_yuv = true },
+ { .format = DRM_FORMAT_Y210,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+ { .format = DRM_FORMAT_Y212,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+ { .format = DRM_FORMAT_Y216,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
};
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 139632b..95118cc 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -153,6 +153,14 @@
#define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U', 'V') /*
[31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
/*
+ * Y2xx indicate for each component, xx valid data occupy msb
+ * 16-xx padding occupy lsb
+ */
+#define DRM_FORMAT_Y210 fourcc_code('Y', '2', '1', '0') /* [63:0]
Y0:Cb0:Y1:Cr1 10:10:10:10 little endian */
+#define DRM_FORMAT_Y212 fourcc_code('Y', '2', '1', '2') /* [63:0]
Y0:Cb0:Y1:Cr1 12:12:12:12 little endian */
+#define DRM_FORMAT_Y216 fourcc_code('Y', '2', '1', '6') /* [63:0]
Y0:Cb0:Y1:Cr1 16:16:16:16 little endian */
+
+/*
* 2 plane RGB + A
* index 0 = RGB plane, same format as the corresponding non _A8 format has
* index 1 = A plane, [7:0] A
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/4] drm/i915/icl: Add Y210, Y212, Y216 plane control definitions
From: Vidya Srinivas
Added needed plane control flag definitions for Y210, Y212 and
Y216 formats.
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c80e569..d90d51c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6506,6 +6506,9 @@ enum {
#define PLANE_CTL_FORMAT_RGB_565 (14 << 24)
#define ICL_PLANE_CTL_FORMAT_MASK(0x1f << 23)
#define PLANE_CTL_PIPE_CSC_ENABLE(1 << 23) /* Pre-GLK */
+#define PLANE_CTL_FORMAT_Y210(1 << 23)
+#define PLANE_CTL_FORMAT_Y212(3 << 23)
+#define PLANE_CTL_FORMAT_Y216(5 << 23)
#define PLANE_CTL_KEY_ENABLE_MASK(0x3 << 21)
#define PLANE_CTL_KEY_ENABLE_SOURCE (1 << 21)
#define PLANE_CTL_KEY_ENABLE_DESTINATION (2 << 21)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Enable Y210, Y212, Y216 format for primary and sprite planes
From: Vidya Srinivas
In this patch, a list for icl specific pixel formats is created
in which Y210, Y212 and Y216 pixel formats are added along with
legacy pixel formats for primary and sprite plane.
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/intel_display.c | 25 +++--
drivers/gpu/drm/i915/intel_sprite.c | 22 --
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7c68a0d..f341cbd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -105,6 +105,24 @@
DRM_FORMAT_NV12,
};
+static const uint32_t icl_primary_formats[] = {
+ DRM_FORMAT_C8,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_Y210,
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_Y216,
+};
+
static const uint64_t skl_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -13725,8 +13743,11 @@ bool skl_plane_has_planar(struct drm_i915_private
*dev_priv,
if (INTEL_GEN(dev_priv) >= 9) {
primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
PLANE_PRIMARY);
-
- if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ intel_primary_formats = icl_primary_formats;
+ num_formats = ARRAY_SIZE(icl_primary_formats);
+ } else if (skl_plane_has_planar(dev_priv, pipe,
+ PLANE_PRIMARY)) {
intel_primary_formats = skl_pri_planar_formats;
num_formats = ARRAY_SIZE(skl_pri_planar_formats);
} else {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index f7e2768..2e61fe19 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1281,6 +1281,21 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device
*dev, void *data,
DRM_FORMAT_NV12,
};
+static uint32_t icl_plane_formats[] = {
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_Y210,
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_Y216,
+};
+
static const uint64_t skl_plane_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -1536,8 +1551,11 @@ struct intel_plane *
intel_plane->disable_plane = skl_disable_plane;
intel_plane->get_hw_state = skl_plane_get_hw_state;
- if (skl_plane_has_planar(dev_priv, pipe,
-PLANE_SPRITE0 + plane)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ plane_formats = icl_plane_formats;
+ num_plane_formats = ARRAY_SIZE(icl_plane_formats);
+ } else if (skl_plane_has_planar(dev_priv, pipe,
+ PLANE_SPRITE0 + plane)) {
plane_formats = skl_planar_formats;
num_plane_formats = ARRAY_SIZE(skl_planar_formats);
} else {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
== Series Details == Series: drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G URL : https://patchwork.freedesktop.org/series/49548/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809 -> Patchwork_10150 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49548/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10150 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_pipe_crc_basic@hang-read-crc-pipe-b: fi-ilk-650: PASS -> DMESG-WARN (fdo#106387) igt@kms_pipe_crc_basic@read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#107362) igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-r: FAIL (fdo#107336) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10150 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10150: 6120c5292b7c906538cba66cf5e0e33f16dd49ec @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 6120c5292b7c drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10150/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Enable Y210, Y212, Y216 formats for ICL (rev2)
== Series Details ==
Series: Enable Y210, Y212, Y216 formats for ICL (rev2)
URL : https://patchwork.freedesktop.org/series/48729/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
7d34116618f8 drm: Add Y210, Y212, Y216 format definitions and fourcc
-:38: WARNING:LONG_LINE: line over 100 characters
#38: FILE: drivers/gpu/drm/drm_fourcc.c:193:
+ { .format = DRM_FORMAT_Y210,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
-:39: WARNING:LONG_LINE: line over 100 characters
#39: FILE: drivers/gpu/drm/drm_fourcc.c:194:
+ { .format = DRM_FORMAT_Y212,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
-:40: WARNING:LONG_LINE: line over 100 characters
#40: FILE: drivers/gpu/drm/drm_fourcc.c:195:
+ { .format = DRM_FORMAT_Y216,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
-:56: WARNING:LONG_LINE_COMMENT: line over 100 characters
#56: FILE: include/uapi/drm/drm_fourcc.h:159:
+#define DRM_FORMAT_Y210 fourcc_code('Y', '2', '1', '0') /* [63:0]
Y0:Cb0:Y1:Cr1 10:10:10:10 little endian */
-:57: WARNING:LONG_LINE_COMMENT: line over 100 characters
#57: FILE: include/uapi/drm/drm_fourcc.h:160:
+#define DRM_FORMAT_Y212 fourcc_code('Y', '2', '1', '2') /* [63:0]
Y0:Cb0:Y1:Cr1 12:12:12:12 little endian */
-:58: WARNING:LONG_LINE_COMMENT: line over 100 characters
#58: FILE: include/uapi/drm/drm_fourcc.h:161:
+#define DRM_FORMAT_Y216 fourcc_code('Y', '2', '1', '6') /* [63:0]
Y0:Cb0:Y1:Cr1 16:16:16:16 little endian */
total: 0 errors, 6 warnings, 0 checks, 23 lines checked
99b3ece40ec8 drm/i915/icl: Add Y210, Y212, Y216 plane control definitions
12f11ab479da drm/i915/icl: Preparations for enabling Y210, Y212, Y216 formats
-:8: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one
total: 0 errors, 1 warnings, 0 checks, 48 lines checked
8ae9cb919ce4 drm/i915/icl: Enable Y210, Y212, Y216 format for primary and
sprite planes
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
If we try and fail to allocate a i915_request, we apply some backpressure on the clients to throttle the memory allocations coming from i915.ko. Currently, we wait until completely idle, but this is far too heavy and leads to some situations where the only escape is to declare a client hung and reset the GPU. The intent is to only ratelimit the allocation requests, so we need only wait for a jiffie before using the normal direct reclaim. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 09ed48833b54..588bc5a4d18b 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED | I915_WAIT_INTERRUPTIBLE, -MAX_SCHEDULE_TIMEOUT); +1); if (ret) goto err_unreserve; -- 2.19.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] igt/gem_exec_await: Flush vm caches between runs
On allocating a request, we apply some backpressure if we fail to
allocate a request. The backpressure we apply involves waiting for the
device to idle, causing a stall on the clients (trying to throttle heavy
allocators) and as we may be inside a plugged critical section, the only
way to idle the GPU is by waiting for hangcheck to kick in.
Try to avoid the allocation failure by trimming the VM caches between
iterations.
References: https://bugs.freedesktop.org/show_bug.cgi?id=106680
Signed-off-by: Chris Wilson
---
tests/gem_exec_await.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tests/gem_exec_await.c b/tests/gem_exec_await.c
index 5cfeb8ec8..b9a2c6105 100644
--- a/tests/gem_exec_await.c
+++ b/tests/gem_exec_await.c
@@ -147,6 +147,13 @@ static void wide(int fd, int ring_size, int timeout,
unsigned int flags)
count = 0;
igt_until_timeout(timeout) {
struct timespec start, now;
+
+ /*
+* Flush all caches between runs, we do not want to encounter
+* reclaim in the middle of the plugged critical section.
+*/
+ intel_purge_vm_caches(fd);
+
for (unsigned e = 0; e < nengine; e++) {
uint64_t address;
int i;
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Enable Y210, Y212, Y216 formats for ICL (rev2)
== Series Details == Series: Enable Y210, Y212, Y216 formats for ICL (rev2) URL : https://patchwork.freedesktop.org/series/48729/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4809 -> Patchwork_10151 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10151 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10151, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/48729/revisions/2/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10151: === IGT changes === Possible regressions igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-icl-u: PASS -> INCOMPLETE == Known issues == Here are the changes found in Patchwork_10151 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@hang-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) Possible fixes igt@kms_psr@primary_page_flip: fi-kbl-r: FAIL (fdo#107336) -> PASS fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10151 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10151: 8ae9cb919ce46331fe110716ae28db135c8860c8 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 8ae9cb919ce4 drm/i915/icl: Enable Y210, Y212, Y216 format for primary and sprite planes 12f11ab479da drm/i915/icl: Preparations for enabling Y210, Y212, Y216 formats 99b3ece40ec8 drm/i915/icl: Add Y210, Y212, Y216 plane control definitions 7d34116618f8 drm: Add Y210, Y212, Y216 format definitions and fourcc == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10151/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for add LG panel to dpcd quirk database (rev2)
== Series Details == Series: add LG panel to dpcd quirk database (rev2) URL : https://patchwork.freedesktop.org/series/49413/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809 -> Patchwork_10152 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49413/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_10152 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_evict: fi-bsw-kefka: PASS -> DMESG-WARN (fdo#107709) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: PASS -> INCOMPLETE (fdo#102657) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-r: FAIL (fdo#107336) -> PASS fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107709 https://bugs.freedesktop.org/show_bug.cgi?id=107709 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10152 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10152: 4947b044fdaf8bac17fdd9c581451e2e0848b9f7 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 4947b044fdaf drm: add LG eDP panel to quirk database fd3ec3d72559 drm: Change limited M/N quirk to constant N quirk. d9c4e8c40739 drm: Add support for device_id based detection. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10152/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATH i-g-t v12 2/2] tests: add slice power programming test
Quoting Tvrtko Ursulin (2018-09-11 15:42:10)
> + last_with_engines = -1;
> + for (class = 0; class < ~0; class++) {
> + for (instance = 0; instance < ~0; instance++) {
> + int ret;
> +
> + sseu.class = class;
> + sseu.instance = instance;
> +
> + ret = __gem_context_set_param(fd, &arg);
> +
> + if (has_engine(fd, class, instance)) {
> + if (engine_supports_sseu(fd, class, instance))
Meh, . I just don't like having hardcoded db on this side of the
ABI. The ABI imo is to ask the kernel if the device/engine is supported,
and should not allow for assumptions.
> +static void
> +test_dynamic(int fd, unsigned int flags)
> +{
> + uint64_t pg_slice_mask = mask_minus_one(__slice_mask__);
> + unsigned int pg_slice_count = __slice_count__ - 1;
> + struct drm_i915_gem_context_param_sseu sseu = { };
> + struct drm_i915_gem_context_param arg =
> + { .param = I915_CONTEXT_PARAM_SSEU,
> + .ctx_id = gem_context_create(fd),
> + .size = sizeof(sseu),
> + .value = to_user_pointer(&sseu) };
> + igt_spin_t *spin = NULL;
> +
> + gem_context_get_param(fd, &arg);
> +
> + /* First set the default mask */
> + if (flags & TEST_BUSY)
> + spin = __spin_sync(fd, arg.ctx_id, I915_EXEC_RENDER);
> +
> + sseu.slice_mask = __slice_mask__;
> + gem_context_set_param(fd, &arg);
I would also suggest a reset test here. Both reset when idle, and by
hangcheck/forced-reset of the spinner & active context.
And across suspend.
> + igt_assert_eq(read_slice_count_busy(fd, arg.ctx_id, 0, spin),
> + __slice_count__);
> + igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__);
In the read_slice I would suggest having a
igt_assert(gem_bo_busy(spin->handle));
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATH i-g-t 1/2] intel: Be consistent with test results on simulation
Quoting Tvrtko Ursulin (2018-09-12 10:33:05) > From: Tvrtko Ursulin > > Tests which call intel_require_memory currently always skip on simulation, > unless they fail first due insufficient memory. This can create different > outcomes depending on the simulation environment so move the simulation > skip to the start of the function for 100% consistency in results. > > Signed-off-by: Tvrtko Ursulin > Cc: Radoslaw Szwichtenberg Makes sense. Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Limit the backpressure for i915_request allocation
== Series Details == Series: drm/i915: Limit the backpressure for i915_request allocation URL : https://patchwork.freedesktop.org/series/49555/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809 -> Patchwork_10153 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49555/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10153 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: PASS -> DMESG-WARN (fdo#107139, fdo#105128) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) Possible fixes igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS igt@kms_psr@primary_page_flip: fi-kbl-r: FAIL (fdo#107336) -> PASS fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 == Participating hosts (51 -> 45) == Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10153 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10153: 193b2e33a951f696605d282a49529f35bbb19cb1 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 193b2e33a951 drm/i915: Limit the backpressure for i915_request allocation == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10153/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/icl: Add Y210, Y212, Y216 plane control definitions
On 12.09.2018 13:32, Swati Sharma wrote:
From: Vidya Srinivas
Added needed plane control flag definitions for Y210, Y212 and
Y216 formats.
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c80e569..d90d51c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6506,6 +6506,9 @@ enum {
#define PLANE_CTL_FORMAT_RGB_565(14 << 24)
#define ICL_PLANE_CTL_FORMAT_MASK (0x1f << 23)
#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
+#define PLANE_CTL_FORMAT_Y210(1 << 23)
+#define PLANE_CTL_FORMAT_Y212(3 << 23)
+#define PLANE_CTL_FORMAT_Y216(5 << 23)
#define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
#define PLANE_CTL_KEY_ENABLE_SOURCE (1 << 21)
#define PLANE_CTL_KEY_ENABLE_DESTINATION(2 << 21)
Reviewed-by: Juha-Pekka Heikkila
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/4] drm: Add Y210, Y212, Y216 format definitions and fourcc
On 12.09.2018 13:32, Swati Sharma wrote:
From: Vidya Srinivas
The following pixel formats are packed format that follows 4:2:2
chroma sampling. For memory represenation each component is
allocated 16 bits each. Thus each pixel occupies a DWORD.
Just to be clear I wouldn't use 'DWORD' here but if it's 64 bits bits
call it so. Not a big issue as its just in the patch comment but still.
Y210:Valid data occupies MSB 10 bits.
LSB 6 bits are filled with zeroes.
Y212:Valid data occupies MSB 12 bits.
LSB 4 bits are filled with zeroes.
Y216:Valid data occupies 16 bits,
doesn't require any padding bits.
First 16 bits stores the Y value and the next 16 bits stores one
of the chroma samples alternatively. The first luma sample will
be accompanied by first U sample and second luma sample is
accompanied by the first V sample.
v2: is_yuv setted to true (mahesh)
different order of yuv samples (mahesh): still update from
hardware team pending
change in comment (alexandru)
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/drm_fourcc.c | 3 +++
include/uapi/drm/drm_fourcc.h | 8
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index be1d6aa..143c26a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -190,6 +190,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_VYUY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true,
.is_yuv = true },
+ { .format = DRM_FORMAT_Y210,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+ { .format = DRM_FORMAT_Y212,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+ { .format = DRM_FORMAT_Y216,.depth = 0,
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
};
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 139632b..95118cc 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -153,6 +153,14 @@
#define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') /*
[31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
/*
+ * Y2xx indicate for each component, xx valid data occupy msb
+ * 16-xx padding occupy lsb
+ */
+#define DRM_FORMAT_Y210 fourcc_code('Y', '2', '1', '0') /* [63:0]
Y0:Cb0:Y1:Cr1 10:10:10:10 little endian */
+#define DRM_FORMAT_Y212 fourcc_code('Y', '2', '1', '2') /* [63:0]
Y0:Cb0:Y1:Cr1 12:12:12:12 little endian */
+#define DRM_FORMAT_Y216 fourcc_code('Y', '2', '1', '6') /* [63:0]
Y0:Cb0:Y1:Cr1 16:16:16:16 little endian */
I think what Alexandru-Cosmin Gheorghe meant with earlier review comment
is these defines should be better explained here in part which will be
visible in header file. Looking how other formats have been commented
this would probably be something along the lines of
#define DRM_FORMAT_Y210 fourcc_code('Y', '2', '1', '0') /*
[63:0] Y0:x:Cb0:x:Y1:x:Cr1:x 10:6:10:6:10:6:10:6 little endian */
or something.
/Juha-Pekka
+
+/*
* 2 plane RGB + A
* index 0 = RGB plane, same format as the corresponding non _A8 format has
* index 1 = A plane, [7:0] A
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm: Differentiate the lack of an interface from invalid parameter (rev3)
== Series Details == Series: drm: Differentiate the lack of an interface from invalid parameter (rev3) URL : https://patchwork.freedesktop.org/series/49536/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4808_full -> Patchwork_10149_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10149_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_schedule@preempt-hang-blt: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: shard-hsw: PASS -> FAIL (fdo#105767) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt: shard-glk: PASS -> FAIL (fdo#103167) igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend: shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133) Possible fixes igt@kms_cursor_crc@cursor-64x64-suspend: shard-snb: INCOMPLETE (fdo#105411) -> PASS +1 igt@kms_cursor_legacy@cursor-vs-flip-toggle: shard-hsw: FAIL (fdo#103355) -> PASS igt@perf@blocking: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4808 -> Patchwork_10149 CI_DRM_4808: 1a24d0c172a97f513014071eb0e5532a7d1ccbce @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10149: 5e5ca8579b38a0a7b1757c92ce4be819af472615 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10149/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Add POWER_DOMAIN_GT_IRQ to ICL DC_OFF_POWER_DOMAINS
On Tue, Sep 11, 2018 at 06:19:56PM -0700, José Roberto de Souza wrote: > Without this gem will not be able to turn off DC states to redunce > interruption latency when no sink is being driven by driver. Do we know that for sure this is still required? Some kind of test results to confirm would be nice. > > Cc: Paulo Zanoni > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 480dadb1047b..af8a0b6ee79b 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1978,6 +1978,7 @@ void intel_display_power_put(struct drm_i915_private > *dev_priv, >*/ > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > ICL_PW_2_POWER_DOMAINS |\ > + BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > -- > 2.18.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/icl: Add POWER_DOMAIN_GT_IRQ to ICL DC_OFF_POWER_DOMAINS
Quoting Ville Syrjälä (2018-09-12 13:48:42) > On Tue, Sep 11, 2018 at 06:19:56PM -0700, José Roberto de Souza wrote: > > Without this gem will not be able to turn off DC states to redunce > > interruption latency when no sink is being driven by driver. > > Do we know that for sure this is still required? Some kind of test > results to confirm would be nice. After all it was working around a bug in the dmc that was meant to be fixed at some point in the future. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
Hi Kumar, Takashi,
On Tue, Jun 19, 2018 at 03:01:11PM -0700, Abhay Kumar wrote:
> From: Ville Syrjälä
>
> CDCLK has to be at least twice the BLCK regardless of audio. Audio
> driver has to probe using this hook and increase the clock even in
> absence of any display.
>
> v2: Use atomic refcount for get_power, put_power so that we can
> call each once(Abhay).
> v3: Reset power well 2 to avoid any transaction on iDisp link
> during cdclk change(Abhay).
> v4: Remove Power well 2 reset workaround(Ville).
> v5: Remove unwanted Power well 2 register defined in v4(Abhay).
>
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Abhay Kumar
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_audio.c | 67
> +---
> drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
> drivers/gpu/drm/i915/intel_display.c | 7 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 5 files changed, 83 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6104d7115054..a4a386a5db69 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1702,6 +1702,7 @@ struct drm_i915_private {
> unsigned int hpll_freq;
> unsigned int fdi_pll_freq;
> unsigned int czclk_freq;
> + u32 get_put_refcount;
>
> struct {
> /*
> @@ -1719,6 +1720,8 @@ struct drm_i915_private {
> struct intel_cdclk_state actual;
> /* The current hardware cdclk state */
> struct intel_cdclk_state hw;
> +
> + int force_min_cdclk;
> } cdclk;
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..ca8f04c7cbb3 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder
> *encoder,
>
> if (!connector->eld[0])
> return;
> -
> DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>connector->base.id,
>connector->name,
> @@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
> *dev_priv)
> }
> }
>
> +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> + bool enable)
> +{
> + struct drm_modeset_acquire_ctx ctx;
> + struct drm_atomic_state *state;
> + int ret;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> + state = drm_atomic_state_alloc(&dev_priv->drm);
> + if (WARN_ON(!state))
> + return;
> +
> + state->acquire_ctx = &ctx;
> +
> +retry:
> + to_intel_atomic_state(state)->modeset = true;
> + to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> + enable ? 2 * 96000 : 0;
> +
> + /*
> + * Protects dev_priv->cdclk.force_min_cdclk
> + * Need to lock this here in case we have no active pipes
> + * and thus wouldn't lock it during the commit otherwise.
> + */
> + ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> &ctx);
> + if (!ret)
> + ret = drm_atomic_commit(state);
Ville mentioned a potential dead-lock problem here: a normal modeset
enabling an HDMI/DP output with an audio sink will call the
drm_audio_component_audio_ops::pin_eld_notify() callback with the
connection_mutex already held. This callback in turn could call
drm_audio_component_ops::get_power()/put_power() callbacks and
dead-lock at the above drm_modeset_lock() call.
Looks like that
sound/pci/hda/patch_hdmi.c / intel_pin_eld_notify()->
check_presence_and_report()->
hdmi_present_sense()
would prevent this, but for instance
sound/soc/codecs/hdac_hdmi.c / hdac_hdmi_eld_notify_cb()->
hdac_hdmi_present_sense()->
hdac_hdmi_jack_report()->
snd_soc_jack_report()->
snd_soc_dapm_sync()->
snd_soc_dapm_sync_unlocked()->
dapm_power_widgets()->
dapm_pre_sequence_async()
could call pm_runtime_get_sync() and so I guess also the aops
get_power() hook.
Could someone in your team check if the above can indeed happen?
> +
> + if (ret == -EDEADLK) {
> + drm_atomic_state_clear(state);
> + drm_modeset_backoff(&ctx);
> + goto retry;
> + }
> +
> + WARN_ON(ret);
> +
> + drm_atomic_state_put(state);
> +
> + drm_modeset_drop_locks(&ctx);
> + drm_modeset_acquire_fini(&ctx);
> +}
> +
> static void i915_audio_component_get_power(struct device *kdev)
> {
> - intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> + struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +
> + dev_priv->get_put_refcount++;
> +
> + /* Force cdclk to 2*BCLK during first time get power call */
> + if (dev_priv->get_put_refcount == 1)
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_pri
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
+Takashi
On Wed, Sep 12, 2018 at 04:18:12PM +0300, Imre Deak wrote:
> Hi Kumar, Takashi,
>
> On Tue, Jun 19, 2018 at 03:01:11PM -0700, Abhay Kumar wrote:
> > From: Ville Syrjälä
> >
> > CDCLK has to be at least twice the BLCK regardless of audio. Audio
> > driver has to probe using this hook and increase the clock even in
> > absence of any display.
> >
> > v2: Use atomic refcount for get_power, put_power so that we can
> > call each once(Abhay).
> > v3: Reset power well 2 to avoid any transaction on iDisp link
> > during cdclk change(Abhay).
> > v4: Remove Power well 2 reset workaround(Ville).
> > v5: Remove unwanted Power well 2 register defined in v4(Abhay).
> >
> > Signed-off-by: Ville Syrjälä
> > Signed-off-by: Abhay Kumar
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 3 ++
> > drivers/gpu/drm/i915/intel_audio.c | 67
> > +---
> > drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
> > drivers/gpu/drm/i915/intel_display.c | 7 +++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > 5 files changed, 83 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 6104d7115054..a4a386a5db69 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1702,6 +1702,7 @@ struct drm_i915_private {
> > unsigned int hpll_freq;
> > unsigned int fdi_pll_freq;
> > unsigned int czclk_freq;
> > + u32 get_put_refcount;
> >
> > struct {
> > /*
> > @@ -1719,6 +1720,8 @@ struct drm_i915_private {
> > struct intel_cdclk_state actual;
> > /* The current hardware cdclk state */
> > struct intel_cdclk_state hw;
> > +
> > + int force_min_cdclk;
> > } cdclk;
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 3ea566f99450..ca8f04c7cbb3 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder
> > *encoder,
> >
> > if (!connector->eld[0])
> > return;
> > -
> > DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> > connector->base.id,
> > connector->name,
> > @@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
> > *dev_priv)
> > }
> > }
> >
> > +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> > + bool enable)
> > +{
> > + struct drm_modeset_acquire_ctx ctx;
> > + struct drm_atomic_state *state;
> > + int ret;
> > +
> > + drm_modeset_acquire_init(&ctx, 0);
> > + state = drm_atomic_state_alloc(&dev_priv->drm);
> > + if (WARN_ON(!state))
> > + return;
> > +
> > + state->acquire_ctx = &ctx;
> > +
> > +retry:
> > + to_intel_atomic_state(state)->modeset = true;
> > + to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> > + enable ? 2 * 96000 : 0;
> > +
> > + /*
> > +* Protects dev_priv->cdclk.force_min_cdclk
> > +* Need to lock this here in case we have no active pipes
> > +* and thus wouldn't lock it during the commit otherwise.
> > +*/
> > + ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> > &ctx);
> > + if (!ret)
> > + ret = drm_atomic_commit(state);
>
> Ville mentioned a potential dead-lock problem here: a normal modeset
> enabling an HDMI/DP output with an audio sink will call the
> drm_audio_component_audio_ops::pin_eld_notify() callback with the
> connection_mutex already held. This callback in turn could call
> drm_audio_component_ops::get_power()/put_power() callbacks and
> dead-lock at the above drm_modeset_lock() call.
>
> Looks like that
>
>sound/pci/hda/patch_hdmi.c / intel_pin_eld_notify()->
>check_presence_and_report()->
>hdmi_present_sense()
>
> would prevent this, but for instance
>
>sound/soc/codecs/hdac_hdmi.c / hdac_hdmi_eld_notify_cb()->
>hdac_hdmi_present_sense()->
>hdac_hdmi_jack_report()->
>snd_soc_jack_report()->
>snd_soc_dapm_sync()->
>snd_soc_dapm_sync_unlocked()->
>dapm_power_widgets()->
>dapm_pre_sequence_async()
>
> could call pm_runtime_get_sync() and so I guess also the aops
> get_power() hook.
>
> Could someone in your team check if the above can indeed happen?
>
> > +
> > + if (ret == -EDEADLK) {
> > + drm_atomic_state_clear(state);
> > + drm_modeset_backoff(&ctx);
> > + goto retry;
> > + }
> > +
> > + WARN_ON(ret);
> > +
> > + drm_atomic_state_put(state);
> > +
> > + drm_modeset_drop_locks(&ctx);
> > + drm_modeset_acquire_fini(&ctx);
> > +}
> > +
> > static void i915_audio_component_get_power(struct device *kdev)
> > {
> > - intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> > + struct drm_i915_priv
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/icl: Enable Y210, Y212, Y216 format for primary and sprite planes
On 12.09.2018 13:32, Swati Sharma wrote:
From: Vidya Srinivas
In this patch, a list for icl specific pixel formats is created
in which Y210, Y212 and Y216 pixel formats are added along with
legacy pixel formats for primary and sprite plane.
Signed-off-by: Swati Sharma
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/intel_display.c | 25 +++--
drivers/gpu/drm/i915/intel_sprite.c | 22 --
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7c68a0d..f341cbd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -105,6 +105,24 @@
DRM_FORMAT_NV12,
};
+static const uint32_t icl_primary_formats[] = {
+ DRM_FORMAT_C8,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_Y210,
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_Y216,
+};
+
static const uint64_t skl_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -13725,8 +13743,11 @@ bool skl_plane_has_planar(struct drm_i915_private
*dev_priv,
if (INTEL_GEN(dev_priv) >= 9) {
primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
PLANE_PRIMARY);
-
- if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ intel_primary_formats = icl_primary_formats;
+ num_formats = ARRAY_SIZE(icl_primary_formats);
+ } else if (skl_plane_has_planar(dev_priv, pipe,
+ PLANE_PRIMARY)) {
Above doesn't look right. I think it could be written "if(gen>=11) {...}
else if(gen>=9){...". Now it looks support for planar formats on ICL is
totally skipped. As is inside skl_plane_has_planar(..) there's check if
running on ICL no planar support is available but it is only until
planar support for ICL is patched in. I suspect you are going to need
icl_primary_formats[] as well as icl_pri_planar_formats[] in similar way
as on SKL. It is similar issue below with changes for intel_sprite.c
/Juha-Pekka
intel_primary_formats = skl_pri_planar_formats;
num_formats = ARRAY_SIZE(skl_pri_planar_formats);
} else {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index f7e2768..2e61fe19 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1281,6 +1281,21 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device
*dev, void *data,
DRM_FORMAT_NV12,
};
+static uint32_t icl_plane_formats[] = {
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
+ DRM_FORMAT_Y210,
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_Y216,
+};
+
static const uint64_t skl_plane_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -1536,8 +1551,11 @@ struct intel_plane *
intel_plane->disable_plane = skl_disable_plane;
intel_plane->get_hw_state = skl_plane_get_hw_state;
- if (skl_plane_has_planar(dev_priv, pipe,
-PLANE_SPRITE0 + plane)) {
+ if (INTEL_GEN(dev_priv) >= 11) {
+ plane_formats = icl_plane_formats;
+ num_plane_formats = ARRAY_SIZE(icl_plane_formats);
+ } else if (skl_plane_has_planar(dev_priv, pipe,
+ PLANE_SPRITE0 + plane)) {
plane_formats = skl_planar_formats;
num_plane_formats = ARRAY_SIZE(skl_planar_formats);
} else {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Add XYUV format support (rev6)
== Series Details == Series: Add XYUV format support (rev6) URL : https://patchwork.freedesktop.org/series/48007/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4810 -> Patchwork_10154 = == Summary - WARNING == Minor unknown changes coming with Patchwork_10154 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10154, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/48007/revisions/6/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10154: === IGT changes === Warnings igt@pm_rpm@module-reload: fi-hsw-4770r: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_10154 that come from known issues: === IGT changes === Possible fixes igt@drv_hangman@error-state-basic: fi-kbl-7560u: INCOMPLETE -> PASS igt@drv_module_reload@basic-reload-inject: fi-hsw-4770r: DMESG-WARN (fdo#107425) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425 == Participating hosts (50 -> 45) == Additional (1): fi-glk-j4005 Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4810 -> Patchwork_10154 CI_DRM_4810: 4c0e813835d47e6e9285175e7b92e21fe3d4328f @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10154: 9490ef9a07aee695e0e1360495c2384ea417ed64 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 9490ef9a07ae drm/i915: Adding YUV444 packed format support for skl+ b0a4446bc5d1 drm: Introduce new DRM_FORMAT_XYUV == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10154/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
On 12/09/2018 12:11, Chris Wilson wrote: If we try and fail to allocate a i915_request, we apply some backpressure on the clients to throttle the memory allocations coming from i915.ko. Currently, we wait until completely idle, but this is far too heavy and leads to some situations where the only escape is to declare a client hung and reset the GPU. The intent is to only ratelimit the allocation requests, so we need only wait for a jiffie before using the normal direct reclaim. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 09ed48833b54..588bc5a4d18b 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED | I915_WAIT_INTERRUPTIBLE, -MAX_SCHEDULE_TIMEOUT); +1); if (ret) goto err_unreserve; What is the remaining value of even trying to wait for idle, instead of maybe just i915_request_retire and sleep for a jiffie? The intention would potentially read clearer since it is questionable there is any relationship with idle and rate limiting clients. In fact, now that I think of it, waiting for idle is a nice way to starve an unlucky client forever. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
Quoting Tvrtko Ursulin (2018-09-12 14:34:16) > > On 12/09/2018 12:11, Chris Wilson wrote: > > If we try and fail to allocate a i915_request, we apply some > > backpressure on the clients to throttle the memory allocations coming > > from i915.ko. Currently, we wait until completely idle, but this is far > > too heavy and leads to some situations where the only escape is to > > declare a client hung and reset the GPU. The intent is to only ratelimit > > the allocation requests, so we need only wait for a jiffie before using > > the normal direct reclaim. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > Cc: Joonas Lahtinen > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index 09ed48833b54..588bc5a4d18b 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, > > struct i915_gem_context *ctx) > > ret = i915_gem_wait_for_idle(i915, > >I915_WAIT_LOCKED | > >I915_WAIT_INTERRUPTIBLE, > > - MAX_SCHEDULE_TIMEOUT); > > + 1); > > if (ret) > > goto err_unreserve; > > > > > > What is the remaining value of even trying to wait for idle, instead of > maybe just i915_request_retire and sleep for a jiffie? There's no point in the wait if actually idle? We just want the retire and kicking of slabs. > The intention > would potentially read clearer since it is questionable there is any > relationship with idle and rate limiting clients. In fact, now that I > think of it, waiting for idle is a nice way to starve an unlucky client > forever. See i915_gem_shrink. It's a seriously moot point. You can define any pathology you'd like, someone has to lose. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
Quoting Tvrtko Ursulin (2018-09-12 14:34:16) > > On 12/09/2018 12:11, Chris Wilson wrote: > > If we try and fail to allocate a i915_request, we apply some > > backpressure on the clients to throttle the memory allocations coming > > from i915.ko. Currently, we wait until completely idle, but this is far > > too heavy and leads to some situations where the only escape is to > > declare a client hung and reset the GPU. The intent is to only ratelimit > > the allocation requests, so we need only wait for a jiffie before using > > the normal direct reclaim. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > Cc: Joonas Lahtinen > > --- > > drivers/gpu/drm/i915/i915_request.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c > > b/drivers/gpu/drm/i915/i915_request.c > > index 09ed48833b54..588bc5a4d18b 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, > > struct i915_gem_context *ctx) > > ret = i915_gem_wait_for_idle(i915, > >I915_WAIT_LOCKED | > >I915_WAIT_INTERRUPTIBLE, > > - MAX_SCHEDULE_TIMEOUT); > > + 1); > > if (ret) > > goto err_unreserve; > > > > > > What is the remaining value of even trying to wait for idle, instead of > maybe just i915_request_retire and sleep for a jiffie? The intention > would potentially read clearer since it is questionable there is any > relationship with idle and rate limiting clients. In fact, now that I > think of it, waiting for idle is a nice way to starve an unlucky client > forever. Better to starve the unlucky client, than to allow the entire system to grind to a halt. One caveat to using RCU is that it is our responsibility to apply backpressure as none is applied by the vm. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 02/25] drm/dp: Add DP DSC DPCD receiver capability size define and missing SHIFT
On 9/12/2018 6:25 AM, Manasi Navare wrote: This patch defines the DP DSC receiver capability size that gives total number of DP DSC DPCD registers. This also adds a missing #defines for DP DSC support missed in the commit id (ab6a46ea6842ce "Add DPCD definitions for DP 1.4 DSC feature") v3: * MIN_SLICE_WIDTH = 2560 (Anusha) * Define DP_DSC_SLICE_WIDTH_MULTIPLIER = 320 v2: * Add SHIFT define and DECOMPRESSION_EN define missed in prev patch Cc: dri-de...@lists.freedesktop.org Cc: Jani Nikula Cc: Ville Syrjala Cc: Anusha Srivatsa Cc: Gaurav K Singh Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa The patch looks good to me. Reviewed-by: Gaurav K Singh --- include/drm/drm_dp_helper.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 698082a02b97..7f6237cad10d 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -231,6 +231,8 @@ #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 # define DP_DSC_RGB (1 << 0) @@ -279,6 +281,8 @@ # define DP_DSC_THROUGHPUT_MODE_1_1000 (14 << 4) #define DP_DSC_MAX_SLICE_WIDTH 0x06C +#define DP_DSC_MIN_SLICE_WIDTH_VALUE2560 +#define DP_DSC_SLICE_WIDTH_MULTIPLIER 320 #define DP_DSC_SLICE_CAP_2 0x06D # define DP_DSC_16_PER_DP_DSC_SINK (1 << 0) @@ -477,6 +481,7 @@ # define DP_AUX_FRAME_SYNC_VALID (1 << 0) #define DP_DSC_ENABLE 0x160 /* DP 1.4 */ +# define DP_DECOMPRESSION_EN(1 << 0) #define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ # define DP_PSR_ENABLE(1 << 0) @@ -963,6 +968,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI #define DP_BRANCH_OUI_HEADER_SIZE 0xc #define DP_RECEIVER_CAP_SIZE 0xf +#define DP_DSC_RECEIVER_CAP_SIZE0xf #define EDP_PSR_RECEIVER_CAP_SIZE 2 #define EDP_DISPLAY_CTL_CAP_SIZE 3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATH i-g-t] igt: Test tagging support
On 12/09/2018 10:07, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-12 09:48:00) On 07/09/2018 12:43, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-07 12:14:20) From: Tvrtko Ursulin Proposal to add test tags as a replacement for separate test list which can be difficult to maintain and get out of date. Putting this maintanenace inline with tests makes it easier to remember to update the (now implicit) lists, and also enables richer test selection possibilities for the test runner. Current method of implying tags from test/subtest names has a couple of problems one of which is where some names can use the same token for different meanings. (One example is the "default" token.) It also creates a name clash between naming and tagging. Test tags are strings of tokens separated by spaces, meaning of which we decide by creating a convetion and helped by the suitable helper macros. For example tags can be things like: gem, kms, basic, guc, flip, semaphore, bz12345678, gt4e, reset, etc.. At runtime this would look something like this (abbreviated for readability): @ tests/gem_sync --list-subtests-with-tags ... forked-each TAGS="gem " forked-store-each TAGS="gem " basic-all TAGS="gem basic " basic-store-all TAGS="gem basic " @ tests/gem_concurrent_blit --list-subtests-with-tags ... 16MiB-swap-gpuX-render-write-read-bcs-bomb TAGS="gem stress " 16MiB-swap-gpuX-render-write-read-rcs-bomb TAGS="gem stress " 16MiB-swap-gpuX-render-gpu-read-after-write-bomb TAGS="gem stress " @ tests/kms_flip --list-subtests-with-tags | grep basic basic-plain-flip TAGS="kms basic " basic-flip-vs-dpms TAGS="kms basic " Test runner could then enable usages like: ./run-tests --include gem --exclude stress Minor comment, I like some basic boolean algebra here --include "gem AND smoketest NOT stress" :) That's what my hypothetical examples showed just with a different syntax! I'd probably tag everything according to which ioctls they use. I feel my endgoal is still to list tests by their kernel functions (which can and should be automatically derived), and their hw function which is the open problem. If we want to do that automatically then tagging in this flavour probably doesn't make sense and it should instead be an external database. If we go on the ioctl granularity it can probably mostly have one version, and it should be static enough to be able to live in the tree, but if we go more granular, like something derived from kcov, then that's out of the window. Since it will be tied both to a platform and kernel version. So I think tagging as proposed here is not appropriate for either, but only if we wanted to tag on coarser functional areas and such. Yeah, I think the same as well, that tags should be "smoke", "stress". (But one man's stress is another's minimal workload :( But both of those are in essence a duration parameter, and I'd prefer if we expressed those as configurable parameters. At which point there is a meta level of test scripts to tag ;) I feel that "gem", "kms" is better expressed in lower level terms, so what's left to tag? Clients? "display", "opencl", "media", "opengl" ? Even using client specs for features (e.g. EGL_IMG_context_priority)? If we overcomplicate and try to change too much at once it is deemed to fail. GEM, KMS, etc, have this advantage of being very established. So I was thinking these basic keywords and then something high level like reset, rps, stolen, and similar. So for instance we can remove the igt_skip_on_simulation, which can be time consuming to answer why it is there, and replace it with --exclude reset,... when running under the simulator. True it is manual work to keep the tags up to date. Is it more work than the current scheme is TBD. Who and why would use those? From a selfish perspective, I want lcov with tests sorted in order of increasing "stress" (start with the precise X does Y test, finish with can it survive pathological client behaviour for 48 hours). Who and why would use which ones? Tags in general, tags in the style the patch proposes, or tags in the flavour you described? My initial idea was that it would make easy for developers to run an approximate sweep when touching a feature. But with good CI and trybot maybe no one would use tags and just rely on "outsourced" test runs? Could be.. but hey, you convinced me to re-send this. Or was it Joonas? :) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On Wed, Sep 12, 2018 at 04:18:12PM +0300, Imre Deak wrote:
> Hi Kumar, Takashi,
>
> On Tue, Jun 19, 2018 at 03:01:11PM -0700, Abhay Kumar wrote:
> > From: Ville Syrjälä
> >
> > CDCLK has to be at least twice the BLCK regardless of audio. Audio
> > driver has to probe using this hook and increase the clock even in
> > absence of any display.
> >
> > v2: Use atomic refcount for get_power, put_power so that we can
> > call each once(Abhay).
> > v3: Reset power well 2 to avoid any transaction on iDisp link
> > during cdclk change(Abhay).
> > v4: Remove Power well 2 reset workaround(Ville).
> > v5: Remove unwanted Power well 2 register defined in v4(Abhay).
> >
> > Signed-off-by: Ville Syrjälä
> > Signed-off-by: Abhay Kumar
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 3 ++
> > drivers/gpu/drm/i915/intel_audio.c | 67
> > +---
> > drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
> > drivers/gpu/drm/i915/intel_display.c | 7 +++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > 5 files changed, 83 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 6104d7115054..a4a386a5db69 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1702,6 +1702,7 @@ struct drm_i915_private {
> > unsigned int hpll_freq;
> > unsigned int fdi_pll_freq;
> > unsigned int czclk_freq;
> > + u32 get_put_refcount;
> >
> > struct {
> > /*
> > @@ -1719,6 +1720,8 @@ struct drm_i915_private {
> > struct intel_cdclk_state actual;
> > /* The current hardware cdclk state */
> > struct intel_cdclk_state hw;
> > +
> > + int force_min_cdclk;
> > } cdclk;
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 3ea566f99450..ca8f04c7cbb3 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder
> > *encoder,
> >
> > if (!connector->eld[0])
> > return;
> > -
> > DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> > connector->base.id,
> > connector->name,
> > @@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
> > *dev_priv)
> > }
> > }
> >
> > +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> > + bool enable)
> > +{
> > + struct drm_modeset_acquire_ctx ctx;
> > + struct drm_atomic_state *state;
> > + int ret;
> > +
> > + drm_modeset_acquire_init(&ctx, 0);
> > + state = drm_atomic_state_alloc(&dev_priv->drm);
> > + if (WARN_ON(!state))
> > + return;
> > +
> > + state->acquire_ctx = &ctx;
> > +
> > +retry:
> > + to_intel_atomic_state(state)->modeset = true;
> > + to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> > + enable ? 2 * 96000 : 0;
> > +
> > + /*
> > +* Protects dev_priv->cdclk.force_min_cdclk
> > +* Need to lock this here in case we have no active pipes
> > +* and thus wouldn't lock it during the commit otherwise.
> > +*/
> > + ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> > &ctx);
> > + if (!ret)
> > + ret = drm_atomic_commit(state);
>
> Ville mentioned a potential dead-lock problem here: a normal modeset
> enabling an HDMI/DP output with an audio sink will call the
> drm_audio_component_audio_ops::pin_eld_notify() callback with the
> connection_mutex already held. This callback in turn could call
> drm_audio_component_ops::get_power()/put_power() callbacks and
> dead-lock at the above drm_modeset_lock() call.
>
> Looks like that
>
>sound/pci/hda/patch_hdmi.c / intel_pin_eld_notify()->
>check_presence_and_report()->
>hdmi_present_sense()
>
> would prevent this, but for instance
>
>sound/soc/codecs/hdac_hdmi.c / hdac_hdmi_eld_notify_cb()->
>hdac_hdmi_present_sense()->
>hdac_hdmi_jack_report()->
>snd_soc_jack_report()->
>snd_soc_dapm_sync()->
>snd_soc_dapm_sync_unlocked()->
>dapm_power_widgets()->
>dapm_pre_sequence_async()
>
> could call pm_runtime_get_sync() and so I guess also the aops
> get_power() hook.
>
> Could someone in your team check if the above can indeed happen?
>
> > +
> > + if (ret == -EDEADLK) {
> > + drm_atomic_state_clear(state);
> > + drm_modeset_backoff(&ctx);
> > + goto retry;
> > + }
> > +
> > + WARN_ON(ret);
> > +
> > + drm_atomic_state_put(state);
> > +
> > + drm_modeset_drop_locks(&ctx);
> > + drm_modeset_acquire_fini(&ctx);
> > +}
> > +
> > static void i915_audio_component_get_power(struct device *kdev)
> > {
> > - intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> > + struct drm_i915_private *dev_p
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
On 12/09/2018 14:38, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-09-12 14:34:16) On 12/09/2018 12:11, Chris Wilson wrote: If we try and fail to allocate a i915_request, we apply some backpressure on the clients to throttle the memory allocations coming from i915.ko. Currently, we wait until completely idle, but this is far too heavy and leads to some situations where the only escape is to declare a client hung and reset the GPU. The intent is to only ratelimit the allocation requests, so we need only wait for a jiffie before using the normal direct reclaim. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 09ed48833b54..588bc5a4d18b 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED | I915_WAIT_INTERRUPTIBLE, - MAX_SCHEDULE_TIMEOUT); + 1); if (ret) goto err_unreserve; What is the remaining value of even trying to wait for idle, instead of maybe just i915_request_retire and sleep for a jiffie? There's no point in the wait if actually idle? We just want the retire and kicking of slabs. Why there is no point? If allocation failed and we want to apply backpressure, lets apply it. Why would it matter which client first spotted GPU is idle and skipped the sleep? What would happen if we added a random back-off in this case between some bounds, like Ethernet frames if my memory serves me well. Don't get me wrong, in practice it is minor point, I am purely thinking that the message presence of i915_gem_wait_for_idle is maybe not clear or is misleading. Regards, Tvrtko The intention would potentially read clearer since it is questionable there is any relationship with idle and rate limiting clients. In fact, now that I think of it, waiting for idle is a nice way to starve an unlucky client forever. See i915_gem_shrink. It's a seriously moot point. You can define any pathology you'd like, someone has to lose. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [STABLE v4.18 BACKPORT] drm/i915: set DP Main Stream Attribute for color range on DDI platforms
commit 6209c285e7a5e68dbcdf8fd2456c6dd68433806b upstream.
Since Haswell we have no color range indication either in the pipe or
port registers for DP. Instead, there's a separate register for setting
the DP Main Stream Attributes (MSA) directly. The MSA register
definition makes no references to colorimetry, just a vague reference to
the DP spec. The connection to the color range was lost.
Apparently we've failed to set the proper MSA bit for limited, or CEA,
range ever since the first DDI platforms. We've started setting other
MSA parameters since commit dae847991a43 ("drm/i915: add
intel_ddi_set_pipe_settings").
Without the crucial bit of information, the DP sink has no way of
knowing the source is actually transmitting limited range RGB, leading
to "washed out" colors. With the colorimetry information, compliant
sinks should be able to handle the limited range properly. Native
(i.e. non-LSPCON) HDMI was not affected because we do pass the color
range via AVI infoframes.
Though not the root cause, the problem was made worse for DDI platforms
with commit 55bc60db5988 ("drm/i915: Add "Automatic" mode for the
"Broadcast RGB" property"), which selects limited range RGB
automatically based on the mode, as per the DP, HDMI and CEA specs.
After all these years, the fix boils down to flipping one bit.
[Per testing reports, this fixes DP sinks, but not the LSPCON. My
educated guess is that the LSPCON fails to turn the CEA range MSA into
AVI infoframes for HDMI.]
Reported-by: Michał Kopeć
Reported-by: N. W.
Reported-by: Nicholas Stommel
Reported-by: Tom Yan
Tested-by: Nicholas Stommel
References: https://bugs.freedesktop.org/show_bug.cgi?id=100023
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107476
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94921
Cc: Paulo Zanoni
Cc: Rodrigo Vivi
Cc: Ville Syrjälä
Cc: # v3.9+
Reviewed-by: Rodrigo Vivi
Signed-off-by: Jani Nikula
Link:
https://patchwork.freedesktop.org/patch/msgid/20180814060001.18224-1-jani.nik...@intel.com
(cherry picked from commit dc5977da99ea28094b8fa4e9bacbd29bedc41de5)
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ddi.c | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7720569f2024..6e048ee88e3f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8825,6 +8825,7 @@ enum skl_power_gate {
#define TRANS_MSA_10_BPC (2<<5)
#define TRANS_MSA_12_BPC (3<<5)
#define TRANS_MSA_16_BPC (4<<5)
+#define TRANS_MSA_CEA_RANGE (1<<3)
/* LCPLL Control */
#define LCPLL_CTL _MMIO(0x130040)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fed26d6e4e27..e195c287c263 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1659,6 +1659,10 @@ void intel_ddi_set_pipe_settings(const struct
intel_crtc_state *crtc_state)
WARN_ON(transcoder_is_dsi(cpu_transcoder));
temp = TRANS_MSA_SYNC_CLK;
+
+ if (crtc_state->limited_color_range)
+ temp |= TRANS_MSA_CEA_RANGE;
+
switch (crtc_state->pipe_bpp) {
case 18:
temp |= TRANS_MSA_6_BPC;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: set DP Main Stream Attribute for color range on DDI platforms (rev2)
== Series Details == Series: drm/i915: set DP Main Stream Attribute for color range on DDI platforms (rev2) URL : https://patchwork.freedesktop.org/series/48145/ State : failure == Summary == Applying: drm/i915: set DP Main Stream Attribute for color range on DDI platforms Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/i915_reg.h M drivers/gpu/drm/i915/intel_ddi.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/i915_reg.h CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_reg.h error: Failed to merge in the changes. Patch failed at 0001 drm/i915: set DP Main Stream Attribute for color range on DDI platforms Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 04/25] drm/dp: DRM DP helper/macros to get DP sink DSC parameters
On 9/12/2018 6:25 AM, Manasi Navare wrote:
This patch adds inline functions and helpers for obtaining
DP sink's supported DSC parameters like DSC sink support,
eDP compressed BPP supported, maximum slice count supported
by the sink devices, DSC line buffer bit depth supported on DP sink,
DSC sink maximum color depth by parsing corresponding DPCD registers.
v4:
* Add helper to give line buf bit depth (Manasi)
* Correct the bit masking in color depth helper (manasi)
v3:
* Use SLICE_CAP_2 for DP (Anusha)
v2:
* Add DSC sink support macro (Jani N)
Cc: Gaurav K Singh
Cc: dri-de...@lists.freedesktop.org
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
This patch looks good to me.
Reviewed-by: Gaurav K Singh
---
drivers/gpu/drm/drm_dp_helper.c | 90 +
include/drm/drm_dp_helper.h | 30 +++
2 files changed, 120 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 8c6b9fd89f8a..5d5879f115ce 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1337,3 +1337,93 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct
drm_dp_desc *desc,
return 0;
}
EXPORT_SYMBOL(drm_dp_read_desc);
+
+/**
+ * DRM DP Helpers for DSC
+ */
+u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
+ bool is_edp)
+{
+ u8 slice_cap1 = dsc_dpcd[DP_DSC_SLICE_CAP_1 - DP_DSC_SUPPORT];
+
+ if (is_edp) {
+ /* For eDP, register DSC_SLICE_CAPABILITIES_1 gives slice count
*/
+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK)
+ return 4;
+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK)
+ return 2;
+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK)
+ return 1;
+ } else {
+ /* For DP, use values from DSC_SLICE_CAP_1 and DSC_SLICE_CAP2 */
+ u8 slice_cap2 = dsc_dpcd[DP_DSC_SLICE_CAP_2 - DP_DSC_SUPPORT];
+
+ if (slice_cap2 & DP_DSC_24_PER_DP_DSC_SINK)
+ return 24;
+ if (slice_cap2 & DP_DSC_20_PER_DP_DSC_SINK)
+ return 20;
+ if (slice_cap2 & DP_DSC_16_PER_DP_DSC_SINK)
+ return 16;
+ if (slice_cap1 & DP_DSC_12_PER_DP_DSC_SINK)
+ return 12;
+ if (slice_cap1 & DP_DSC_10_PER_DP_DSC_SINK)
+ return 10;
+ if (slice_cap1 & DP_DSC_8_PER_DP_DSC_SINK)
+ return 8;
+ if (slice_cap1 & DP_DSC_6_PER_DP_DSC_SINK)
+ return 6;
+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK)
+ return 4;
+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK)
+ return 2;
+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK)
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count);
+
+u8 drm_dp_dsc_sink_line_buf_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH -
DP_DSC_SUPPORT];
+
+ switch (line_buf_depth & DP_DSC_LINE_BUF_BIT_DEPTH_MASK) {
+ case DP_DSC_LINE_BUF_BIT_DEPTH_9:
+ return 9;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_10:
+ return 10;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_11:
+ return 11;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_12:
+ return 12;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_13:
+ return 13;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_14:
+ return 14;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_15:
+ return 15;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_16:
+ return 16;
+ case DP_DSC_LINE_BUF_BIT_DEPTH_8:
+ return 8;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth);
+
+u8 drm_dp_dsc_sink_max_color_depth(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - DP_DSC_SUPPORT];
+
+ if (color_depth & DP_DSC_12_BPC)
+ return 12;
+ if (color_depth & DP_DSC_10_BPC)
+ return 10;
+ if (color_depth & DP_DSC_8_BPC)
+ return 8;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 7f6237cad10d..ce6297908fd6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1065,6 +1065,36 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
return dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT;
}
+/* DP/eDP DSC support */
+u8 drm_dp_dsc_sink_max_slice_count(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
+
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
== Series Details == Series: drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G URL : https://patchwork.freedesktop.org/series/49548/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809_full -> Patchwork_10150_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10150_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10150_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10150_full: === IGT changes === Warnings igt@kms_cursor_legacy@cursora-vs-flipa-atomic: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10150_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_schedule@preempt-other-chain-vebox: shard-snb: SKIP -> INCOMPLETE (fdo#105411) igt@kms_busy@basic-flip-b: shard-kbl: PASS -> DMESG-WARN (fdo#103313, fdo#103558, fdo#105602) +1 igt@kms_chv_cursor_fail@pipe-b-64x64-bottom-edge: shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +12 igt@kms_plane_scaling@pipe-a-scaler-with-rotation: shard-kbl: PASS -> FAIL (fdo#106026) igt@pm_rpm@system-suspend: shard-kbl: PASS -> DMESG-WARN (fdo#103313) igt@pm_rpm@universal-planes: shard-kbl: PASS -> DMESG-FAIL (fdo#103313, fdo#103558, fdo#105602) Possible fixes igt@drv_suspend@shrink: shard-kbl: INCOMPLETE (fdo#106886, fdo#103665) -> PASS igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS shard-kbl: FAIL (fdo#99912) -> PASS igt@perf@blocking: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106026 https://bugs.freedesktop.org/show_bug.cgi?id=106026 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10150 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10150: 6120c5292b7c906538cba66cf5e0e33f16dd49ec @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10150/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Restrict wait to client's timeline on i915_request alloc failure
Let's try not to overly penalize the unlucky client by making them wait
for others to complete their work, and only apply the ratelimit if they
themselves have outstanding work. Still, we apply global reclaim to the
client (we need to scavenge some memory for it) so it doesn't got away
completely scot free.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_request.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 588bc5a4d18b..4866de572d73 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -733,12 +733,21 @@ i915_request_alloc(struct intel_engine_cs *engine, struct
i915_gem_context *ctx)
GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
if (unlikely(!rq)) {
/* Ratelimit ourselves to prevent oom from malicious clients */
- ret = i915_gem_wait_for_idle(i915,
-I915_WAIT_LOCKED |
-I915_WAIT_INTERRUPTIBLE,
-1);
- if (ret)
- goto err_unreserve;
+ rq = i915_gem_active_raw(&ce->ring->timeline->last_request,
+&i915->drm.struct_mutex);
+ if (rq) {
+ long timeout;
+
+ timeout = i915_request_wait(rq,
+ I915_WAIT_LOCKED |
+ I915_WAIT_INTERRUPTIBLE,
+ 1);
+ if (timeout < 0) {
+ ret = timeout;
+ goto err_unreserve;
+ }
+ }
+ i915_retire_requests(i915);
/*
* We've forced the client to stall and catch up with whatever
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Limit the backpressure for i915_request allocation
If we try and fail to allocate a i915_request, we apply some backpressure on the clients to throttle the memory allocations coming from i915.ko. Currently, we wait until completely idle, but this is far too heavy and leads to some situations where the only escape is to declare a client hung and reset the GPU. The intent is to only ratelimit the allocation requests, so we need only wait for a jiffie before using the normal direct reclaim. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 09ed48833b54..588bc5a4d18b 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED | I915_WAIT_INTERRUPTIBLE, -MAX_SCHEDULE_TIMEOUT); +1); if (ret) goto err_unreserve; -- 2.19.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
On Wed, Sep 12, 2018 at 3:42 PM, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-09-12 14:34:16) >> >> On 12/09/2018 12:11, Chris Wilson wrote: >> > If we try and fail to allocate a i915_request, we apply some >> > backpressure on the clients to throttle the memory allocations coming >> > from i915.ko. Currently, we wait until completely idle, but this is far >> > too heavy and leads to some situations where the only escape is to >> > declare a client hung and reset the GPU. The intent is to only ratelimit >> > the allocation requests, so we need only wait for a jiffie before using >> > the normal direct reclaim. >> > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 >> > Signed-off-by: Chris Wilson >> > Cc: Tvrtko Ursulin >> > Cc: Joonas Lahtinen >> > --- >> > drivers/gpu/drm/i915/i915_request.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c >> > b/drivers/gpu/drm/i915/i915_request.c >> > index 09ed48833b54..588bc5a4d18b 100644 >> > --- a/drivers/gpu/drm/i915/i915_request.c >> > +++ b/drivers/gpu/drm/i915/i915_request.c >> > @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, >> > struct i915_gem_context *ctx) >> > ret = i915_gem_wait_for_idle(i915, >> >I915_WAIT_LOCKED | >> >I915_WAIT_INTERRUPTIBLE, >> > - MAX_SCHEDULE_TIMEOUT); >> > + 1); >> > if (ret) >> > goto err_unreserve; >> > >> > >> >> What is the remaining value of even trying to wait for idle, instead of >> maybe just i915_request_retire and sleep for a jiffie? The intention >> would potentially read clearer since it is questionable there is any >> relationship with idle and rate limiting clients. In fact, now that I >> think of it, waiting for idle is a nice way to starve an unlucky client >> forever. > > Better to starve the unlucky client, than to allow the entire system to > grind to a halt. > > One caveat to using RCU is that it is our responsibility to apply > backpressure as none is applied by the vm. So instead of 1 jiffie, should we wait for 1 rcu grace period? My understanding is that under very heavy load these can be extended (since batching is more effective for throughput, if you don't run out of memory). Just a random comment from the sidelines really :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the backpressure for i915_request allocation
Quoting Daniel Vetter (2018-09-12 15:47:21) > On Wed, Sep 12, 2018 at 3:42 PM, Chris Wilson > wrote: > > Quoting Tvrtko Ursulin (2018-09-12 14:34:16) > >> > >> On 12/09/2018 12:11, Chris Wilson wrote: > >> > If we try and fail to allocate a i915_request, we apply some > >> > backpressure on the clients to throttle the memory allocations coming > >> > from i915.ko. Currently, we wait until completely idle, but this is far > >> > too heavy and leads to some situations where the only escape is to > >> > declare a client hung and reset the GPU. The intent is to only ratelimit > >> > the allocation requests, so we need only wait for a jiffie before using > >> > the normal direct reclaim. > >> > > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 > >> > Signed-off-by: Chris Wilson > >> > Cc: Tvrtko Ursulin > >> > Cc: Joonas Lahtinen > >> > --- > >> > drivers/gpu/drm/i915/i915_request.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c > >> > b/drivers/gpu/drm/i915/i915_request.c > >> > index 09ed48833b54..588bc5a4d18b 100644 > >> > --- a/drivers/gpu/drm/i915/i915_request.c > >> > +++ b/drivers/gpu/drm/i915/i915_request.c > >> > @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, > >> > struct i915_gem_context *ctx) > >> > ret = i915_gem_wait_for_idle(i915, > >> >I915_WAIT_LOCKED | > >> >I915_WAIT_INTERRUPTIBLE, > >> > - MAX_SCHEDULE_TIMEOUT); > >> > + 1); > >> > if (ret) > >> > goto err_unreserve; > >> > > >> > > >> > >> What is the remaining value of even trying to wait for idle, instead of > >> maybe just i915_request_retire and sleep for a jiffie? The intention > >> would potentially read clearer since it is questionable there is any > >> relationship with idle and rate limiting clients. In fact, now that I > >> think of it, waiting for idle is a nice way to starve an unlucky client > >> forever. > > > > Better to starve the unlucky client, than to allow the entire system to > > grind to a halt. > > > > One caveat to using RCU is that it is our responsibility to apply > > backpressure as none is applied by the vm. > > So instead of 1 jiffie, should we wait for 1 rcu grace period? My > understanding is that under very heavy load these can be extended > (since batching is more effective for throughput, if you don't run out > of memory). Just a random comment from the sidelines really :-) There's (essentially) a wait for that later ;) But a long time ago Paul did write a missive explaining that there should be some use of cond_synchronize_rcu() to provide the rate-limiting, but I've never been sure of a way to plug in the right figures. Do we say if the timeline does more than two RCU allocations within the same grace period it should be throttled? A light allocation failure has always seemed to be a sensible trigger to start worrying. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/dp: fix shifting by a negative number of bits
Function intel_port_to_tc() returns PORT_TC_NONE on error, which is
a negative value -1. In case PORT_TC_NONE is returned, there is an
undefined behavior when shifting by a negative number of bits in
both DP_PHY_MODE_STATUS_NOT_SAFE and P_PHY_MODE_STATUS_COMPLETED
macros.
Fix this by adding sanity checks on intel_port_to_tc return value, before
using macros DP_PHY_MODE_STATUS_NOT_SAFE and P_PHY_MODE_STATUS_COMPLETED.
Addresses-Coverity-ID: 1473324 ("Bad bit shift operation")
Addresses-Coverity-ID: 1473325 ("Bad bit shift operation")
Fixes: 39d1e234e1e1 ("drm/i915/icl: implement the tc/legacy HPD {dis,}connect
flows")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/i915/intel_dp.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22d..e34b7b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4811,6 +4811,11 @@ static bool icl_tc_phy_connect(struct drm_i915_private
*dev_priv,
dig_port->tc_type != TC_PORT_TYPEC)
return true;
+ if (tc_port < 0) {
+ DRM_DEBUG_KMS("Bad TC port %d\n", tc_port);
+ return false;
+ }
+
val = I915_READ(PORT_TX_DFLEXDPPMS);
if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
@@ -4857,6 +4862,10 @@ static void icl_tc_phy_disconnect(struct
drm_i915_private *dev_priv,
dig_port->tc_type != TC_PORT_TYPEC)
return;
+ if (tc_port < 0) {
+ DRM_DEBUG_KMS("Bad TC port %d\n", tc_port);
+ return;
+ }
/*
* This function may be called many times in a row without an HPD event
* in between, so try to avoid the write when we can.
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Restrict wait to client's timeline on i915_request alloc failure
Let's try not to overly penalize the unlucky client by making them wait
for others to complete their work, and only apply the ratelimit if they
themselves have outstanding work. Still, we apply global reclaim to the
client (we need to scavenge some memory for it) so it doesn't got away
completely scot free.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_request.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 588bc5a4d18b..72bcb4ca0c45 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -733,12 +733,16 @@ i915_request_alloc(struct intel_engine_cs *engine, struct
i915_gem_context *ctx)
GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
if (unlikely(!rq)) {
/* Ratelimit ourselves to prevent oom from malicious clients */
- ret = i915_gem_wait_for_idle(i915,
-I915_WAIT_LOCKED |
-I915_WAIT_INTERRUPTIBLE,
-1);
- if (ret)
+ rq = i915_gem_active_raw(&ce->ring->timeline->last_request,
+&i915->drm.struct_mutex);
+ if (rq && i915_request_wait(rq,
+ I915_WAIT_LOCKED |
+ I915_WAIT_INTERRUPTIBLE,
+ 1) == -EINTR) {
+ ret = -EINTR;
goto err_unreserve;
+ }
+ i915_retire_requests(i915);
/*
* We've forced the client to stall and catch up with whatever
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/13] drm/i915: Display gtt remapping prep stuff
On Fri, Sep 07, 2018 at 06:24:00PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > A reposting of all the reviewed prep stuff for the gtt remapping. > > Changes since the first posting: > - fix the ilk+ x-tiled stride to be 32k > - split out some unrelated changes (those were already pushed > separately) > - some typos etc. fixed > > Ville Syrjälä (13): > drm/i915: s/tile_offset/aligned_offset/ etc. > drm/i915: Add .max_stride() plane hook > drm/i915: Use pipe A primary plane .max_stride() as the global stride > limit > drm/i915: Rename the plane_state->main/aux to > plane_state->color_plane[] > drm/i915: Store the final plane stride in plane_state > drm/i915: Store ggtt_view in plane_state > drm/i915: s/int plane/int color_plane/ > drm/i915: Nuke plane->can_scale/min_downscale > drm/i915: Extract per-platform plane->check() functions > drm/i915: Move skl plane fb related checks into a better place > drm/i915: Move display w/a #1175 > drm/i915: Move chv rotation checks to plane->check() > drm/i915: Extract intel_cursor_check_surface() Series pushed to dinq. Thanks for the reviews. > > drivers/gpu/drm/i915/intel_atomic_plane.c | 53 --- > drivers/gpu/drm/i915/intel_display.c | 610 > +++--- > drivers/gpu/drm/i915/intel_drv.h | 45 ++- > drivers/gpu/drm/i915/intel_fbc.c | 4 +- > drivers/gpu/drm/i915/intel_fbdev.c| 6 +- > drivers/gpu/drm/i915/intel_sprite.c | 495 +--- > 6 files changed, 700 insertions(+), 513 deletions(-) > > -- > 2.16.4 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for add LG panel to dpcd quirk database (rev2)
== Series Details == Series: add LG panel to dpcd quirk database (rev2) URL : https://patchwork.freedesktop.org/series/49413/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809_full -> Patchwork_10152_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10152_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@2x-flip-vs-expired-vblank-interruptible: shard-glk: PASS -> FAIL (fdo#105363) Possible fixes igt@drv_suspend@shrink: shard-kbl: INCOMPLETE (fdo#106886, fdo#103665) -> PASS igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@perf@blocking: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10152 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10152: 4947b044fdaf8bac17fdd9c581451e2e0848b9f7 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10152/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dp: fix shifting by a negative number of bits
== Series Details == Series: drm/i915/dp: fix shifting by a negative number of bits URL : https://patchwork.freedesktop.org/series/49565/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4812 -> Patchwork_10156 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49565/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10156 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_guc: fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258) igt@kms_pipe_crc_basic@read-crc-pipe-b: fi-byt-clapper: PASS -> FAIL (fdo#107362) +1 igt@kms_psr@primary_page_flip: fi-kbl-7560u: PASS -> FAIL (fdo#107336) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258 fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 == Participating hosts (49 -> 44) == Additional (2): fi-byt-j1900 fi-skl-guc Missing(7): fi-ilk-m540 fi-bxt-dsi fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4812 -> Patchwork_10156 CI_DRM_4812: 7bed140e1a21dc0e04f2fd87d2726f47b845e82f @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10156: bfad517ac799fbbfa711620c387d61e645ed8e67 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == bfad517ac799 drm/i915/dp: fix shifting by a negative number of bits == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10156/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915/oa: Simplify updating contexts
From: Tvrtko Ursulin
We can remove the update-via-batch-buffer code path, which is basically an
effective duplicate of update-via-context-image path, if we notice that
after we have idled the GPU, we can update the context image even of the
kernel context directly. (Update-via-batch-buffer path existed only to
solve the problem of how to update the kernel context image.)
Only additional thing needed is to activate the edited configuration by
sending one empty request down the pipe. This accomplishes context restore
of the updated kernel context and so the OA configuration gets written out
to it's control registers.
Signed-off-by: Tvrtko Ursulin
Cc: Lionel Landwerlin
Cc: Matthew Auld
Cc: Chris Wilson
---
drivers/gpu/drm/i915/i915_perf.c | 122 ---
1 file changed, 13 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index ccb20230df2c..3d7a052b4cca 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1679,107 +1679,6 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
}
}
-/*
- * Same as gen8_update_reg_state_unlocked only through the batchbuffer. This
- * is only used by the kernel context.
- */
-static int gen8_emit_oa_config(struct i915_request *rq,
- const struct i915_oa_config *oa_config)
-{
- struct drm_i915_private *dev_priv = rq->i915;
- /* The MMIO offsets for Flex EU registers aren't contiguous */
- u32 flex_mmio[] = {
- i915_mmio_reg_offset(EU_PERF_CNTL0),
- i915_mmio_reg_offset(EU_PERF_CNTL1),
- i915_mmio_reg_offset(EU_PERF_CNTL2),
- i915_mmio_reg_offset(EU_PERF_CNTL3),
- i915_mmio_reg_offset(EU_PERF_CNTL4),
- i915_mmio_reg_offset(EU_PERF_CNTL5),
- i915_mmio_reg_offset(EU_PERF_CNTL6),
- };
- u32 *cs;
- int i;
-
- cs = intel_ring_begin(rq, ARRAY_SIZE(flex_mmio) * 2 + 4);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- *cs++ = MI_LOAD_REGISTER_IMM(ARRAY_SIZE(flex_mmio) + 1);
-
- *cs++ = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
- *cs++ = (dev_priv->perf.oa.period_exponent <<
GEN8_OA_TIMER_PERIOD_SHIFT) |
- (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
- GEN8_OA_COUNTER_RESUME;
-
- for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
- u32 mmio = flex_mmio[i];
-
- /*
-* This arbitrary default will select the 'EU FPU0 Pipeline
-* Active' event. In the future it's anticipated that there
-* will be an explicit 'No Event' we can select, but not
-* yet...
-*/
- u32 value = 0;
-
- if (oa_config) {
- u32 j;
-
- for (j = 0; j < oa_config->flex_regs_len; j++) {
- if
(i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) {
- value = oa_config->flex_regs[j].value;
- break;
- }
- }
- }
-
- *cs++ = mmio;
- *cs++ = value;
- }
-
- *cs++ = MI_NOOP;
- intel_ring_advance(rq, cs);
-
- return 0;
-}
-
-static int gen8_switch_to_updated_kernel_context(struct drm_i915_private
*dev_priv,
-const struct i915_oa_config
*oa_config)
-{
- struct intel_engine_cs *engine = dev_priv->engine[RCS];
- struct i915_timeline *timeline;
- struct i915_request *rq;
- int ret;
-
- lockdep_assert_held(&dev_priv->drm.struct_mutex);
-
- i915_retire_requests(dev_priv);
-
- rq = i915_request_alloc(engine, dev_priv->kernel_context);
- if (IS_ERR(rq))
- return PTR_ERR(rq);
-
- ret = gen8_emit_oa_config(rq, oa_config);
- if (ret) {
- i915_request_add(rq);
- return ret;
- }
-
- /* Queue this switch after all other activity */
- list_for_each_entry(timeline, &dev_priv->gt.timelines, link) {
- struct i915_request *prev;
-
- prev = i915_gem_active_raw(&timeline->last_request,
- &dev_priv->drm.struct_mutex);
- if (prev)
- i915_request_await_dma_fence(rq, &prev->fence);
- }
-
- i915_request_add(rq);
-
- return 0;
-}
-
/*
* Manages updating the per-context aspects of the OA stream
* configuration across all contexts.
@@ -1809,16 +1708,11 @@ static int gen8_configure_all_contexts(struct
drm_i915_private *dev_priv,
{
struct intel_engine_cs *engine = dev_priv->engine[RCS];
struct i915_gem_context *ctx;
+ struct i915_request *rq;
Re: [Intel-gfx] [PATCH] i915/oa: Simplify updating contexts
On 12/09/2018 16:29, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
We can remove the update-via-batch-buffer code path, which is basically an
effective duplicate of update-via-context-image path, if we notice that
after we have idled the GPU, we can update the context image even of the
kernel context directly. (Update-via-batch-buffer path existed only to
solve the problem of how to update the kernel context image.)
Only additional thing needed is to activate the edited configuration by
sending one empty request down the pipe. This accomplishes context restore
of the updated kernel context and so the OA configuration gets written out
to it's control registers.
Signed-off-by: Tvrtko Ursulin
Cc: Lionel Landwerlin
Cc: Matthew Auld
Cc: Chris Wilson
After the discussion we had on IRC about context save/restore timings,
this looks good to me :
Reviewed-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_perf.c | 122 ---
1 file changed, 13 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index ccb20230df2c..3d7a052b4cca 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1679,107 +1679,6 @@ static void gen8_update_reg_state_unlocked(struct
i915_gem_context *ctx,
}
}
-/*
- * Same as gen8_update_reg_state_unlocked only through the batchbuffer. This
- * is only used by the kernel context.
- */
-static int gen8_emit_oa_config(struct i915_request *rq,
- const struct i915_oa_config *oa_config)
-{
- struct drm_i915_private *dev_priv = rq->i915;
- /* The MMIO offsets for Flex EU registers aren't contiguous */
- u32 flex_mmio[] = {
- i915_mmio_reg_offset(EU_PERF_CNTL0),
- i915_mmio_reg_offset(EU_PERF_CNTL1),
- i915_mmio_reg_offset(EU_PERF_CNTL2),
- i915_mmio_reg_offset(EU_PERF_CNTL3),
- i915_mmio_reg_offset(EU_PERF_CNTL4),
- i915_mmio_reg_offset(EU_PERF_CNTL5),
- i915_mmio_reg_offset(EU_PERF_CNTL6),
- };
- u32 *cs;
- int i;
-
- cs = intel_ring_begin(rq, ARRAY_SIZE(flex_mmio) * 2 + 4);
- if (IS_ERR(cs))
- return PTR_ERR(cs);
-
- *cs++ = MI_LOAD_REGISTER_IMM(ARRAY_SIZE(flex_mmio) + 1);
-
- *cs++ = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
- *cs++ = (dev_priv->perf.oa.period_exponent <<
GEN8_OA_TIMER_PERIOD_SHIFT) |
- (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
- GEN8_OA_COUNTER_RESUME;
-
- for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
- u32 mmio = flex_mmio[i];
-
- /*
-* This arbitrary default will select the 'EU FPU0 Pipeline
-* Active' event. In the future it's anticipated that there
-* will be an explicit 'No Event' we can select, but not
-* yet...
-*/
- u32 value = 0;
-
- if (oa_config) {
- u32 j;
-
- for (j = 0; j < oa_config->flex_regs_len; j++) {
- if
(i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) {
- value = oa_config->flex_regs[j].value;
- break;
- }
- }
- }
-
- *cs++ = mmio;
- *cs++ = value;
- }
-
- *cs++ = MI_NOOP;
- intel_ring_advance(rq, cs);
-
- return 0;
-}
-
-static int gen8_switch_to_updated_kernel_context(struct drm_i915_private
*dev_priv,
-const struct i915_oa_config
*oa_config)
-{
- struct intel_engine_cs *engine = dev_priv->engine[RCS];
- struct i915_timeline *timeline;
- struct i915_request *rq;
- int ret;
-
- lockdep_assert_held(&dev_priv->drm.struct_mutex);
-
- i915_retire_requests(dev_priv);
-
- rq = i915_request_alloc(engine, dev_priv->kernel_context);
- if (IS_ERR(rq))
- return PTR_ERR(rq);
-
- ret = gen8_emit_oa_config(rq, oa_config);
- if (ret) {
- i915_request_add(rq);
- return ret;
- }
-
- /* Queue this switch after all other activity */
- list_for_each_entry(timeline, &dev_priv->gt.timelines, link) {
- struct i915_request *prev;
-
- prev = i915_gem_active_raw(&timeline->last_request,
- &dev_priv->drm.struct_mutex);
- if (prev)
- i915_request_await_dma_fence(rq, &prev->fence);
- }
-
- i915_request_add(rq);
-
- return 0;
-}
-
/*
* Manages updating the per-context aspects of the OA stream
* configuration across all contexts.
@@ -1809,16 +1708,11 @@ static int gen8_configure_a
Re: [Intel-gfx] [PATCH] i915/oa: Simplify updating contexts
Quoting Tvrtko Ursulin (2018-09-12 16:29:30) > /* > * The OA register config is setup through the context image. This > image > * might be written to by the GPU on context switch (in particular on > @@ -1833,7 +1727,7 @@ static int gen8_configure_all_contexts(struct > drm_i915_private *dev_priv, > * the GPU from any submitted work. > */ > ret = i915_gem_wait_for_idle(dev_priv, > -wait_flags, > +I915_WAIT_LOCKED, > MAX_SCHEDULE_TIMEOUT); Wait until idle includes a wait for the gpu to switch off. At least it does for execlists, not so clear for ringbuffer as there is no explicit idle-event. However, that shouldn't matter as the kernel context doesn't exist for legacy ringbuffer anyway ;) But the reload will be forced on the next actual use. > if (ret) > return ret; > @@ -1859,7 +1753,17 @@ static int gen8_configure_all_contexts(struct > drm_i915_private *dev_priv, > i915_gem_object_unpin_map(ce->state->obj); > } > > - return ret; > + /* > +* Apply the configuration by doing one context restore of the edited > +* context image. > +*/ > + rq = i915_request_alloc(engine, dev_priv->kernel_context); By feeding a request, you ensure the reconfig is loaded again. +1 for having it turn off when idle (and not instrument the kernel context at all)! Still I follow your logic that this should leave the oa config in exactly the same state as before the patch, so Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Limit the backpressure for i915_request allocation
== Series Details == Series: drm/i915: Limit the backpressure for i915_request allocation URL : https://patchwork.freedesktop.org/series/49555/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4809_full -> Patchwork_10153_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_10153_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_await@wide-contexts: shard-apl: PASS -> FAIL (fdo#106680) shard-kbl: PASS -> FAIL (fdo#106680) Possible fixes igt@drv_suspend@shrink: shard-kbl: INCOMPLETE (fdo#103665, fdo#106886) -> PASS igt@gem_exec_big: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@perf@blocking: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4809 -> Patchwork_10153 CI_DRM_4809: 398eca316f0ed73de201afc12c4ab1ef3b2890cb @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10153: 193b2e33a951f696605d282a49529f35bbb19cb1 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10153/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
48 bit ppgtt device configuration is really just extended address
range full ppgtt and may actually be something other than 48 bits.
Change USES_FULL_48BIT_PPGTT() to USES_4LVL_PPGTT() to better
describe that a 4 level walk table extended range PPGTT is being
used. Add a new device info field that specifies the number of
bits to prepare for cases where the range is not 32 or 48 bits.
Also rename other functions and comments from 48bit to 4-level.
v2: keep USES_FULL_PPGTT() unchanged (Chris)
v3: Simplify condition in gen8_ppgtt_create() (Chris)
Remove unnecessary line coninuations (Bob)
Rename functions/defines/comments from 48bit to 4lvl (Rodrigo/Bob)
v4: Rename FULL_4LVL_PPGTT to simply 4LVL_PPGTT (Rodrigo)
Be explised in setting vm.total to 1ULL << 32 (Rodrigo)
Gen 7 is 31 bits, not 32 (Chris)
Signed-off-by: Bob Paauwe
CC: Rodrigo Vivi
CC: Michel Thierry
CC: Chris Wilson
---
Beyond this, there are two follow on patches I'd like to work on:
1) Move parts of gen6_create_ppgtt() and gen8_create_ppgtt() that
are common into a single function. This will make gen6)_create_ppgtt()
use the device_info field for number of address bits.
2) Remove enable_ppgtt module parameter.
drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 24
drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
drivers/gpu/drm/i915/i915_pci.c | 7 +--
drivers/gpu/drm/i915/i915_pvinfo.h | 2 +-
drivers/gpu/drm/i915/i915_vgpu.c | 4 ++--
drivers/gpu/drm/i915/i915_vgpu.h | 2 +-
drivers/gpu/drm/i915/intel_device_info.h | 4 +++-
drivers/gpu/drm/i915/intel_lrc.c | 8
drivers/gpu/drm/i915/selftests/huge_pages.c | 8
drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++
13 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index a4e8e3cf74fd..ab33208e59d6 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -44,7 +44,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
vgpu_vreg_t(vgpu, vgtif_reg(display_ready)) = 0;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_id)) = vgpu->id;
- vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_48BIT_PPGTT;
+ vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_4LVL_PPGTT;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HWSP_EMULATION;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HUGE_GTT;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7ea442033a57..ac86000a1027 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2572,7 +2572,7 @@ intel_info(const struct drm_i915_private *dev_priv)
#define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt)
#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2)
-#define USES_FULL_48BIT_PPGTT(dev_priv)(i915_modparams.enable_ppgtt ==
3)
+#define USES_4LVL_PPGTT(dev_priv) ((dev_priv)->info.full_ppgtt_bits > 32)
#define HAS_PAGE_SIZES(dev_priv, sizes) ({ \
GEM_BUG_ON((sizes) == 0); \
((sizes) & ~(dev_priv)->info.page_sizes) == 0; \
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f772593b99ab..561f8e30ea36 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -307,7 +307,7 @@ static u32 default_desc_template(const struct
drm_i915_private *i915,
desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (ppgtt && i915_vm_is_48bit(&ppgtt->vm))
+ if (ppgtt && i915_vm_is_4lvl(&ppgtt->vm))
address_mode = INTEL_LEGACY_64B_CONTEXT;
desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index eb0e446d6482..561a911d09d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -137,18 +137,18 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private
*dev_priv,
int enable_ppgtt)
{
bool has_full_ppgtt;
- bool has_full_48bit_ppgtt;
+ bool has_full_4lvl_ppgtt;
if (!dev_priv->info.has_aliasing_ppgtt)
return 0;
has_full_ppgtt = dev_priv->info.has_full_ppgtt;
- has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
+ has_full_4lvl_ppgtt = USES_4LVL_PPGTT(dev_priv);
if (intel_vgpu_active(dev_priv)) {
/* GVT-g has no support for 32bit ppgtt */
has_full_ppgtt = false;
- has_full_48bit_p
Re: [Intel-gfx] [PATCH] drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB
On Wed, Sep 12, 2018 at 09:13:07AM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-09-11 17:54:57)
> > From: Ville Syrjälä
> >
> > If we have framebuffers that are >= 4GiB in size we will overflow
> > the fb size check in intel_fill_fb_info().
> >
> > Currently that is only possible with NV12 and CCS as offsets[1]
> > may be anything between 0 and 0x. ofsets[0] is currently
> > required to be 0 so we can't hit the overflow with any single
> > plane format (thanks to max fb size of 8kx8k and max stride of
> > 32 KiB).
> >
> > In the future we may allow almost any framebuffer to exceed 4GiB
> > in size so we really should fix the overflow. Not that the overflow
> > is particularly dangerous. It's mostly just a sanity check against
> > insane userspace. The display engine can't write to memory anyway
> > so I suppose in the worst case we might anger the hw by attempting
> > scanout past the end of the ggtt, or we might scan out some data
> > that we're not supposed to see from other parts of the ggtt.
> >
> > Note that triggering this overflow depends on the driver
> > aligning the fb height to the next tile boundary to push the
> > calculated size above 4GiB. With linear buffers the effective
> > tile height is one so that never happens, and the core already
> > has a check for 32bit overflow of offsets[]+pitches[]*height.
> >
> > Testcase: igt/kms_big_fb/x-tiled-addfb-size-offset-overflow
> > Testcase: igt/kms_big_fb/y-tiled-addfb-size-offset-overflow
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2b77d9350a3a..2b474d049074 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2636,9 +2636,10 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> > max_size = max(max_size, offset + size);
> > }
> >
> > - if (max_size * tile_size > obj->base.size) {
> > - DRM_DEBUG_KMS("fb too big for bo (need %u bytes, have %zu
> > bytes)\n",
> > - max_size * tile_size, obj->base.size);
> > + if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
> > + DRM_DEBUG_KMS("fb too big for bo (need %llu bytes, have %zu
> > bytes)\n",
> > + (unsigned long long) mul_u32_u32(max_size,
> > tile_size),
>
> mul_u32_u32 returns u64 i.e. unsigned long long; %llu is the one true
> format specifier for u64 (Linus decree #103789)
Well whaddyaknow, so it is. Never realized that for some reason.
>
> Reviewed-by: Chris Wilson
> -Chris
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v4)
Quoting Bob Paauwe (2018-09-12 17:04:30) > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index 43ed8b28aeaa..33d7225edbbb 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) > I915_GTT_PAGE_SIZE_64K | > I915_GTT_PAGE_SIZE_2M; > > + mkwrite_device_info(i915)->full_ppgtt_bits = 48; Actually the mock ppgtt is 64b. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i915, HDMI/DP audio with multiple monitors
Hi, I have a system with multiple monitors and would like to send notification sounds to the monitor on which corresponding window is visible. For a workstation and a tiny computer things look different: - workstation (Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz): 00:02.0 VGA compatible controller [0300]: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor Integrated Graphics Controller [8086:0412] (rev 06) 00:03.0 Audio device [0403]: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor HD Audio Controller [8086:0c0c] (rev 06) 00:1b.0 Audio device [0403]: Intel Corporation 8 Series/C220 Series Chipset High Definition Audio Controller [8086:8c20] (rev 04) Here alsa show me two cards: - HDA Intel PCH (Realtek ALC671) - HDA Intel HDMI (Intel Generic) List of PLAYBACK Hardware Devices card 0: HDMI [HDA Intel HDMI], device 3: Generic Digital [Generic Digital] Subdevices: 1/1 Subdevice #0: subdevice #0 card 1: PCH [HDA Intel PCH], device 0: ALC671 Analog [ALC671 Analog] Subdevices: 1/1 Subdevice #0: subdevice #0 - tiny computer (Intel(R) Core(TM) i5-6500T CPU @ 2.50GHz): 00:02.0 VGA compatible controller [0300]: Intel Corporation HD Graphics 530 [8086:1912] (rev 06) 00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-H HD Audio [8086:a170] (rev 31) Here alsa shows a single card: - HDA Intel PCH (Realtek ALC671) List of PLAYBACK Hardware Devices card 0: PCH [HDA Intel PCH], device 0: ALC671 Analog [ALC671 Analog] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: PCH [HDA Intel PCH], device 3: Generic Digital [Generic Digital] Subdevices: 1/1 Subdevice #0: subdevice #0 How can I determine/set to which monitor the sound should go, and preferably send different sounds to both monitors at same time? Cheers, Bruno ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION 4.19-rc2] sometimes hangs with black screen when resuming from suspend or hibernation (was: Re: Linux 4.19-rc2)
Cc´d Intel Gfx mailing list, in case somebody there knows something: Cc´d Thorsten for regression tracking… forgot initially. Can also open bug report at a later time but so far I cannot provide many details about the issue. Rafael J. Wysocki - 11.09.18, 10:17: > On Tue, Sep 11, 2018 at 10:01 AM Martin Steigerwald wrote: > > Hi. > > > > Linus Torvalds - 02.09.18, 23:45: > > > As usual, the rc2 release is pretty small. People are taking a > > > > With 4.19-rc2 this ThinkPad T520 with i5 Sandybrdige sometimes hangs > > with black screen when resuming from suspend or hibernation. With > > 4.18.1 it did not. Of course there have been userspace related > > updates that could be related. > > > > I currently have no time to dig into this and on this production > > laptop I generally do not do bisects between major kernel releases. > > So currently I only answer questions that do not require much time > > to answer. > > > > For now I switched back to 4.18. If that is stable – and thus likely > > no userspace component is related –, I go with 4.19-rc3 or whatever > > is most recent version to see if the issue has been fixed already. > > There were almost no general changes related to system-wide PM between > 4.18 and current, so I would suspect one of the device drivers or the > x86 core. It also may be something like CPU online/offline, however. I see. I wondered about intel-gfx driver already. Of course it could also be something else. I forgot to mention: The mouse pointer was visible, but the screen remained black. That may again point away from Intel gfx driver. There has been a MESA update in between in userspace. Currently running 4.18.7 to make sure it is no userspace issue. Thanks, -- Martin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter
Am 12.09.2018 um 11:12 schrieb Chris Wilson: Quoting Daniel Vetter (2018-09-12 10:02:47) On Wed, Sep 12, 2018 at 10:50 AM, Chris Wilson wrote: Quoting Daniel Vetter (2018-09-12 09:39:30) On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson wrote: If the ioctl is not supported on a particular piece of HW/driver combination, report ENODEV so that it can be easily distinguished from both the lack of the ioctl and from a regular invalid parameter. Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä Hm, I thought the canonical errno for "ioctl doesn't apply to this device" is ENOTTY? That's ioctl doesn't exist. Sometimes it's nice to know the kernel knows about the ioctl but it isn't applicable. Either is fine for my purpose, which is to know the ioctl exists, I just need to distinguish it from EINVAL. And ENODEV means that it applies, but atm nothing plugged in, or device gone already. I found a few more modeset ioctl: - drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl - drm_mode_getconnector - drm_mode_getcrtc, drm_mode_setcrtc - drm_mode_getencoder - drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ... Ok now I stop looking through the grep thing, looks like we've been using EINVAL consistently. I'm all for switching, it makes sense, but I think we should at least try to be consistent across all kms ioctl. Ah, but we've been consistent on the other side and have been using ENODEV for the better part of a decade for this meaning (that the ioctl doesn't apply to this HW) :) Hm indeed, we've been using either ENODEV or EINVAL for "this ioctl doesn't exist/doesn't apply". ENODEV is clearly used in many places (also outside of drm) for "hw absent/gone/disappeared". And we have very few uses of ENOTTY. So I'm kinda leaning towards starting a new trend here, and using ENOTTY for "this ioctl doesn't apply". I don't think we need to differentiate this from the case of "this kernel has no idea about this ioctl at all", since from a userspace pov there's no difference really: Either way it can't use it. But I'm also fine if we're just sticking to ENODEV, just feels like wasting a perfectly useful errno (and there's not many) if we don't give ENOTTY some more use. In going through the drm_core_checks, I noticed one brave soul used -ENOTSUPP. How about that? +1 for that as well and I'm pretty sure I have seen that in a couple of drivers if an IOCTL isn't supported for a specific device. Christian. -Chris ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Limit the backpressure for i915_request allocation (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: Limit the backpressure for i915_request allocation (rev2) URL : https://patchwork.freedesktop.org/series/49564/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4813 -> Patchwork_10157 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49564/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_10157 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s3: fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773) igt@kms_flip@basic-flip-vs-wf_vblank: fi-ilk-650: PASS -> DMESG-WARN (fdo#106387) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-guc: DMESG-FAIL (fdo#107710) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4813 -> Patchwork_10157 CI_DRM_4813: 3c13515b12339366b414637b69227a4e3cbe21ae @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10157: c12cc33c9748a36fe40789e18f5ed8e0548b2b63 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == c12cc33c9748 drm/i915: Restrict wait to client's timeline on i915_request alloc failure 975f8d9dadc6 drm/i915: Limit the backpressure for i915_request allocation == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10157/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Restrict wait to client's timeline on i915_request alloc failure
Let's try not to overly penalize the unlucky client by making them wait
for others to complete their work, and only apply the ratelimit if they
themselves have outstanding work. Still, we apply global reclaim to the
client (we need to scavenge some memory for it) so it doesn't got away
completely scot free.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_request.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 588bc5a4d18b..72bcb4ca0c45 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -733,12 +733,16 @@ i915_request_alloc(struct intel_engine_cs *engine, struct
i915_gem_context *ctx)
GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
if (unlikely(!rq)) {
/* Ratelimit ourselves to prevent oom from malicious clients */
- ret = i915_gem_wait_for_idle(i915,
-I915_WAIT_LOCKED |
-I915_WAIT_INTERRUPTIBLE,
-1);
- if (ret)
+ rq = i915_gem_active_raw(&ce->ring->timeline->last_request,
+&i915->drm.struct_mutex);
+ if (rq && i915_request_wait(rq,
+ I915_WAIT_LOCKED |
+ I915_WAIT_INTERRUPTIBLE,
+ 1) == -EINTR) {
+ ret = -EINTR;
goto err_unreserve;
+ }
+ i915_retire_requests(i915);
/*
* We've forced the client to stall and catch up with whatever
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Wait for the previous RCU grace period, not request completion
Under mempressure, our goal is to allow ourselves sufficient time to
reclaim the RCU protected slabs without overly penalizing our clients.
Currently, we use a 1 jiffie wait if the client is still active as a
means of throttling the allocations, but we can instead wait for the end
of the RCU grace period of the clients previous allocation.
Suggested-by: Daniel Vetter
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/i915_request.c | 14 ++
drivers/gpu/drm/i915/i915_request.h | 8
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 72bcb4ca0c45..a492385b2089 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -732,17 +732,13 @@ i915_request_alloc(struct intel_engine_cs *engine, struct
i915_gem_context *ctx)
rq = kmem_cache_alloc(i915->requests,
GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
if (unlikely(!rq)) {
+ i915_retire_requests(i915);
+
/* Ratelimit ourselves to prevent oom from malicious clients */
rq = i915_gem_active_raw(&ce->ring->timeline->last_request,
&i915->drm.struct_mutex);
- if (rq && i915_request_wait(rq,
- I915_WAIT_LOCKED |
- I915_WAIT_INTERRUPTIBLE,
- 1) == -EINTR) {
- ret = -EINTR;
- goto err_unreserve;
- }
- i915_retire_requests(i915);
+ if (rq)
+ cond_synchronize_rcu(rq->rcustate);
/*
* We've forced the client to stall and catch up with whatever
@@ -762,6 +758,8 @@ i915_request_alloc(struct intel_engine_cs *engine, struct
i915_gem_context *ctx)
}
}
+ rq->rcustate = get_state_synchronize_rcu();
+
INIT_LIST_HEAD(&rq->active_list);
rq->i915 = i915;
rq->engine = engine;
diff --git a/drivers/gpu/drm/i915/i915_request.h
b/drivers/gpu/drm/i915/i915_request.h
index 9898301ab7ef..7fa94b024968 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -100,6 +100,14 @@ struct i915_request {
struct i915_timeline *timeline;
struct intel_signal_node signaling;
+ /*
+* The rcu epoch of when this request was allocated. Used to judiciously
+* apply backpressure on future allocations to ensure that under
+* mempressure there is sufficient RCU ticks for us to reclaim our
+* RCU protected slabs.
+*/
+ unsigned long rcustate;
+
/*
* Fences for the various phases in the request's lifetime.
*
--
2.19.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Limit the backpressure for i915_request allocation
If we try and fail to allocate a i915_request, we apply some backpressure on the clients to throttle the memory allocations coming from i915.ko. Currently, we wait until completely idle, but this is far too heavy and leads to some situations where the only escape is to declare a client hung and reset the GPU. The intent is to only ratelimit the allocation requests, so we need only wait for a jiffie before using the normal direct reclaim. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106680 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 09ed48833b54..588bc5a4d18b 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -736,7 +736,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) ret = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED | I915_WAIT_INTERRUPTIBLE, -MAX_SCHEDULE_TIMEOUT); +1); if (ret) goto err_unreserve; -- 2.19.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for i915/oa: Simplify updating contexts
== Series Details == Series: i915/oa: Simplify updating contexts URL : https://patchwork.freedesktop.org/series/49569/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4813 -> Patchwork_10158 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49569/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10158 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s3: fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773) igt@kms_frontbuffer_tracking@basic: fi-hsw-peppy: PASS -> DMESG-WARN (fdo#102614) igt@kms_pipe_crc_basic@hang-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-guc: DMESG-FAIL (fdo#107710) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 == Participating hosts (51 -> 45) == Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-glk-j4005 == Build changes == * Linux: CI_DRM_4813 -> Patchwork_10158 CI_DRM_4813: 3c13515b12339366b414637b69227a4e3cbe21ae @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10158: 2a916e5572e4e76f7a31d748b15b417b280e8123 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2a916e5572e4 i915/oa: Simplify updating contexts == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10158/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION 4.19-rc2] sometimes hangs with black screen when resuming from suspend or hibernation (was: Re: Linux 4.19-rc2)
On Tue, Sep 11, 2018 at 12:17:05PM +0200, Martin Steigerwald wrote: > Cc´d Intel Gfx mailing list, in case somebody there knows something: > > Cc´d Thorsten for regression tracking… forgot initially. Can also open > bug report at a later time but so far I cannot provide many details > about the issue. > > Rafael J. Wysocki - 11.09.18, 10:17: > > On Tue, Sep 11, 2018 at 10:01 AM Martin Steigerwald > wrote: > > > Hi. > > > > > > Linus Torvalds - 02.09.18, 23:45: > > > > As usual, the rc2 release is pretty small. People are taking a > > > > > > With 4.19-rc2 this ThinkPad T520 with i5 Sandybrdige sometimes hangs > > > with black screen when resuming from suspend or hibernation. With > > > 4.18.1 it did not. Of course there have been userspace related > > > updates that could be related. > > > > > > I currently have no time to dig into this and on this production > > > laptop I generally do not do bisects between major kernel releases. > > > So currently I only answer questions that do not require much time > > > to answer. > > > > > > For now I switched back to 4.18. If that is stable – and thus likely > > > no userspace component is related –, I go with 4.19-rc3 or whatever > > > is most recent version to see if the issue has been fixed already. > > > > There were almost no general changes related to system-wide PM between > > 4.18 and current, so I would suspect one of the device drivers or the > > x86 core. It also may be something like CPU online/offline, however. > > I see. I wondered about intel-gfx driver already. Of course it could > also be something else. > > I forgot to mention: The mouse pointer was visible, but the screen > remained black. Did the mouse cursor still move or not? Could also try suspend without any GUI stuff in the way. Or try the intel ddx instead of the modesetting ddx (assuming that's what you're using now) and no compositor to rule out GPU hangs killing the compositor. The intel ddx can also deal with the GPU not recovering from a hang by switching to software rendering, whereas modesetting cannot. Hmm. Also T520 is an optimus laptop maybe? If there's an nvidia GPU involved it's going to be hard to get anyone to care. Better switch that off in the BIOS if you haven't already. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Rename full ppgtt configuration to be more generic (rev4)
== Series Details == Series: drm/i915: Rename full ppgtt configuration to be more generic (rev4) URL : https://patchwork.freedesktop.org/series/49021/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4813 -> Patchwork_10159 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49021/revisions/4/mbox/ == Known issues == Here are the changes found in Patchwork_10159 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773) fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_frontbuffer_tracking@basic: fi-hsw-peppy: PASS -> DMESG-WARN (fdo#102614) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-guc: DMESG-FAIL (fdo#107710) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4813 -> Patchwork_10159 CI_DRM_4813: 3c13515b12339366b414637b69227a4e3cbe21ae @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10159: dcdfffbe599f3e59b296fd9891a9fd36dd9d6104 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == dcdfffbe599f drm/i915: Make 48bit full ppgtt configuration generic (v4) == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10159/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Limit the backpressure for i915_request allocation
== Series Details == Series: series starting with [1/3] drm/i915: Limit the backpressure for i915_request allocation URL : https://patchwork.freedesktop.org/series/49572/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4813 -> Patchwork_10160 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49572/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10160 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s4-devices: fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: fi-ilk-650: PASS -> DMESG-WARN (fdo#106387) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-guc: DMESG-FAIL (fdo#107710) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 == Participating hosts (51 -> 46) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4813 -> Patchwork_10160 CI_DRM_4813: 3c13515b12339366b414637b69227a4e3cbe21ae @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10160: 3f7aff32f54467dfbc1b6bffbe82182eaa3ebcc0 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 3f7aff32f544 drm/i915: Wait for the previous RCU grace period, not request completion 27b5d3fba477 drm/i915: Restrict wait to client's timeline on i915_request alloc failure 908461b0dad8 drm/i915: Limit the backpressure for i915_request allocation == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10160/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915, HDMI/DP audio with multiple monitors
On Tue, Sep 11, 2018 at 03:50:13PM +0200, Bruno Prémont wrote:
> Hi,
>
> I have a system with multiple monitors and would like to send
> notification sounds to the monitor on which corresponding
> window is visible.
>
> For a workstation and a tiny computer things look different:
> - workstation (Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz):
> 00:02.0 VGA compatible controller [0300]: Intel Corporation Xeon E3-1200
> v3/4th Gen Core Processor Integrated Graphics Controller [8086:0412] (rev 06)
> 00:03.0 Audio device [0403]: Intel Corporation Xeon E3-1200 v3/4th Gen Core
> Processor HD Audio Controller [8086:0c0c] (rev 06)
> 00:1b.0 Audio device [0403]: Intel Corporation 8 Series/C220 Series Chipset
> High Definition Audio Controller [8086:8c20] (rev 04)
>
> Here alsa show me two cards:
> - HDA Intel PCH (Realtek ALC671)
> - HDA Intel HDMI (Intel Generic)
>
> List of PLAYBACK Hardware Devices
> card 0: HDMI [HDA Intel HDMI], device 3: Generic Digital [Generic Digital]
>Subdevices: 1/1
>Subdevice #0: subdevice #0
There should normally be multiple HDMI devices (one for each HDMI/DP
connector more or less). Eg. my hsw shows:
card 0: HDMI [HDA Intel HDMI], device 3: HDMI 0 [HDMI 0]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: HDMI [HDA Intel HDMI], device 7: HDMI 1 [HDMI 1]
Subdevices: 1/1
Subdevice #0: subdevice #0
Looking at the hda_codec.c we see:
static int audio_idx[HDA_PCM_NTYPES][5] = {
...
[HDA_PCM_TYPE_HDMI] = { 3, 7, 8, 9, -1 },
So you always get those device numbers, but I don't see an immediate
relationship between those and the pin numbers (which do have some
kind of relationship with the HDMI/DP port). I guess if they always
get registered in order of the pin numbers then those would translate
to 3 == port B, 7 == port C, etc. And after that the problem is figuring
out which port is related to which connector, for which we have nothing
at the moment.
I suppose the ideal solution might be to have a sysfs symlink (or
something) to connect the two together.
But then there's MST where I think the pcm device correlates with the
crtc rather than the connector. Maybe. I can't remember anymore, and
I'm not sure it even works (I've recently heard people saying it
doesn't).
> card 1: PCH [HDA Intel PCH], device 0: ALC671 Analog [ALC671 Analog]
>Subdevices: 1/1
>Subdevice #0: subdevice #0
>
>
> - tiny computer (Intel(R) Core(TM) i5-6500T CPU @ 2.50GHz):
> 00:02.0 VGA compatible controller [0300]: Intel Corporation HD Graphics 530
> [8086:1912] (rev 06)
> 00:1f.3 Audio device [0403]: Intel Corporation Sunrise Point-H HD Audio
> [8086:a170] (rev 31)
>
> Here alsa shows a single card:
> - HDA Intel PCH (Realtek ALC671)
>
> List of PLAYBACK Hardware Devices
> card 0: PCH [HDA Intel PCH], device 0: ALC671 Analog [ALC671 Analog]
>Subdevices: 1/1
>Subdevice #0: subdevice #0
> card 0: PCH [HDA Intel PCH], device 3: Generic Digital [Generic Digital]
>Subdevices: 1/1
>Subdevice #0: subdevice #0
>
>
> How can I determine/set to which monitor the sound should go, and preferably
> send
> different sounds to both monitors at same time?
>
>
> Cheers,
> Bruno
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 11/13] drm/i915/icl: Add macros for MMIO of DSI transcoder registers
On Wed, Sep 12, 2018 at 03:06:41PM +0530, Madhav Chauhan wrote:
> On 7/19/2018 9:52 PM, Ville Syrjälä wrote:
> > On Tue, Jul 10, 2018 at 03:10:12PM +0530, Madhav Chauhan wrote:
> >> This patch adds _MMIO_DSI and _DSI_TRANS macros for accessing
> >> DSI transcoder registers.
> >>
> >> Credits-to: Jani N
> >>
> >> Cc: Jani Nikula
> >> Signed-off-by: Madhav Chauhan
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h | 5 +
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> index 1d13ba9..62bc76e 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -9576,6 +9576,11 @@ enum skl_power_gate {
> >> #define _MIPI_PORT(port, a, c) (((port) == PORT_A) ? a : c)/*
> >> ports A and C only */
> >> #define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c))
> >>
> >> +/* gen11 DSI */
> >> +#define _DSI_TRANS(tc, dsi0, dsi1)(((tc) == TRANSCODER_DSI_0) ?
> >> \
> >> + (dsi0) : (dsi1))
> > _PIPE() etc. should result in slughtly better code IIRC.
>
> Can you please clarify on this??
Plenty of examples in i915_reg.h for using _PIPE().
>
> Regards,
> Madhav
>
> >
> >> +#define _MMIO_DSI(tc, dsi0, dsi1) _MMIO(_DSI_TRANS(tc, dsi0, dsi1))
> >> +
> >> #define MIPIO_TXESC_CLK_DIV1 _MMIO(0x160004)
> >> #define GLK_TX_ESC_CLK_DIV1_MASK0x3FF
> >> #define MIPIO_TXESC_CLK_DIV2 _MMIO(0x160008)
> >> --
> >> 2.7.4
> >>
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB
From: Ville Syrjälä
If we have framebuffers that are >= 4GiB in size we will overflow
the fb size check in intel_fill_fb_info().
Currently that is only possible with NV12 and CCS as offsets[1]
may be anything between 0 and 0x. offsets[0] is currently
required to be 0 so we can't hit the overflow with any single
plane format (thanks to max fb size of 8kx8k and max stride of
32 KiB).
In the future we may allow almost any framebuffer to exceed 4GiB
in size so we really should fix the overflow. Not that the overflow
is particularly dangerous. It's mostly just a sanity check against
insane userspace. The display engine can't write to memory anyway
so I suppose in the worst case we might anger the hw by attempting
scanout past the end of the ggtt, or we might scan out some data
that we're not supposed to see from other parts of the ggtt.
Note that triggering this overflow depends on the driver
aligning the fb height to the next tile boundary to push the
calculated size above 4GiB. With linear buffers the effective
tile height is one so that never happens, and the core already
has a check for 32bit overflow of offsets[]+pitches[]*height.
v2: Drop the unnecessary cast (Chris)
Testcase: igt/kms_big_fb/x-tiled-addfb-size-offset-overflow
Testcase: igt/kms_big_fb/y-tiled-addfb-size-offset-overflow
Signed-off-by: Ville Syrjälä
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7fbc006be44a..6928dcc926ef 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2640,9 +2640,9 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
max_size = max(max_size, offset + size);
}
- if (max_size * tile_size > obj->base.size) {
- DRM_DEBUG_KMS("fb too big for bo (need %u bytes, have %zu
bytes)\n",
- max_size * tile_size, obj->base.size);
+ if (mul_u32_u32(max_size, tile_size) > obj->base.size) {
+ DRM_DEBUG_KMS("fb too big for bo (need %llu bytes, have %zu
bytes)\n",
+ mul_u32_u32(max_size, tile_size), obj->base.size);
return -EINVAL;
}
--
2.16.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915, HDMI/DP audio with multiple monitors
On Wed, 12 Sep 2018 19:46:58 +0200, Ville Syrjälä wrote: > > On Tue, Sep 11, 2018 at 03:50:13PM +0200, Bruno Prémont wrote: > > Hi, > > > > I have a system with multiple monitors and would like to send > > notification sounds to the monitor on which corresponding > > window is visible. > > > > For a workstation and a tiny computer things look different: > > - workstation (Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz): > > 00:02.0 VGA compatible controller [0300]: Intel Corporation Xeon E3-1200 > > v3/4th Gen Core Processor Integrated Graphics Controller [8086:0412] (rev > > 06) > > 00:03.0 Audio device [0403]: Intel Corporation Xeon E3-1200 v3/4th Gen > > Core Processor HD Audio Controller [8086:0c0c] (rev 06) > > 00:1b.0 Audio device [0403]: Intel Corporation 8 Series/C220 Series > > Chipset High Definition Audio Controller [8086:8c20] (rev 04) > > > > Here alsa show me two cards: > > - HDA Intel PCH (Realtek ALC671) > > - HDA Intel HDMI (Intel Generic) > > > > List of PLAYBACK Hardware Devices > > card 0: HDMI [HDA Intel HDMI], device 3: Generic Digital [Generic Digital] > >Subdevices: 1/1 > >Subdevice #0: subdevice #0 Is a proper kernel config (CONFIG_SND_HDA_CODEC_HDMI) enabled? The device name looks strange as if it's not properly bound with the HDMI codec driver. Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB (rev2)
== Series Details == Series: drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB (rev2) URL : https://patchwork.freedesktop.org/series/49495/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB -O:drivers/gpu/drm/i915/intel_display.c:2640:28: warning: expression using sizeof(void) -O:drivers/gpu/drm/i915/intel_display.c:2640:28: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_display.c:2640:28: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_display.c:2640:28: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On Wed, 12 Sep 2018 15:18:47 +0200,
Imre Deak wrote:
>
> +Takashi
>
> On Wed, Sep 12, 2018 at 04:18:12PM +0300, Imre Deak wrote:
> > Hi Kumar, Takashi,
> >
> > On Tue, Jun 19, 2018 at 03:01:11PM -0700, Abhay Kumar wrote:
> > > From: Ville Syrjälä
> > >
> > > CDCLK has to be at least twice the BLCK regardless of audio. Audio
> > > driver has to probe using this hook and increase the clock even in
> > > absence of any display.
> > >
> > > v2: Use atomic refcount for get_power, put_power so that we can
> > > call each once(Abhay).
> > > v3: Reset power well 2 to avoid any transaction on iDisp link
> > > during cdclk change(Abhay).
> > > v4: Remove Power well 2 reset workaround(Ville).
> > > v5: Remove unwanted Power well 2 register defined in v4(Abhay).
> > >
> > > Signed-off-by: Ville Syrjälä
> > > Signed-off-by: Abhay Kumar
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 3 ++
> > > drivers/gpu/drm/i915/intel_audio.c | 67
> > > +---
> > > drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
> > > drivers/gpu/drm/i915/intel_display.c | 7 +++-
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > 5 files changed, 83 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 6104d7115054..a4a386a5db69 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1702,6 +1702,7 @@ struct drm_i915_private {
> > > unsigned int hpll_freq;
> > > unsigned int fdi_pll_freq;
> > > unsigned int czclk_freq;
> > > + u32 get_put_refcount;
> > >
> > > struct {
> > > /*
> > > @@ -1719,6 +1720,8 @@ struct drm_i915_private {
> > > struct intel_cdclk_state actual;
> > > /* The current hardware cdclk state */
> > > struct intel_cdclk_state hw;
> > > +
> > > + int force_min_cdclk;
> > > } cdclk;
> > >
> > > /**
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > > b/drivers/gpu/drm/i915/intel_audio.c
> > > index 3ea566f99450..ca8f04c7cbb3 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder
> > > *encoder,
> > >
> > > if (!connector->eld[0])
> > > return;
> > > -
> > > DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> > >connector->base.id,
> > >connector->name,
> > > @@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
> > > *dev_priv)
> > > }
> > > }
> > >
> > > +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> > > + bool enable)
> > > +{
> > > + struct drm_modeset_acquire_ctx ctx;
> > > + struct drm_atomic_state *state;
> > > + int ret;
> > > +
> > > + drm_modeset_acquire_init(&ctx, 0);
> > > + state = drm_atomic_state_alloc(&dev_priv->drm);
> > > + if (WARN_ON(!state))
> > > + return;
> > > +
> > > + state->acquire_ctx = &ctx;
> > > +
> > > +retry:
> > > + to_intel_atomic_state(state)->modeset = true;
> > > + to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> > > + enable ? 2 * 96000 : 0;
> > > +
> > > + /*
> > > + * Protects dev_priv->cdclk.force_min_cdclk
> > > + * Need to lock this here in case we have no active pipes
> > > + * and thus wouldn't lock it during the commit otherwise.
> > > + */
> > > + ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> > > &ctx);
> > > + if (!ret)
> > > + ret = drm_atomic_commit(state);
> >
> > Ville mentioned a potential dead-lock problem here: a normal modeset
> > enabling an HDMI/DP output with an audio sink will call the
> > drm_audio_component_audio_ops::pin_eld_notify() callback with the
> > connection_mutex already held. This callback in turn could call
> > drm_audio_component_ops::get_power()/put_power() callbacks and
> > dead-lock at the above drm_modeset_lock() call.
> >
> > Looks like that
> >
> >sound/pci/hda/patch_hdmi.c / intel_pin_eld_notify()->
> >check_presence_and_report()->
> >hdmi_present_sense()
> >
> > would prevent this, but for instance
> >
> >sound/soc/codecs/hdac_hdmi.c / hdac_hdmi_eld_notify_cb()->
> >hdac_hdmi_present_sense()->
> >hdac_hdmi_jack_report()->
> >snd_soc_jack_report()->
> >snd_soc_dapm_sync()->
> >snd_soc_dapm_sync_unlocked()->
> >dapm_power_widgets()->
> >dapm_pre_sequence_async()
> >
> > could call pm_runtime_get_sync() and so I guess also the aops
> > get_power() hook.
> >
> > Could someone in your team check if the above can indeed happen?
Through a quick glance, I'm afraid that it's indeed possible. We need
to off-load either the hdac_hdmi jack handling or this new
force_audio_cdclk stuff, I suppose.
thanks,
Takashi
> > > +
> > > + if (ret == -EDEADLK) {
> > > + drm_atomic_state_clear(state);
> >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB (rev2)
== Series Details == Series: drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB (rev2) URL : https://patchwork.freedesktop.org/series/49495/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4813 -> Patchwork_10161 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49495/revisions/2/mbox/ == Known issues == Here are the changes found in Patchwork_10161 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) igt@kms_pipe_crc_basic@hang-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-skl-guc: PASS -> FAIL (fdo#103191) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-guc: DMESG-FAIL (fdo#107710) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107710 https://bugs.freedesktop.org/show_bug.cgi?id=107710 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 == Participating hosts (51 -> 44) == Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_4813 -> Patchwork_10161 CI_DRM_4813: 3c13515b12339366b414637b69227a4e3cbe21ae @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10161: 712f10408dd4482a72bf80eebf61d5f63761e8a7 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 712f10408dd4 drm/i915: Fix a potential integer overflow with framebuffers extending past 4 GiB == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10161/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
