Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
On Tue, 14 May 2019 16:34:01 +0200 Daniel Vetter wrote: > On Tue, May 14, 2019 at 3:36 PM Pekka Paalanen wrote: > > > > On Tue, 14 May 2019 13:02:09 +0200 > > Daniel Vetter wrote: > > > > > On Tue, May 14, 2019 at 10:18 AM Ser, Simon wrote: > > > > > > > > On Tue, 2019-05-14 at 11:02 +0300, Pekka Paalanen wrote: ... > > > > > Hi Daniel, > > > > > > > > > > just to clarify the first case, specific to one very particular > > > > > property: > > > > > > > > > > With HDCP, there is a property that may change dynamically at runtime > > > > > (the undesired/desired/enabled tristate). Userspace must be notified > > > > > when it changes, I do not want userspace have to poll that property > > > > > with a timer. > > > > > > > > > > When that property alone changes, and userspace is prepared to handle > > > > > that property changing alone, it must not trigger a reprobe of the > > > > > connector. There is no reason to reprobe at that point AFAIU. > > > > > > > > > > How do you ensure that userspace can avoid triggering a reprobe with > > > > > the > > > > > epoch approach or with any alternate uevent design? > > > > > > > > > > We need an event to userspace that indicates that re-reading the > > > > > properties is enough and reprobe of the connector is not necessary. > > > > > This is complementary to indicating to userspace that only some > > > > > connectors need to be reprobed instead of everything. > > > > > > > > Can't you use the PROPERTY hint? If PROPERTY is the HDCP one, skip the > > > > reprobing. Would that work? > > > > Hi, > > > > yes, that would work, if it was acceptable to DRM upstream. The replies > > to Paul seemed to be going south so fast that I thought we wouldn't get > > any new uevent fields in favour of "epoch counters". > > > > > Yes that's the idea, depending upon which property you get you know > > > it's a sink change (needs full reprobe) or something else like hdcp > > > state machinery update. > > > > Right. > > > > > Wrt avoiding the full reprobe for sink changes: I think we should > > > indeed decouple that from the per-connector event for sink changes. > > > That along is a good win already, since you know for which connector > > > you need to call drmGetConnector (which forces the reprobe). It would > > > be nice to only call drmGetConnectorCurrent (avoids the reprobe), but > > > historically speaking every time we tried to rely on this we ended up > > > regretting things. > > > > What changed? This sounds very much what Paul suggested. Looking at it > > from userspace side: > > This sounds solid, some refinements below: > > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy > > > > - If yy is "Content Protection", no need to drmModeGetConnector(), just > > re-get the connector properties. > > > > - Kernel probably shouldn't bother sending this for properties where > > re-probe could be necessary, and send the below instead. > > > I think we should assert that the kernel can get the new property > values using drmModeGetConnectorCurrent for this case, i.e. the kernel > does not expect a full reprobe. I.e. upgrade your idea from "should" > to "must" Hi Daniel, ok, that's good. > Furthermore different property can indicate different kind of updates, > e.g. hdcp vs general sink change vs. whatever else might come in the > future. What do you mean by different kinds of updates? Btw. I started thinking, maybe we should completely leave out the "If yy is "Content Protection"" and require the kernel to guarantee, that if PROPERTY is set, then drmModeGetConnector() (probing) must not be necessary based on this event alone. Writing it down again: HOTPLUG=1 CONNECTOR=xx PROPERTY=yy - yy denotes which connector xx property changed. - Userspace does not need to do drmModeGetConnector(), it only needs to drmModeObjectGetProperties() on the connector to receive the new updated property values. - Kernel must not send this event for changes that may require probing for correct results, exceptional conditions (buggy hardware, etc.) included. Instead, the kernel must send one of the below events. Is there actually anything interesting that drmModeGetConnectorCurrent() could guaranteed correctly return that is not a property already? I'd probably leave this consideration out completely, and just say do one of the needs-probing events if anything there changed. > > HOTPLUG=1 CONNECTOR=xx > > > > - Needs to drmModeGetConnector() on the one connector, no need to probe > > others. Implies that one needs to re-get the connector properties as > > well. > > Sounds good. > > > HOTPLUG=1 > > > > - Need to do drmModeGetResouces() to discover new/disappeared > > connectors, and need to drmModeGetConnector to re-probe every > > connector. (As always.) > > Maybe we should clarify that this is also what you get when an entire > connector appears/disappears (for dp mst hotplug). Yes, that's what I wrote. :-) Weston implements the discovery of appearing/d
Re: [Intel-gfx] [v6][PATCH 01/12] drm/i915: Introduce vfunc read_luts() to create hw lut
On Tue, 14 May 2019, Ville Syrjälä wrote:
> On Tue, May 14, 2019 at 03:13:19PM +0530, Swati Sharma wrote:
>> In this patch, a vfunc read_luts() is introduced to create a hw lut
>> i.e. lut having values read from gamma/degamma registers which will
>> later be used to compare with sw lut to validate gamma/degamma lut values.
>>
>> v3: -Rebase
>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>> -Wrapped get_color_config() [Jani]
>> v5: -Renamed intel_color_get_config() to intel_color_read_luts()
>> -Renamed get_color_config to read_luts
>> v6: -Renamed intel_color_read_luts() back to intel_color_get_config()
>> [Jani and Ville]
>>
>> Signed-off-by: Swati Sharma
>> ---
>> drivers/gpu/drm/i915/i915_drv.h| 1 +
>> drivers/gpu/drm/i915/intel_color.c | 8
>> drivers/gpu/drm/i915/intel_color.h | 1 +
>> 3 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index d025780..6343e70 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -343,6 +343,7 @@ struct drm_i915_display_funcs {
>> * involved with the same commit.
>> */
>> void (*load_luts)(const struct intel_crtc_state *crtc_state);
>> +void (*read_luts)(struct intel_crtc_state *crtc_state);
>
> I think Jani wanted the entire vfunc renamed back.
*shrug* not so important as the main entry point below.
BR,
Jani,
>
>> };
>>
>> struct intel_csr {
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 962db12..50b98ee 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state
>> *crtc_state)
>> return dev_priv->display.color_check(crtc_state);
>> }
>>
>> +void intel_color_get_config(struct intel_crtc_state *crtc_state)
>> +{
>> +struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>> +
>> +if (dev_priv->display.read_luts)
>> +dev_priv->display.read_luts(crtc_state);
>> +}
>> +
>> static bool need_plane_update(struct intel_plane *plane,
>>const struct intel_crtc_state *crtc_state)
>> {
>> diff --git a/drivers/gpu/drm/i915/intel_color.h
>> b/drivers/gpu/drm/i915/intel_color.h
>> index b8a3ce6..057e8ac 100644
>> --- a/drivers/gpu/drm/i915/intel_color.h
>> +++ b/drivers/gpu/drm/i915/intel_color.h
>> @@ -13,5 +13,6 @@
>> int intel_color_check(struct intel_crtc_state *crtc_state);
>> void intel_color_commit(const struct intel_crtc_state *crtc_state);
>> void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>> +void intel_color_get_config(struct intel_crtc_state *crtc_state);
>>
>> #endif /* __INTEL_COLOR_H__ */
>> --
>> 1.9.1
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-next-fixes
Hi Dave & Daniel, A fix to close a race opportunity between IRQ handler and RCU. Two fixes that are also stable, disabling FBC on GLK and HSW EDP fastset correction. These patches definitely caused conflicts when merged, resolutions should be all good. Regards, Joonas *** drm-intel-next-fixes-2019-05-15: - Disable framebuffer compression on Geminilake - Fixes for HSW EDP fastset and a IRQ handler vs. RCU race The following changes since commit 23372cce8fe7ee98a6458fd3d035a55b87f0c6fe: Merge tag 'gvt-next-fixes-2019-05-07' of https://github.com/intel/gvt-linux into drm-intel-next-fixes (2019-05-07 15:29:15 +0300) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2019-05-15 for you to fetch changes up to c36beba6b296b3c05a0f29753b04775e5ae23886: drm/i915: Seal races between async GPU cancellation, retirement and signaling (2019-05-13 13:53:35 +0300) - Disable framebuffer compression on Geminilake - Fixes for HSW EDP fastset and a IRQ handler vs. RCU race Chris Wilson (1): drm/i915: Seal races between async GPU cancellation, retirement and signaling Daniel Drake (1): drm/i915/fbc: disable framebuffer compression on GeminiLake Ville Syrjälä (1): drm/i915: Fix fastset vs. pfit on/off on HSW EDP transcoder drivers/dma-buf/dma-fence.c | 1 + drivers/gpu/drm/i915/i915_request.c | 1 + drivers/gpu/drm/i915/intel_breadcrumbs.c| 78 + drivers/gpu/drm/i915/intel_display.c| 9 drivers/gpu/drm/i915/intel_fbc.c| 4 ++ drivers/gpu/drm/i915/intel_guc_submission.c | 1 - drivers/gpu/drm/i915/intel_pipe_crc.c | 13 +++-- 7 files changed, 82 insertions(+), 25 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
On Wed, May 15, 2019 at 09:43:24AM +0200, Paul Kocialkowski wrote: > Hi, > > On Tue, 2019-05-14 at 16:28 +0200, Daniel Vetter wrote: > > On Tue, May 14, 2019 at 4:13 PM Paul Kocialkowski > > wrote: > > > Hi, > > > > > > On Tue, 2019-05-14 at 13:09 +0200, Daniel Vetter wrote: > > > > On Mon, May 13, 2019 at 7:14 PM Paul Kocialkowski > > > > wrote: > > > > > Hey, > > > > > > > > > > Le lundi 13 mai 2019 à 11:34 +0200, Daniel Vetter a écrit : > > > > > > On Mon, May 13, 2019 at 11:02 AM Paul Kocialkowski > > > > > > wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Fri, 2019-05-10 at 16:54 +0200, Daniel Vetter wrote: > > > > > > > > On Fri, May 10, 2019 at 2:12 PM Paul Kocialkowski > > > > > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote: > > > > > > > > > > DRM API for generating uevent for a status changes of > > > > > > > > > > connector's > > > > > > > > > > property. > > > > > > > > > > > > > > > > > > > > This uevent will have following details related to the > > > > > > > > > > status change: > > > > > > > > > > > > > > > > > > > > HOTPLUG=1, CONNECTOR= and > > > > > > > > > > PROPERTY= > > > > > > > > > > > > > > > > > > > > Need ACK from this uevent from userspace consumer. > > > > > > > > > > > > > > > > > > So we just had some discussions over on IRC and at about the > > > > > > > > > hotplug > > > > > > > > > issue and came up with similar ideas: > > > > > > > > > https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html > > > > > > > > > > > > > > > > > > The conclusions of these discussions so far would be to have > > > > > > > > > a more or > > > > > > > > > less fine grain of uevent reporting depending on what > > > > > > > > > happened. The > > > > > > > > > point is that we need to cover different cases: > > > > > > > > > - one or more properties changed; > > > > > > > > > - the connector status changed; > > > > > > > > > - something else about the connector changed (e.g. EDID/modes) > > > > > > > > > > > > > > > > > > For the first case, we can send out: > > > > > > > > > HOTPLUG=1 > > > > > > > > > CONNECTOR= > > > > > > > > > PROPERTY= > > > > > > > > > > > > > > > > > > and no reprobe is required. > > > > > > > > > > > > > > > > > > For the second one, something like: > > > > > > > > > HOTPLUG=1 > > > > > > > > > CONNECTOR= > > > > > > > > > STATUS=Connected/Disconnected > > > > > > > > > > > > > > > > > > and a connector probe is needed for connected, but not for > > > > > > > > > disconnected; > > > > > > > > > > > > > > > > > > For the third one, we can only indicate the connector: > > > > > > > > > HOTPLUG=1 > > > > > > > > > CONNECTOR= > > > > > > > > > > > > > > > > > > and a reprobe of the connector is always needed > > > > > > > > > > > > > > > > There's no material difference between this one and the > > > > > > > > previous one. > > > > > > > > Plus there's no beenfit in supplying the actual value of the > > > > > > > > property, > > > > > > > > i.e. we can reuse the same PROPERTY= > > > > > > > > trick. > > > > > > > > > > > > > > That's the idea, but we need to handle status changes differently > > > > > > > than > > > > > > > properties, since as far as I know, connected/unconnected status > > > > > > > is not > > > > > > > exposed as a prop for the connector. > > > > > > > > > > > > Oops, totally missed that. "Everything is a property" is kinda > > > > > > new-ish, at least compared to kms. Kinda tempted to just make status > > > > > > into a property. Or another excuse why we should expose the epoch > > > > > > property :-) > > > > > > > > > > Well I think it would make sense anyway, as long as we can make sure > > > > > it > > > > > stays consistent with the one reported in the connector struct. > > > > > > > > > > > > > Here's why: > > > > > > > > - A side effect of forcing a probe on a connector is that you > > > > > > > > get to > > > > > > > > read all the properties, so supplying them is kinda pointless. > > > > > > > > > > > > > > Agreed, except for the status case where it's useful to know it's > > > > > > > a > > > > > > > disconnect, because we don't need any probe step in that case. > > > > > > > > > > > > > > > - You can read STATUS without forcing a reprobe, if you want to > > > > > > > > avoid > > > > > > > > the reprobe for disconnected. I'd kinda not recommend that > > > > > > > > though, > > > > > > > > feels a bit like overoptimizing. And for reasonable connectors > > > > > > > > (i.e. > > > > > > > > dp) reprobing a disconnected output is fast. HDMI is ... less > > > > > > > > reasonable unfortunately, but oh well. > > > > > > > > > > > > > > How would that be retreived then? From the looks of it, that's a > > > > > > > MODE_GETCONNECTOR ioctl and I was under the impression this is > > > > > > > what > > > > > > > does the full reprobe. > > > > > > > > > > > > drmGetConnector vs drmGetConnectorCurrent. > > > > >
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
Hi, On Wed, 2019-05-15 at 10:37 +0300, Pekka Paalanen wrote: > On Tue, 14 May 2019 16:34:01 +0200 > Daniel Vetter wrote: > > > On Tue, May 14, 2019 at 3:36 PM Pekka Paalanen wrote: > > > On Tue, 14 May 2019 13:02:09 +0200 > > > Daniel Vetter wrote: > > > > > > > On Tue, May 14, 2019 at 10:18 AM Ser, Simon > > > > wrote: > > > > > On Tue, 2019-05-14 at 11:02 +0300, Pekka Paalanen wrote: > > ... > > > > > > > Hi Daniel, > > > > > > > > > > > > just to clarify the first case, specific to one very particular > > > > > > property: > > > > > > > > > > > > With HDCP, there is a property that may change dynamically at > > > > > > runtime > > > > > > (the undesired/desired/enabled tristate). Userspace must be notified > > > > > > when it changes, I do not want userspace have to poll that property > > > > > > with a timer. > > > > > > > > > > > > When that property alone changes, and userspace is prepared to > > > > > > handle > > > > > > that property changing alone, it must not trigger a reprobe of the > > > > > > connector. There is no reason to reprobe at that point AFAIU. > > > > > > > > > > > > How do you ensure that userspace can avoid triggering a reprobe > > > > > > with the > > > > > > epoch approach or with any alternate uevent design? > > > > > > > > > > > > We need an event to userspace that indicates that re-reading the > > > > > > properties is enough and reprobe of the connector is not necessary. > > > > > > This is complementary to indicating to userspace that only some > > > > > > connectors need to be reprobed instead of everything. > > > > > > > > > > Can't you use the PROPERTY hint? If PROPERTY is the HDCP one, skip the > > > > > reprobing. Would that work? > > > > > > Hi, > > > > > > yes, that would work, if it was acceptable to DRM upstream. The replies > > > to Paul seemed to be going south so fast that I thought we wouldn't get > > > any new uevent fields in favour of "epoch counters". > > > > > > > Yes that's the idea, depending upon which property you get you know > > > > it's a sink change (needs full reprobe) or something else like hdcp > > > > state machinery update. > > > > > > Right. > > > > > > > Wrt avoiding the full reprobe for sink changes: I think we should > > > > indeed decouple that from the per-connector event for sink changes. > > > > That along is a good win already, since you know for which connector > > > > you need to call drmGetConnector (which forces the reprobe). It would > > > > be nice to only call drmGetConnectorCurrent (avoids the reprobe), but > > > > historically speaking every time we tried to rely on this we ended up > > > > regretting things. > > > > > > What changed? This sounds very much what Paul suggested. Looking at it > > > from userspace side: > > > > This sounds solid, some refinements below: > > > > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy > > > > > > - If yy is "Content Protection", no need to drmModeGetConnector(), just > > > re-get the connector properties. > > > > > > - Kernel probably shouldn't bother sending this for properties where > > > re-probe could be necessary, and send the below instead. > > > > I think we should assert that the kernel can get the new property > > values using drmModeGetConnectorCurrent for this case, i.e. the kernel > > does not expect a full reprobe. I.e. upgrade your idea from "should" > > to "must" > > Hi Daniel, > > ok, that's good. > > > Furthermore different property can indicate different kind of updates, > > e.g. hdcp vs general sink change vs. whatever else might come in the > > future. > > What do you mean by different kinds of updates? > > Btw. I started thinking, maybe we should completely leave out the "If > yy is "Content Protection"" and require the kernel to guarantee, that > if PROPERTY is set, then drmModeGetConnector() (probing) must not be > necessary based on this event alone. I agree, this is precisely what I had in mind. > Writing it down again: > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy > > - yy denotes which connector xx property changed. > > - Userspace does not need to do drmModeGetConnector(), it only needs to > drmModeObjectGetProperties() on the connector to receive the new > updated property values. > > - Kernel must not send this event for changes that may require probing > for correct results, exceptional conditions (buggy hardware, etc.) > included. Instead, the kernel must send one of the below events. Agreed, and leave this up to the driver in the end, not the core. > Is there actually anything interesting that > drmModeGetConnectorCurrent() could guaranteed correctly return that is > not a property already? I'd probably leave this consideration out > completely, and just say do one of the needs-probing events if anything > there changed. In the end, I think this should help move to a situation where userspace would not have to do a reprobe at any point and the kernel side just does it. I see no justification for ask
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
On Wed, May 15, 2019 at 10:37:31AM +0300, Pekka Paalanen wrote: > On Tue, 14 May 2019 16:34:01 +0200 > Daniel Vetter wrote: > > > On Tue, May 14, 2019 at 3:36 PM Pekka Paalanen wrote: > > > > > > On Tue, 14 May 2019 13:02:09 +0200 > > > Daniel Vetter wrote: > > > > > > > On Tue, May 14, 2019 at 10:18 AM Ser, Simon > > > > wrote: > > > > > > > > > > On Tue, 2019-05-14 at 11:02 +0300, Pekka Paalanen wrote: > > ... > > > > > > > Hi Daniel, > > > > > > > > > > > > just to clarify the first case, specific to one very particular > > > > > > property: > > > > > > > > > > > > With HDCP, there is a property that may change dynamically at > > > > > > runtime > > > > > > (the undesired/desired/enabled tristate). Userspace must be notified > > > > > > when it changes, I do not want userspace have to poll that property > > > > > > with a timer. > > > > > > > > > > > > When that property alone changes, and userspace is prepared to > > > > > > handle > > > > > > that property changing alone, it must not trigger a reprobe of the > > > > > > connector. There is no reason to reprobe at that point AFAIU. > > > > > > > > > > > > How do you ensure that userspace can avoid triggering a reprobe > > > > > > with the > > > > > > epoch approach or with any alternate uevent design? > > > > > > > > > > > > We need an event to userspace that indicates that re-reading the > > > > > > properties is enough and reprobe of the connector is not necessary. > > > > > > This is complementary to indicating to userspace that only some > > > > > > connectors need to be reprobed instead of everything. > > > > > > > > > > Can't you use the PROPERTY hint? If PROPERTY is the HDCP one, skip the > > > > > reprobing. Would that work? > > > > > > Hi, > > > > > > yes, that would work, if it was acceptable to DRM upstream. The replies > > > to Paul seemed to be going south so fast that I thought we wouldn't get > > > any new uevent fields in favour of "epoch counters". > > > > > > > Yes that's the idea, depending upon which property you get you know > > > > it's a sink change (needs full reprobe) or something else like hdcp > > > > state machinery update. > > > > > > Right. > > > > > > > Wrt avoiding the full reprobe for sink changes: I think we should > > > > indeed decouple that from the per-connector event for sink changes. > > > > That along is a good win already, since you know for which connector > > > > you need to call drmGetConnector (which forces the reprobe). It would > > > > be nice to only call drmGetConnectorCurrent (avoids the reprobe), but > > > > historically speaking every time we tried to rely on this we ended up > > > > regretting things. > > > > > > What changed? This sounds very much what Paul suggested. Looking at it > > > from userspace side: > > > > This sounds solid, some refinements below: > > > > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy > > > > > > - If yy is "Content Protection", no need to drmModeGetConnector(), just > > > re-get the connector properties. > > > > > > - Kernel probably shouldn't bother sending this for properties where > > > re-probe could be necessary, and send the below instead. > > > > > > I think we should assert that the kernel can get the new property > > values using drmModeGetConnectorCurrent for this case, i.e. the kernel > > does not expect a full reprobe. I.e. upgrade your idea from "should" > > to "must" > > Hi Daniel, > > ok, that's good. > > > Furthermore different property can indicate different kind of updates, > > e.g. hdcp vs general sink change vs. whatever else might come in the > > future. > > What do you mean by different kinds of updates? Atm we're discussing two: - "Content Protection" - "sink changed, but you don't need to reprobe" this would be quite a bit a catch all from the output detection. Paul thinks differently, but I'm not sold on splitting this up more, at least not right now. This would include connector status (and related things returned by drmGetConnector which currently aren't a property), EDID, the mst path id, that kind of stuff. Ime once we have 2, there's more bound to come :-) > Btw. I started thinking, maybe we should completely leave out the "If > yy is "Content Protection"" and require the kernel to guarantee, that > if PROPERTY is set, then drmModeGetConnector() (probing) must not be > necessary based on this event alone. > > Writing it down again: > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy > > - yy denotes which connector xx property changed. Maybe yy denotes which group of properties changed, and part of the uapi is picking the canonical one. E.g. content protection might also gain more properties in the future (there's patches, but the userspace won't be open sourced). And for that case I don't think we should then send an even for every single individual property, but just for the lead property. Maybe we should change it to UPDATE_TYPE=, but it felt better to use the property id we already have for this.
Re: [Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change
Hi, On Tue, 2019-05-14 at 16:28 +0200, Daniel Vetter wrote: > On Tue, May 14, 2019 at 4:13 PM Paul Kocialkowski > wrote: > > Hi, > > > > On Tue, 2019-05-14 at 13:09 +0200, Daniel Vetter wrote: > > > On Mon, May 13, 2019 at 7:14 PM Paul Kocialkowski > > > wrote: > > > > Hey, > > > > > > > > Le lundi 13 mai 2019 à 11:34 +0200, Daniel Vetter a écrit : > > > > > On Mon, May 13, 2019 at 11:02 AM Paul Kocialkowski > > > > > wrote: > > > > > > Hi, > > > > > > > > > > > > On Fri, 2019-05-10 at 16:54 +0200, Daniel Vetter wrote: > > > > > > > On Fri, May 10, 2019 at 2:12 PM Paul Kocialkowski > > > > > > > wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote: > > > > > > > > > DRM API for generating uevent for a status changes of > > > > > > > > > connector's > > > > > > > > > property. > > > > > > > > > > > > > > > > > > This uevent will have following details related to the status > > > > > > > > > change: > > > > > > > > > > > > > > > > > > HOTPLUG=1, CONNECTOR= and > > > > > > > > > PROPERTY= > > > > > > > > > > > > > > > > > > Need ACK from this uevent from userspace consumer. > > > > > > > > > > > > > > > > So we just had some discussions over on IRC and at about the > > > > > > > > hotplug > > > > > > > > issue and came up with similar ideas: > > > > > > > > https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html > > > > > > > > > > > > > > > > The conclusions of these discussions so far would be to have a > > > > > > > > more or > > > > > > > > less fine grain of uevent reporting depending on what happened. > > > > > > > > The > > > > > > > > point is that we need to cover different cases: > > > > > > > > - one or more properties changed; > > > > > > > > - the connector status changed; > > > > > > > > - something else about the connector changed (e.g. EDID/modes) > > > > > > > > > > > > > > > > For the first case, we can send out: > > > > > > > > HOTPLUG=1 > > > > > > > > CONNECTOR= > > > > > > > > PROPERTY= > > > > > > > > > > > > > > > > and no reprobe is required. > > > > > > > > > > > > > > > > For the second one, something like: > > > > > > > > HOTPLUG=1 > > > > > > > > CONNECTOR= > > > > > > > > STATUS=Connected/Disconnected > > > > > > > > > > > > > > > > and a connector probe is needed for connected, but not for > > > > > > > > disconnected; > > > > > > > > > > > > > > > > For the third one, we can only indicate the connector: > > > > > > > > HOTPLUG=1 > > > > > > > > CONNECTOR= > > > > > > > > > > > > > > > > and a reprobe of the connector is always needed > > > > > > > > > > > > > > There's no material difference between this one and the previous > > > > > > > one. > > > > > > > Plus there's no beenfit in supplying the actual value of the > > > > > > > property, > > > > > > > i.e. we can reuse the same PROPERTY= trick. > > > > > > > > > > > > That's the idea, but we need to handle status changes differently > > > > > > than > > > > > > properties, since as far as I know, connected/unconnected status is > > > > > > not > > > > > > exposed as a prop for the connector. > > > > > > > > > > Oops, totally missed that. "Everything is a property" is kinda > > > > > new-ish, at least compared to kms. Kinda tempted to just make status > > > > > into a property. Or another excuse why we should expose the epoch > > > > > property :-) > > > > > > > > Well I think it would make sense anyway, as long as we can make sure it > > > > stays consistent with the one reported in the connector struct. > > > > > > > > > > > Here's why: > > > > > > > - A side effect of forcing a probe on a connector is that you get > > > > > > > to > > > > > > > read all the properties, so supplying them is kinda pointless. > > > > > > > > > > > > Agreed, except for the status case where it's useful to know it's a > > > > > > disconnect, because we don't need any probe step in that case. > > > > > > > > > > > > > - You can read STATUS without forcing a reprobe, if you want to > > > > > > > avoid > > > > > > > the reprobe for disconnected. I'd kinda not recommend that though, > > > > > > > feels a bit like overoptimizing. And for reasonable connectors > > > > > > > (i.e. > > > > > > > dp) reprobing a disconnected output is fast. HDMI is ... less > > > > > > > reasonable unfortunately, but oh well. > > > > > > > > > > > > How would that be retreived then? From the looks of it, that's a > > > > > > MODE_GETCONNECTOR ioctl and I was under the impression this is what > > > > > > does the full reprobe. > > > > > > > > > > drmGetConnector vs drmGetConnectorCurrent. > > > > > > > > Ah right, forgot about that one, thanks. > > > > > > > > > > Not sure what issues could arise in case of disconnect without > > > > > > reprobe > > > > > > -- at least I don't see why userspace should have to do anything in > > > > > > particular except no longer using the connector, even in complex DP > > > > > > MST > > > > > > cases. > > > > > > >
Re: [Intel-gfx] [PATCH] drm/i915: Engine relative MMIO
On 13/05/2019 20:45, john.c.harri...@intel.com wrote:
From: John Harrison
With virtual engines, it is no longer possible to know which specific
physical engine a given request will be executed on at the time that
request is generated. This means that the request itself must be engine
agnostic - any direct register writes must be relative to the engine
and not absolute addresses.
The LRI command has support for engine relative addressing. However,
the mechanism is not transparent to the driver. The scheme for Gen11
(MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
absolute engine base component. The hardware then adds on the correct
engine offset at execution time.
Due to the non-trivial and differing schemes on different hardware, it
is not possible to simply update the code that creates the LRI
commands to set a remap flag and let the hardware get on with it.
Instead, this patch adds function wrappers for generating the LRI
command itself and then for constructing the correct address to use
with the LRI.
v2: Fix build break in GVT. Remove flags parameter [review feedback
from Chris W].
v3: Fix build break in selftest. Rebase to newer base tree and fix
merge conflict.
v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
code paths [review feedback from Chris W].
v5: More rebasing (new 'gt' directory). Fix white space issue. Use
COPY class rather than BCS0 id for checking against BCS engine.
Signed-off-by: John Harrison
CC: Rodrigo Vivi
CC: Tvrtko Ursulin
CC: Wilson, Chris P
---
drivers/gpu/drm/i915/gt/intel_engine.h| 4 +
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 +++
drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 6 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 79 ++-
drivers/gpu/drm/i915/gt/intel_lrc_reg.h | 4 +-
drivers/gpu/drm/i915/gt/intel_mocs.c | 17 ++--
drivers/gpu/drm/i915/gt/intel_ringbuffer.c| 45 +--
drivers/gpu/drm/i915/gt/intel_workarounds.c | 4 +-
.../gpu/drm/i915/gt/selftest_workarounds.c| 9 ++-
drivers/gpu/drm/i915/gvt/mmio_context.c | 16 +++-
drivers/gpu/drm/i915/i915_cmd_parser.c| 4 +-
drivers/gpu/drm/i915/i915_gem_context.c | 12 +--
drivers/gpu/drm/i915/i915_gem_execbuffer.c| 3 +-
drivers/gpu/drm/i915/i915_perf.c | 19 +++--
14 files changed, 154 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
b/drivers/gpu/drm/i915/gt/intel_engine.h
index 9359b3a7ad9c..3506c992182c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -546,4 +546,8 @@ static inline bool inject_preempt_hang(struct
intel_engine_execlists *execlists)
#endif
+bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine);
+u32 i915_get_lri_cmd(const struct intel_engine_cs *engine, u32 word_count);
+u32 i915_get_lri_reg(const struct intel_engine_cs *engine, i915_reg_t reg);
+
#endif /* _INTEL_RINGBUFFER_H_ */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 4c3753c1b573..233295d689d2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -253,6 +253,17 @@ static u32 __engine_mmio_base(struct drm_i915_private
*i915,
return bases[i].base;
}
+bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine)
+{
+ if (INTEL_GEN(engine->i915) < 11)
+ return false;
+
+ if (engine->class == COPY_ENGINE_CLASS)
+ return false;
+
+ return true;
I think engine->flags would be better. At least it is one conditional
instead of two at runtime, even one too many for something that is
invariant.
+}
+
static void __sprint_engine_name(char *name, const struct engine_info *info)
{
WARN_ON(snprintf(name, INTEL_ENGINE_CS_MAX_NAME, "%s%u",
diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index a34ece53a771..e7784b3fb759 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -123,9 +123,13 @@
* simply ignores the register load under certain conditions.
* - One can actually load arbitrary many arbitrary registers: Simply issue x
* address/value pairs. Don't overdue it, though, x <= 2^4 must hold!
+ * - Newer hardware supports engine relative addresses but older hardware does
+ * not. So never call MI_LRI directly, always use the i915_get_lri_cmd()
+ * and i915_get_lri_reg() helper functions.
*/
-#define MI_LOAD_REGISTER_IMM(x)MI_INSTR(0x22, 2*(x)-1)
+#define __MI_LOAD_REGISTER_IMM(x) MI_INSTR(0x22, 2*(x)-1)
So we end up with code using __MI_LOAD_REGISTER_IMM for absolute, and
i915_get_lri_cmd for relative addressing. One is a macro, another is a
function call, and naming/case is different.
How about we static inline it li
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling in DRM layer (rev10)
>-Original Message-
>From: Patchwork [mailto:patchw...@emeril.freedesktop.org]
>Sent: Wednesday, May 15, 2019 6:54 AM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org
>Subject: ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling in DRM
>layer
>(rev10)
>
>== Series Details ==
>
>Series: Add HDR Metadata Parsing and handling in DRM layer (rev10)
>URL : https://patchwork.freedesktop.org/series/25091/
>State : failure
>
>== Summary ==
>
>CI Bug Log - changes from CI_DRM_6081_full -> Patchwork_13017_full
>
>
>Summary
>---
>
> **FAILURE**
>
> Serious unknown changes coming with Patchwork_13017_full absolutely need to
> be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_13017_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_13017_full:
>
>### IGT changes ###
>
> Possible regressions
>
> * igt@gem_exec_suspend@basic-s3:
>- shard-iclb: [PASS][1] -> [SKIP][2] +43 similar issues
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@gem_exec_susp...@basic-s3.html
> [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@gem_exec_susp...@basic-s3.html
>
> * igt@kms_prop_blob@invalid-set-prop-any:
>- shard-iclb: [PASS][3] -> [FAIL][4]
> [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@kms_prop_b...@invalid-set-prop-any.html
> [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@kms_prop_b...@invalid-set-prop-any.html
>
Hi Martin,
These issues are unrelated to the changes made in this series. Can you please
have a look
and confirm.
Regards,
Uma Shankar
> Warnings
>
> * igt@gem_mocs_settings@mocs-settings-ctx-dirty-render:
>- shard-iclb: [SKIP][5] ([fdo#110206]) -> [SKIP][6]
> [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@gem_mocs_setti...@mocs-settings-ctx-dirty-render.html
> [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@gem_mocs_setti...@mocs-settings-ctx-dirty-render.html
>
> * igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
>- shard-iclb: [SKIP][7] ([fdo#109278]) -> [SKIP][8] +1 similar
> issue
> [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@kms_atomic_transit...@3x-modeset-transitions-nonblocking.html
> [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@kms_atomic_transit...@3x-modeset-transitions-nonblocking.html
>
> * igt@kms_chamelium@hdmi-crc-abgr:
>- shard-iclb: [SKIP][9] ([fdo#109284]) -> [SKIP][10]
> [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@kms_chamel...@hdmi-crc-abgr.html
> [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@kms_chamel...@hdmi-crc-abgr.html
>
> * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-blt:
>- shard-iclb: [SKIP][11] ([fdo#109280]) -> [SKIP][12] +6 similar
> issues
> [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@kms_frontbuffer_track...@psr-2p-primscrn-pri-shrfb-draw-blt.html
> [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@kms_frontbuffer_track...@psr-2p-primscrn-pri-shrfb-draw-blt.html
>
>
> Suppressed
>
> The following results come from untrusted machines, tests, or statuses.
> They do not affect the overall result.
>
> * {igt@gem_exec_schedule@semaphore-resolve}:
>- shard-iclb: [FAIL][13] ([fdo#110519]) -> [SKIP][14]
> [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@gem_exec_sched...@semaphore-resolve.html
> [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@gem_exec_sched...@semaphore-resolve.html
>
> * {igt@kms_cursor_crc@pipe-b-cursor-64x21-random}:
>- shard-iclb: [PASS][15] -> [SKIP][16] +2 similar issues
> [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>iclb6/igt@kms_cursor_...@pipe-b-cursor-64x21-random.html
> [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
>iclb5/igt@kms_cursor_...@pipe-b-cursor-64x21-random.html
>
>
>Known issues
>
>
> Here are the changes found in Patchwork_13017_full that come from known
> issues:
>
>### IGT changes ###
>
> Issues hit
>
> * igt@gem_eio@in-flight-suspend:
>- shard-apl: [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +3
> similar issues
> [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
>apl5/igt@gem_...@in-flight-suspend.html
> [18]: https://intel-gfx-ci.01.org/tree/drm-
Re: [Intel-gfx] [PATCH] drm/i915/perf: Refactor oa object to better manage resources
On 14/05/2019 19:14, Umesh Nerlige Ramappa wrote:
On Tue, May 14, 2019 at 10:34:49AM +0100, Lionel Landwerlin wrote:
Hi Umesh,
I just noticed this different between v1 & v2.
My understanding is that if destroy() is called, stream should be the
same as dev_priv->perf.exclusive_stream.
If it's not it sounds like a bug. So why change this?
v2 fixes only checkpatch warnings. it warned on use of BUG_ON. BUG_ON
is intended to crash the system in severe cases where the
driver/kernel is unusable. In this case, the mismatch between user
passed information and exclusive_stream may not require a crash.
This is called from i915_perf_release() which is attached to the
i915-perf file descriptor only in i915_perf.c.
If we managed to reach that function it must be because the file
descriptor given by userspace is associated to the i915-perf stream.
Having stream != dev_priv->perf.exclusive_stream means that we probably
screwed up the locking somewhere in this file.
So I would argue this is a kernel issue, not a user issue and that using
BUG_ON() is justified.
Thanks,
-Lionel
-Lionel
On 03/05/2019 00:13, Umesh Nerlige Ramappa wrote:
static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
{
struct drm_i915_private *dev_priv = stream->dev_priv;
- BUG_ON(stream != dev_priv->perf.oa.exclusive_stream);
+ if (stream != dev_priv->perf.exclusive_stream) {
+ WARN_ON_ONCE(stream != dev_priv->perf.exclusive_stream);
+ return;
+ }
/*
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 09/11] drm/fb-helper: Prepare to move out modeset config code
Hi Noralf On Mon, May 06, 2019 at 08:01:37PM +0200, Noralf Trønnes wrote: > This prepares the modeset code so it can be moved out as-is in the next > patch. > > v3: Remove stray newline > > Signed-off-by: Noralf Trønnes > Reviewed-by: Maxime Ripard Reviewed-by: Sam Ravnborg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 07/11] drm/fb-helper: Move out commit code
Hi Noralf. On Mon, May 06, 2019 at 08:01:35PM +0200, Noralf Trønnes wrote: > Move the modeset commit code to drm_client_modeset. > No changes except exporting API. > > v2: Move to drm_client_modeset.c instead of drm_client.c > > Signed-off-by: Noralf Trønnes Reviewed-by: Sam Ravnborg Assumes this is a one-to-one copy. I did not check it line-by-line. Should git be able to tell this with proper options? Sam ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 06/11] drm/fb-helper: Prepare to move out commit code
Hi Noralf. On Mon, May 06, 2019 at 08:01:34PM +0200, Noralf Trønnes wrote: > This makes the necessary changes so the commit code can be moved out to > drm_client as-is in the next patch. It's split up to ease review. > > Signed-off-by: Noralf Trønnes Reviewed-by: Sam Ravnborg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/11] drm/fb-helper: Remove drm_fb_helper_crtc
Hi Noralf.
I have read through the cahnes a copuple of times not and feel confident
to add my r-b if the comments are considered.
On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
> It now only contains the modeset so use that directly instead and attach
> a modeset array to drm_client_dev. drm_fb_helper will use this array.
> Code will later be moved to drm_client, so add code there in a new file
> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
The first part of this commit log could use some re-pharsing.
What is "It" etc.
> @@ -92,14 +91,20 @@ int drm_client_init(struct drm_device *dev, struct
> drm_client_dev *client,
> client->name = name;
> client->funcs = funcs;
>
> - ret = drm_client_open(client);
> + ret = drm_client_modeset_create(client);
> if (ret)
> goto err_put_module;
>
> + ret = drm_client_open(client);
> + if (ret)
> + goto err_free;
> +
> drm_dev_get(dev);
>
> return 0;
>
> +err_free:
> + drm_client_modeset_free(client);
> err_put_module:
> if (funcs)
> module_put(funcs->owner);
> @@ -148,6 +153,7 @@ void drm_client_release(struct drm_client_dev *client)
>
> DRM_DEV_DEBUG_KMS(dev->dev, "%s\n", client->name);
>
> + drm_client_modeset_free(client);
> drm_client_close(client);
> drm_dev_put(dev);
If the order should be the opposite of init then call drm_client_close() before
drm_client_modeset_free()
> if (client->funcs)
> + drm_client_for_each_modeset(mode_set, client) {
> struct drm_plane *primary = mode_set->crtc->primary;
> unsigned int rotation;
>
> @@ -517,9 +518,11 @@ static int restore_fbdev_mode_atomic(struct
> drm_fb_helper *fb_helper, bool activ
>
> static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
> {
> + struct drm_client_dev *client = &fb_helper->client;
> struct drm_device *dev = fb_helper->dev;
> + struct drm_mode_set *mode_set;
> struct drm_plane *plane;
> - int i, ret = 0;
> + int ret = 0;
>
> drm_modeset_lock_all(fb_helper->dev);
> drm_for_each_plane(plane, dev) {
> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper
> *fb_helper)
> DRM_MODE_ROTATE_0);
> }
>
> - for (i = 0; i < fb_helper->crtc_count; i++) {
> - struct drm_mode_set *mode_set =
> &fb_helper->crtc_info[i].mode_set;
> + drm_client_for_each_modeset(mode_set, client) {
> struct drm_crtc *crtc = mode_set->crtc;
>
> if (crtc->funcs->cursor_set2) {
This function requires modeset_mutex to be held. Maybe add comment?
> @@ -682,15 +689,14 @@ static struct sysrq_key_op
> sysrq_drm_fb_helper_restore_op = { };
>
> static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
> {
> + struct drm_client_dev *client = &fb_helper->client;
> struct drm_device *dev = fb_helper->dev;
> struct drm_connector *connector;
> struct drm_mode_set *modeset;
> - int i, j;
> + int j;
>
> drm_modeset_lock_all(dev);
> - for (i = 0; i < fb_helper->crtc_count; i++) {
> - modeset = &fb_helper->crtc_info[i].mode_set;
> -
> + drm_client_for_each_modeset(modeset, client) {
> if (!modeset->crtc->enabled)
> continue;
>
This function needs modeset_mutex - maybe add comment?
> @@ -1390,13 +1353,14 @@ static int setcmap_pseudo_palette(struct fb_cmap
> *cmap, struct fb_info *info)
> static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
> {
> struct drm_fb_helper *fb_helper = info->par;
> + struct drm_mode_set *modeset;
> struct drm_crtc *crtc;
> u16 *r, *g, *b;
> - int i, ret = 0;
> + int ret = 0;
>
> drm_modeset_lock_all(fb_helper->dev);
> - for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_info[i].mode_set.crtc;
> + drm_client_for_each_modeset(modeset, &fb_helper->client) {
> + crtc = modeset->crtc;
> if (!crtc->funcs->gamma_set || !crtc->gamma_size)
> return -EINVAL;
>
This function needs modeset_mutex - maybe add comment?
> @@ -1472,10 +1436,11 @@ static int setcmap_atomic(struct fb_cmap *cmap,
> struct fb_info *info)
> struct drm_modeset_acquire_ctx ctx;
> struct drm_crtc_state *crtc_state;
> struct drm_atomic_state *state;
> + struct drm_mode_set *modeset;
> struct drm_crtc *crtc;
> u16 *r, *g, *b;
> - int i, ret = 0;
> bool replaced;
> + int ret = 0;
>
> drm_modeset_acquire_init(&ctx, 0);
>
> @@ -1487,8 +1452,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct
> fb_info *info)
>
> state->acquire_ctx = &ctx;
> retry:
> - for (i = 0; i < fb_helper->crtc_count; i++) {
> - crtc = fb_helper->crtc_i
Re: [Intel-gfx] [RFC 1/2] drm/i915: start moving state checker to intel_verify.c
On Tue, Apr 16, 2019 at 01:36:02PM +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/Makefile| 1 +
> drivers/gpu/drm/i915/intel_display.c | 474 +--
> drivers/gpu/drm/i915/intel_drv.h | 12 +
> drivers/gpu/drm/i915/intel_verify.c | 464 ++
> drivers/gpu/drm/i915/intel_verify.h | 22 ++
> 5 files changed, 510 insertions(+), 463 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_verify.c
> create mode 100644 drivers/gpu/drm/i915/intel_verify.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index fbcb0904..a000fad 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -166,6 +166,7 @@ i915-y += dvo_ch7017.o \
> intel_panel.o \
> intel_sdvo.o \
> intel_tv.o \
> + intel_verify.o \
> vlv_dsi.o \
> vlv_dsi_pll.o \
> intel_vdsc.o
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3bd40a..31a931 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -69,6 +69,7 @@
> #include "intel_sdvo.h"
> #include "intel_sprite.h"
> #include "intel_tv.h"
> +#include "intel_verify.h"
>
> /* Primary plane formats for gen <= 3 */
> static const u32 i8xx_primary_formats[] = {
> @@ -1243,7 +1244,7 @@ void assert_pipe(struct drm_i915_private *dev_priv,
> pipe_name(pipe), onoff(state), onoff(cur_state));
> }
>
> -static void assert_plane(struct intel_plane *plane, bool state)
> +void assert_plane(struct intel_plane *plane, bool state)
> {
> enum pipe pipe;
> bool cur_state;
> @@ -6607,45 +6608,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder)
> kfree(intel_encoder);
> }
>
> -/* Cross check the actual hw state with our own modeset state tracking (and
> it's
> - * internal consistency). */
> -static void intel_connector_verify_state(struct drm_crtc_state *crtc_state,
> - struct drm_connector_state *conn_state)
> -{
> - struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> -
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> - connector->base.base.id,
> - connector->base.name);
> -
> - if (connector->get_hw_state(connector)) {
> - struct intel_encoder *encoder = connector->encoder;
> -
> - I915_STATE_WARN(!crtc_state,
> - "connector enabled without attached crtc\n");
> -
> - if (!crtc_state)
> - return;
> -
> - I915_STATE_WARN(!crtc_state->active,
> - "connector is active, but attached crtc isn't\n");
> -
> - if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST)
> - return;
> -
> - I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
> - "atomic encoder doesn't match attached encoder\n");
> -
> - I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
> - "attached encoder crtc differs from connector crtc\n");
> - } else {
> - I915_STATE_WARN(crtc_state && crtc_state->active,
> - "attached crtc is active, but connector isn't\n");
> - I915_STATE_WARN(!crtc_state && conn_state->best_encoder,
> - "best encoder set without crtc!\n");
> - }
> -}
> -
> static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
> {
> if (crtc_state->base.enable && crtc_state->has_pch_encoder)
> @@ -6879,7 +6841,7 @@ static u32 ilk_pipe_pixel_rate(const struct
> intel_crtc_state *pipe_config)
> return pixel_rate;
> }
>
> -static void intel_crtc_compute_pixel_rate(struct intel_crtc_state
> *crtc_state)
> +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>
> @@ -11584,9 +11546,9 @@ static const char *output_formats(enum
> intel_output_format format)
> return output_format_str[format];
> }
>
> -static void intel_dump_pipe_config(struct intel_crtc *crtc,
> -struct intel_crtc_state *pipe_config,
> -const char *context)
> +void intel_dump_pipe_config(struct intel_crtc *crtc,
> + struct intel_crtc_state *pipe_config,
> + const char *context)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -12076,7 +12038,7 @@ static bool fastboot_enabled(struct drm_i915_private
> *dev_priv)
> return false;
> }
>
> -static bool
> +bool
> intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> struct intel_crtc_state *current_config,
>
Re: [Intel-gfx] [RFC 2/2] drm/i915: move pipe config compare to intel_verify.c
On Tue, Apr 16, 2019 at 01:36:03PM +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula
pipe_config_compare is used both for the atomic_check logic, and the
verifier. I think stuffing it into the verifier code is misplacing it as
much as e.g. stuffing it into intel_atomic_check.c file.
What I'd do instead is create an intel_crtc_state.[hc] pair and collect
everything that operates on struct intel_crtc_state, plus the struct
itself. Of course we might want to split out specific subtopics from this,
like e.g. the fairly massive amount of code related to compute clock
state.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 471 +--
> drivers/gpu/drm/i915/intel_drv.h | 9 +-
> drivers/gpu/drm/i915/intel_verify.c | 465 ++
> drivers/gpu/drm/i915/intel_verify.h | 8 +
> 4 files changed, 478 insertions(+), 475 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 31a931..14899f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -202,9 +202,9 @@ static void intel_update_czclk(struct drm_i915_private
> *dev_priv)
> DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> }
>
> -static inline u32 /* units of 100MHz */
> -intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> - const struct intel_crtc_state *pipe_config)
> +/* units of 100MHz */
> +u32 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> + const struct intel_crtc_state *pipe_config)
> {
> if (HAS_DDI(dev_priv))
> return pipe_config->port_clock; /* SPLL */
> @@ -11904,471 +11904,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> return 0;
> }
>
> -static bool intel_fuzzy_clock_check(int clock1, int clock2)
> -{
> - int diff;
> -
> - if (clock1 == clock2)
> - return true;
> -
> - if (!clock1 || !clock2)
> - return false;
> -
> - diff = abs(clock1 - clock2);
> -
> - if (diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105)
> - return true;
> -
> - return false;
> -}
> -
> -static bool
> -intel_compare_m_n(unsigned int m, unsigned int n,
> - unsigned int m2, unsigned int n2,
> - bool exact)
> -{
> - if (m == m2 && n == n2)
> - return true;
> -
> - if (exact || !m || !n || !m2 || !n2)
> - return false;
> -
> - BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
> -
> - if (n > n2) {
> - while (n > n2) {
> - m2 <<= 1;
> - n2 <<= 1;
> - }
> - } else if (n < n2) {
> - while (n < n2) {
> - m <<= 1;
> - n <<= 1;
> - }
> - }
> -
> - if (n != n2)
> - return false;
> -
> - return intel_fuzzy_clock_check(m, m2);
> -}
> -
> -static bool
> -intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> -struct intel_link_m_n *m2_n2,
> -bool adjust)
> -{
> - if (m_n->tu == m2_n2->tu &&
> - intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> - m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> - intel_compare_m_n(m_n->link_m, m_n->link_n,
> - m2_n2->link_m, m2_n2->link_n, !adjust)) {
> - if (adjust)
> - *m2_n2 = *m_n;
> -
> - return true;
> - }
> -
> - return false;
> -}
> -
> -static bool
> -intel_compare_infoframe(const union hdmi_infoframe *a,
> - const union hdmi_infoframe *b)
> -{
> - return memcmp(a, b, sizeof(*a)) == 0;
> -}
> -
> -static void
> -pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
> - bool adjust, const char *name,
> - const union hdmi_infoframe *a,
> - const union hdmi_infoframe *b)
> -{
> - if (adjust) {
> - if ((drm_debug & DRM_UT_KMS) == 0)
> - return;
> -
> - drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
> - drm_dbg(DRM_UT_KMS, "expected:");
> - hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
> - drm_dbg(DRM_UT_KMS, "found");
> - hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
> - } else {
> - drm_err("mismatch in %s infoframe", name);
> - drm_err("expected:");
> - hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
> - drm_err("found");
> - hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
> - }
> -}
> -
> -static void __printf(3, 4)
> -pipe_config_err(bool adjust, const char *name, const char *format, ...)
> -{
> - struct va_format vaf;
> - va_list args;
> -
> - va_start(args, format);
> - vaf.fmt = format;
> - vaf.va = &args;
> -
>
Re: [Intel-gfx] [RFC 1/2] drm/i915: start moving state checker to intel_verify.c
On Wed, May 15, 2019 at 11:33:31AM +0200, Daniel Vetter wrote:
> On Tue, Apr 16, 2019 at 01:36:02PM +0300, Jani Nikula wrote:
> > Signed-off-by: Jani Nikula
> > ---
> > drivers/gpu/drm/i915/Makefile| 1 +
> > drivers/gpu/drm/i915/intel_display.c | 474 +--
> > drivers/gpu/drm/i915/intel_drv.h | 12 +
> > drivers/gpu/drm/i915/intel_verify.c | 464 ++
> > drivers/gpu/drm/i915/intel_verify.h | 22 ++
> > 5 files changed, 510 insertions(+), 463 deletions(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_verify.c
> > create mode 100644 drivers/gpu/drm/i915/intel_verify.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index fbcb0904..a000fad 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -166,6 +166,7 @@ i915-y += dvo_ch7017.o \
> > intel_panel.o \
> > intel_sdvo.o \
> > intel_tv.o \
> > + intel_verify.o \
> > vlv_dsi.o \
> > vlv_dsi_pll.o \
> > intel_vdsc.o
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3bd40a..31a931 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -69,6 +69,7 @@
> > #include "intel_sdvo.h"
> > #include "intel_sprite.h"
> > #include "intel_tv.h"
> > +#include "intel_verify.h"
> >
> > /* Primary plane formats for gen <= 3 */
> > static const u32 i8xx_primary_formats[] = {
> > @@ -1243,7 +1244,7 @@ void assert_pipe(struct drm_i915_private *dev_priv,
> > pipe_name(pipe), onoff(state), onoff(cur_state));
> > }
> >
> > -static void assert_plane(struct intel_plane *plane, bool state)
> > +void assert_plane(struct intel_plane *plane, bool state)
> > {
> > enum pipe pipe;
> > bool cur_state;
> > @@ -6607,45 +6608,6 @@ void intel_encoder_destroy(struct drm_encoder
> > *encoder)
> > kfree(intel_encoder);
> > }
> >
> > -/* Cross check the actual hw state with our own modeset state tracking
> > (and it's
> > - * internal consistency). */
> > -static void intel_connector_verify_state(struct drm_crtc_state *crtc_state,
> > -struct drm_connector_state *conn_state)
> > -{
> > - struct intel_connector *connector =
> > to_intel_connector(conn_state->connector);
> > -
> > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > - connector->base.base.id,
> > - connector->base.name);
> > -
> > - if (connector->get_hw_state(connector)) {
> > - struct intel_encoder *encoder = connector->encoder;
> > -
> > - I915_STATE_WARN(!crtc_state,
> > -"connector enabled without attached crtc\n");
> > -
> > - if (!crtc_state)
> > - return;
> > -
> > - I915_STATE_WARN(!crtc_state->active,
> > - "connector is active, but attached crtc isn't\n");
> > -
> > - if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST)
> > - return;
> > -
> > - I915_STATE_WARN(conn_state->best_encoder != &encoder->base,
> > - "atomic encoder doesn't match attached encoder\n");
> > -
> > - I915_STATE_WARN(conn_state->crtc != encoder->base.crtc,
> > - "attached encoder crtc differs from connector crtc\n");
> > - } else {
> > - I915_STATE_WARN(crtc_state && crtc_state->active,
> > - "attached crtc is active, but connector isn't\n");
> > - I915_STATE_WARN(!crtc_state && conn_state->best_encoder,
> > - "best encoder set without crtc!\n");
> > - }
> > -}
> > -
> > static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
> > {
> > if (crtc_state->base.enable && crtc_state->has_pch_encoder)
> > @@ -6879,7 +6841,7 @@ static u32 ilk_pipe_pixel_rate(const struct
> > intel_crtc_state *pipe_config)
> > return pixel_rate;
> > }
> >
> > -static void intel_crtc_compute_pixel_rate(struct intel_crtc_state
> > *crtc_state)
> > +void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> > {
> > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> >
> > @@ -11584,9 +11546,9 @@ static const char *output_formats(enum
> > intel_output_format format)
> > return output_format_str[format];
> > }
> >
> > -static void intel_dump_pipe_config(struct intel_crtc *crtc,
> > - struct intel_crtc_state *pipe_config,
> > - const char *context)
> > +void intel_dump_pipe_config(struct intel_crtc *crtc,
> > + struct intel_crtc_state *pipe_config,
> > + const char *context)
> > {
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > @@ -12076,7 +12038,7 @@ static bool fastboot_enabled(struct
> > drm_i9
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: drop unneeded -Wall addition
== Series Details == Series: drm/i915: drop unneeded -Wall addition URL : https://patchwork.freedesktop.org/series/60652/ State : success == Summary == CI Bug Log - changes from CI_DRM_6085_full -> Patchwork_13019_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13019_full that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@read_all_entries_display_off: - shard-skl: [PASS][1] -> [INCOMPLETE][2] ([fdo#104108]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl10/igt@debugfs_test@read_all_entries_display_off.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-skl5/igt@debugfs_test@read_all_entries_display_off.html * igt@gem_eio@in-flight-suspend: - shard-apl: [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-apl1/igt@gem_...@in-flight-suspend.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-apl3/igt@gem_...@in-flight-suspend.html * igt@gem_tiled_swapping@non-threaded: - shard-hsw: [PASS][5] -> [FAIL][6] ([fdo#108686]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-hsw6/igt@gem_tiled_swapp...@non-threaded.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-hsw2/igt@gem_tiled_swapp...@non-threaded.html * igt@i915_pm_rpm@gem-execbuf-stress: - shard-skl: [PASS][7] -> [INCOMPLETE][8] ([fdo#107803] / [fdo#107807]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl2/igt@i915_pm_...@gem-execbuf-stress.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-skl4/igt@i915_pm_...@gem-execbuf-stress.html * igt@i915_pm_rpm@gem-pread: - shard-skl: [PASS][9] -> [INCOMPLETE][10] ([fdo#107807]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl8/igt@i915_pm_...@gem-pread.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-skl6/igt@i915_pm_...@gem-pread.html * igt@kms_atomic_transition@plane-all-transition-nonblocking: - shard-iclb: [PASS][11] -> [INCOMPLETE][12] ([fdo#107713]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb1/igt@kms_atomic_transit...@plane-all-transition-nonblocking.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-iclb7/igt@kms_atomic_transit...@plane-all-transition-nonblocking.html * igt@kms_flip@flip-vs-expired-vblank-interruptible: - shard-skl: [PASS][13] -> [FAIL][14] ([fdo#105363]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl7/igt@kms_f...@flip-vs-expired-vblank-interruptible.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-skl5/igt@kms_f...@flip-vs-expired-vblank-interruptible.html * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite: - shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167]) +2 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb5/igt@kms_frontbuffer_track...@fbc-rgb565-draw-pwrite.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-iclb3/igt@kms_frontbuffer_track...@fbc-rgb565-draw-pwrite.html * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping: - shard-iclb: [PASS][17] -> [INCOMPLETE][18] ([fdo#107713] / [fdo#110036 ]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb8/igt@kms_pl...@pixel-format-pipe-c-planes-source-clamping.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-iclb7/igt@kms_pl...@pixel-format-pipe-c-planes-source-clamping.html * igt@kms_psr@psr2_cursor_plane_move: - shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-iclb7/igt@kms_psr@psr2_cursor_plane_move.html * igt@kms_psr@suspend: - shard-skl: [PASS][21] -> [INCOMPLETE][22] ([fdo#107773]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl3/igt@kms_...@suspend.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-skl3/igt@kms_...@suspend.html Possible fixes * igt@gem_eio@in-flight-suspend: - shard-kbl: [FAIL][23] ([fdo#110667]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-kbl2/igt@gem_...@in-flight-suspend.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13019/shard-kbl3/igt@gem_...@in-flight-suspend.html * igt@gem_exec_suspend@basic-s3-devices: - shard-glk: [INCOMPLETE][25] ([fdo#103359] / [k.org#198133]) -> [PASS][26] [2
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling in DRM layer (rev10)
+ Martin for real.
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Shankar, Uma
> Sent: keskiviikko 15. toukokuuta 2019 12.00
> To: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and
> handling in DRM layer (rev10)
>
>
>
> >-Original Message-
> >From: Patchwork [mailto:patchw...@emeril.freedesktop.org]
> >Sent: Wednesday, May 15, 2019 6:54 AM
> >To: Shankar, Uma
> >Cc: intel-gfx@lists.freedesktop.org
> >Subject: ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling
> >in DRM layer
> >(rev10)
> >
> >== Series Details ==
> >
> >Series: Add HDR Metadata Parsing and handling in DRM layer (rev10)
> >URL : https://patchwork.freedesktop.org/series/25091/
> >State : failure
> >
> >== Summary ==
> >
> >CI Bug Log - changes from CI_DRM_6081_full -> Patchwork_13017_full
> >
> >
> >Summary
> >---
> >
> > **FAILURE**
> >
> > Serious unknown changes coming with Patchwork_13017_full absolutely
> > need to be verified manually.
> >
> > If you think the reported changes have nothing to do with the changes
> > introduced in Patchwork_13017_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_13017_full:
> >
> >### IGT changes ###
> >
> > Possible regressions
> >
> > * igt@gem_exec_suspend@basic-s3:
> >- shard-iclb: [PASS][1] -> [SKIP][2] +43 similar issues
> > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@gem_exec_susp...@basic-s3.html
> > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@gem_exec_susp...@basic-s3.html
> >
> > * igt@kms_prop_blob@invalid-set-prop-any:
> >- shard-iclb: [PASS][3] -> [FAIL][4]
> > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@kms_prop_b...@invalid-set-prop-any.html
> > [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@kms_prop_b...@invalid-set-prop-any.html
> >
>
> Hi Martin,
> These issues are unrelated to the changes made in this series. Can you please
> have a
> look and confirm.
>
> Regards,
> Uma Shankar
>
> > Warnings
> >
> > * igt@gem_mocs_settings@mocs-settings-ctx-dirty-render:
> >- shard-iclb: [SKIP][5] ([fdo#110206]) -> [SKIP][6]
> > [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@gem_mocs_setti...@mocs-settings-ctx-dirty-render.html
> > [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@gem_mocs_setti...@mocs-settings-ctx-dirty-render.html
> >
> > * igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
> >- shard-iclb: [SKIP][7] ([fdo#109278]) -> [SKIP][8] +1 similar
> > issue
> > [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@kms_atomic_transit...@3x-modeset-transitions-nonblocking.html
> > [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@kms_atomic_transit...@3x-modeset-transitions-nonblocking.html
> >
> > * igt@kms_chamelium@hdmi-crc-abgr:
> >- shard-iclb: [SKIP][9] ([fdo#109284]) -> [SKIP][10]
> > [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@kms_chamel...@hdmi-crc-abgr.html
> > [10]:
> >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@kms_chamel...@hdmi-crc-abgr.html
> >
> > * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-blt:
> >- shard-iclb: [SKIP][11] ([fdo#109280]) -> [SKIP][12] +6 similar
> > issues
> > [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@kms_frontbuffer_track...@psr-2p-primscrn-pri-shrfb-draw-blt.html
> > [12]:
> >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-blt.h
> >tml
> >
> >
> > Suppressed
> >
> > The following results come from untrusted machines, tests, or statuses.
> > They do not affect the overall result.
> >
> > * {igt@gem_exec_schedule@semaphore-resolve}:
> >- shard-iclb: [FAIL][13] ([fdo#110519]) -> [SKIP][14]
> > [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@gem_exec_sched...@semaphore-resolve.html
> > [14]:
> >https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-
> >iclb5/igt@gem_exec_sched...@semaphore-resolve.html
> >
> > * {igt@kms_cursor_crc@pipe-b-cursor-64x21-random}:
> >- shard-iclb: [PASS][15] -> [SKIP][16] +2 similar issues
> > [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-
> >iclb6/igt@kms_cursor_...@pipe-b-cursor-64x21-rando
Re: [Intel-gfx] [PATCH] drm/i915: drop unneeded -Wall addition
On Wed, May 15, 2019 at 3:25 PM Chris Wilson wrote: > > Quoting Masahiro Yamada (2019-05-15 05:37:53) > > The top level Makefile adds -Wall globally: > > > > KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs > > \ > > > > I see two "-Wall" added for compiling under drivers/gpu/drm/i915/. > > Does it matter? Is the statement in i915/Makefile not more complete for > saying "-Wall -Wextra -Werror"? Not fatal, but better to fix. Why not fix the comment if you mind "-Wall" in the comment? It will be easy to rephrase the comments without explicitly mentioning -Wall or -Wextra. I reworded it more concisely: # We aggressively eliminate warnings, # so here are more warning options than default. That's it. The CI is your local matter. Distracting comments should not be added in the upstream code in the first place. > > Signed-off-by: Masahiro Yamada > > --- > > > > BTW, I have a question in the comment: > > > > "Note the danger in using -Wall -Wextra is that when CI updates gcc we > > will most likely get a sudden build breakage... Hopefully we will fix > > new warnings before CI updates!" > > > > Enabling whatever warning options does not cause build breakage. > > -Werror does. > > > > So, I think the correct statement is: > > > > "Note the danger in using -Werror is that when CI updates gcc we ... > > No. Heh, I thought the answer was Yes, since I saw the following in this Makefile. # Add a set of useful warning flags and enable -Werror for CI to prevent > CI enforces -Werror and that is constant, so the uncontrolled > variable, the danger, lies in using the unreliable heuristics gcc may > arbitrary enable between versions. That the set of warnings causing an > error may be different between CI and the developer. > -Chris -- Best Regards Masahiro Yamada ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 13/16] i915: Add gem_exec_balancer
On 08/05/2019 11:09, Chris Wilson wrote:
Exercise the in-kernel load balancer checking that we can distribute
batches across the set of ctx->engines to avoid load.
Signed-off-by: Chris Wilson
---
tests/Makefile.am |1 +
tests/Makefile.sources |1 +
tests/i915/gem_exec_balancer.c | 1050
tests/meson.build |7 +
4 files changed, 1059 insertions(+)
create mode 100644 tests/i915/gem_exec_balancer.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5097debf6..c6af0aeaf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -96,6 +96,7 @@ gem_close_race_LDADD = $(LDADD) -lpthread
gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_ctx_thrash_LDADD = $(LDADD) -lpthread
gem_ctx_sseu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
+i915_gem_exec_balancer_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
gem_exec_capture_LDADD = $(LDADD) -lz
gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_exec_parallel_LDADD = $(LDADD) -lpthread
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index e7ee27e81..323b625aa 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -24,6 +24,7 @@ TESTS_progs = \
i915/gem_ctx_clone \
i915/gem_ctx_engines \
i915/gem_ctx_shared \
+ i915/gem_exec_balancer \
i915/gem_vm_create \
kms_3d \
kms_addfb_basic \
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
new file mode 100644
index 0..25195d478
--- /dev/null
+++ b/tests/i915/gem_exec_balancer.c
@@ -0,0 +1,1050 @@
+/*
+ * Copyright © 2018 Intel Corporation
2019 I guess, even though work was started in 2018?
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include
+
+#include "igt.h"
+#include "igt_perf.h"
+#include "i915/gem_ring.h"
+#include "sw_sync.h"
+
+IGT_TEST_DESCRIPTION("Exercise in-kernel load-balancing");
+
+#define INSTANCE_COUNT (1 << I915_PMU_SAMPLE_INSTANCE_BITS)
Hmm.. this is a strange surrogate but I guess it works.
+
+static bool has_class_instance(int i915, uint16_t class, uint16_t instance)
+{
+ int fd;
+
+ fd = perf_i915_open(I915_PMU_ENGINE_BUSY(class, instance));
More work for Andi to replace with real engine discovery. :)
+ if (fd != -1) {
+ close(fd);
+ return true;
+ }
+
+ return false;
+}
+
+static struct i915_engine_class_instance *
+list_engines(int i915, uint32_t class_mask, unsigned int *out)
+{
+ unsigned int count = 0, size = 64;
+ struct i915_engine_class_instance *engines;
+
+ engines = malloc(size * sizeof(*engines));
+ if (!engines) {
+ *out = 0;
+ return NULL;
+ }
+
+ for (enum drm_i915_gem_engine_class class = I915_ENGINE_CLASS_RENDER;
+class_mask;
+class++, class_mask >>= 1) {
+ if (!(class_mask & 1))
+ continue;
+
+ for (unsigned int instance = 0;
+instance < INSTANCE_COUNT;
+instance++) {
+if (!has_class_instance(i915, class, instance))
+continue;
+
+ if (count == size) {
+ struct i915_engine_class_instance *e;
+
+ size *= 2;
+ e = realloc(engines, size*sizeof(*engines));
+ if (!e) {
I'd just assert. On malloc as well.
+ *out = count;
+ return engines;
+ }
+
+ engines = e;
+ }
+
+ engines[count++] = (struct i915_engine_class_instance){
+ .engine_clas
Re: [Intel-gfx] [PATCH i-g-t 14/16] i915/gem_exec_balancer: Exercise bonded pairs
On 08/05/2019 11:09, Chris Wilson wrote:
The submit-fence + load_balancing apis allow for us to execute a named
pair of engines in parallel; that this by submitting a request to one
engine, we can then use the generated submit-fence to submit a second
request to another engine and have it execute at the same time.
Furthermore, by specifying bonded pairs, we can direct the virtual
engine to use a particular engine in parallel to the first request.
Signed-off-by: Chris Wilson
---
tests/i915/gem_exec_balancer.c | 234 +++--
1 file changed, 224 insertions(+), 10 deletions(-)
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 25195d478..20ad66727 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -98,9 +98,35 @@ list_engines(int i915, uint32_t class_mask, unsigned int
*out)
return engines;
}
+static int __set_engines(int i915, uint32_t ctx,
+const struct i915_engine_class_instance *ci,
+unsigned int count)
+{
+ I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, count);
+ struct drm_i915_gem_context_param p = {
+ .ctx_id = ctx,
+ .param = I915_CONTEXT_PARAM_ENGINES,
+ .size = sizeof(engines),
+ .value = to_user_pointer(&engines)
+ };
+
+ engines.extensions = 0;
+ memcpy(engines.engines, ci, sizeof(engines.engines));
+
+ return __gem_context_set_param(i915, &p);
+}
+
+static void set_engines(int i915, uint32_t ctx,
+ const struct i915_engine_class_instance *ci,
+ unsigned int count)
+{
+ igt_assert_eq(__set_engines(i915, ctx, ci, count), 0);
+}
+
static int __set_load_balancer(int i915, uint32_t ctx,
const struct i915_engine_class_instance *ci,
- unsigned int count)
+ unsigned int count,
+ void *ext)
{
I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, count);
I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1 + count);
@@ -113,6 +139,7 @@ static int __set_load_balancer(int i915, uint32_t ctx,
memset(&balancer, 0, sizeof(balancer));
balancer.base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
+ balancer.base.next_extension = to_user_pointer(ext);
igt_assert(count);
balancer.num_siblings = count;
@@ -131,9 +158,10 @@ static int __set_load_balancer(int i915, uint32_t ctx,
static void set_load_balancer(int i915, uint32_t ctx,
const struct i915_engine_class_instance *ci,
- unsigned int count)
+ unsigned int count,
+ void *ext)
{
- igt_assert_eq(__set_load_balancer(i915, ctx, ci, count), 0);
+ igt_assert_eq(__set_load_balancer(i915, ctx, ci, count, ext), 0);
}
static uint32_t load_balancer_create(int i915,
@@ -143,7 +171,7 @@ static uint32_t load_balancer_create(int i915,
uint32_t ctx;
ctx = gem_context_create(i915);
- set_load_balancer(i915, ctx, ci, count);
+ set_load_balancer(i915, ctx, ci, count, NULL);
return ctx;
}
@@ -288,6 +316,74 @@ static void invalid_balancer(int i915)
}
}
+static void invalid_bonds(int i915)
+{
+ I915_DEFINE_CONTEXT_ENGINES_BOND(bonds[16], 1);
+ I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1);
+ struct drm_i915_gem_context_param p = {
+ .ctx_id = gem_context_create(i915),
+ .param = I915_CONTEXT_PARAM_ENGINES,
+ .value = to_user_pointer(&engines),
+ .size = sizeof(engines),
+ };
+ uint32_t handle;
+ void *ptr;
+
+ memset(&engines, 0, sizeof(engines));
+ gem_context_set_param(i915, &p);
+
+ memset(bonds, 0, sizeof(bonds));
+ for (int n = 0; n < ARRAY_SIZE(bonds); n++) {
+ bonds[n].base.name = I915_CONTEXT_ENGINES_EXT_BOND;
+ bonds[n].base.next_extension =
+ n ? to_user_pointer(&bonds[n - 1]) : 0;
+ bonds[n].num_bonds = 1;
+ }
+ engines.extensions = to_user_pointer(&bonds);
+ gem_context_set_param(i915, &p);
+
+ bonds[0].base.next_extension = -1ull;
+ igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);
+
+ bonds[0].base.next_extension = to_user_pointer(&bonds[0]);
+ igt_assert_eq(__gem_context_set_param(i915, &p), -E2BIG);
+
+ engines.extensions = to_user_pointer(&bonds[1]);
+ igt_assert_eq(__gem_context_set_param(i915, &p), -E2BIG);
+ bonds[0].base.next_extension = 0;
+ gem_context_set_param(i915, &p);
+
+ handle = gem_create(i915, 4096 * 3);
+ ptr = gem_mmap__gtt(i915, handle, 4096 * 3, PROT_WRITE);
+ gem_close(i915, handle);
+
+ memcpy(ptr + 4096, &bonds[0], sizeof(bonds[0]
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 14/21] gem_wsim: Engine map load balance command
On 10/05/2019 14:31, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-08 13:10:51)
From: Tvrtko Ursulin
A new workload command for enabling a load balanced context map (aka
Virtual Engine). Example usage:
B.1
This turns on load balancing for context one, assuming it has already been
configured with an engine map. Only DEFAULT engine specifier can be used
with load balanced engine maps.
Restriction makes sense for keeping linenoise^W file format simple.
Signed-off-by: Tvrtko Ursulin
---
@@ -1172,6 +1210,8 @@ prepare_workload(unsigned int id, struct workload *wrk,
unsigned int flags)
if (ctx->engine_map) {
I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
ctx->engine_map_count + 1);
+ I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
+
ctx->engine_map_count);
struct drm_i915_gem_context_param param = {
.ctx_id = ctx_id,
.param = I915_CONTEXT_PARAM_ENGINES,
@@ -1179,7 +1219,25 @@ prepare_workload(unsigned int id, struct workload *wrk,
unsigned int flags)
.value = to_user_pointer(&set_engines),
};
- set_engines.extensions = 0;
+ if (ctx->wants_balance) {
+ set_engines.extensions =
+ to_user_pointer(&load_balance);
+
+ memset(&load_balance, 0, sizeof(load_balance));
+ load_balance.base.name =
+ I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
+ load_balance.num_siblings =
+ ctx->engine_map_count;
+
+ for (j = 0; j < ctx->engine_map_count; j++) {
+ load_balance.engines[j].engine_class =
+ I915_ENGINE_CLASS_VIDEO; /*
FIXME */
+ load_balance.engines[j].engine_instance
=
+ ctx->engine_map[j] - VCS1; /*
FIXME */
Ok, more fallout from fixing ctx->engine_map[] first?
Not sure I understand the question.
I am at the moment updating the series with review feedback and some
small thing here and there. When done with that I'll see if these VCS
hardcoded assumptions can be easily solved. Basically I will have a go
at integrating engine discovery which I think its definitely needed now
that I have added class based engine map building ability.
Regards,
Tvrtko
Otherwise looks fine.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 14/21] gem_wsim: Engine map load balance command
Quoting Tvrtko Ursulin (2019-05-15 12:44:41)
>
> On 10/05/2019 14:31, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-08 13:10:51)
> >> From: Tvrtko Ursulin
> >>
> >> A new workload command for enabling a load balanced context map (aka
> >> Virtual Engine). Example usage:
> >>
> >>B.1
> >>
> >> This turns on load balancing for context one, assuming it has already been
> >> configured with an engine map. Only DEFAULT engine specifier can be used
> >> with load balanced engine maps.
> >
> > Restriction makes sense for keeping linenoise^W file format simple.
> >
> >> Signed-off-by: Tvrtko Ursulin
> >> ---
> >> @@ -1172,6 +1210,8 @@ prepare_workload(unsigned int id, struct workload
> >> *wrk, unsigned int flags)
> >> if (ctx->engine_map) {
> >> I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
> >>
> >> ctx->engine_map_count + 1);
> >> +
> >> I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
> >> +
> >> ctx->engine_map_count);
> >> struct drm_i915_gem_context_param param = {
> >> .ctx_id = ctx_id,
> >> .param = I915_CONTEXT_PARAM_ENGINES,
> >> @@ -1179,7 +1219,25 @@ prepare_workload(unsigned int id, struct workload
> >> *wrk, unsigned int flags)
> >> .value = to_user_pointer(&set_engines),
> >> };
> >>
> >> - set_engines.extensions = 0;
> >> + if (ctx->wants_balance) {
> >> + set_engines.extensions =
> >> + to_user_pointer(&load_balance);
> >> +
> >> + memset(&load_balance, 0,
> >> sizeof(load_balance));
> >> + load_balance.base.name =
> >> +
> >> I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> >> + load_balance.num_siblings =
> >> + ctx->engine_map_count;
> >> +
> >> + for (j = 0; j < ctx->engine_map_count;
> >> j++) {
> >> +
> >> load_balance.engines[j].engine_class =
> >> + I915_ENGINE_CLASS_VIDEO;
> >> /* FIXME */
> >> +
> >> load_balance.engines[j].engine_instance =
> >> + ctx->engine_map[j] - VCS1;
> >> /* FIXME */
> >
> > Ok, more fallout from fixing ctx->engine_map[] first?
>
> Not sure I understand the question.
The proliferation of FIXME, the assumption of CLASS_VIDEO and an
impedance mismatch between engine_map and class:instance. Basically
those FIXME raise the question of what do you intend this to look like?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] kernel/locking/semaphore: use wake_q in up()
On Fri 2019-05-10 17:20:15, Daniel Vetter wrote:
> On Fri, May 10, 2019 at 11:28 AM Petr Mladek wrote:
> >
> > On Thu 2019-05-09 22:06:33, Daniel Vetter wrote:
> > > console_trylock, called from within printk, can be called from pretty
> > > much anywhere. Including try_to_wake_up. Note that this isn't common,
> > > usually the box is in pretty bad shape at that point already. But it
> > > really doesn't help when then lockdep jumps in and spams the logs,
> > > potentially obscuring the real backtrace we're really interested in.
> > > One case I've seen (slightly simplified backtrace):
> > >
> > > Fix this specific locking recursion by moving the wake_up_process out
> > > from under the semaphore.lock spinlock, using wake_q as recommended by
> > > Peter Zijlstra.
> >
> > It might make sense to mention also the optimization effect mentioned
> > by Peter.
> >
> > > diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> > > index 561acdd39960..7a6f33715688 100644
> > > --- a/kernel/locking/semaphore.c
> > > +++ b/kernel/locking/semaphore.c
> > > @@ -169,6 +169,14 @@ int down_timeout(struct semaphore *sem, long timeout)
> > > }
> > > EXPORT_SYMBOL(down_timeout);
> > >
> > > +/* Functions for the contended case */
> > > +
> > > +struct semaphore_waiter {
> > > + struct list_head list;
> > > + struct task_struct *task;
> > > + bool up;
> > > +};
> > > +
> > > /**
> > > * up - release the semaphore
> > > * @sem: the semaphore to release
> > > @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> > > void up(struct semaphore *sem)
> > > {
> > > unsigned long flags;
> > > + struct semaphore_waiter *waiter;
> > > + DEFINE_WAKE_Q(wake_q);
> >
> > We need to call wake_q_init(&wake_q) to make sure that
> > it is empty.
>
> DEFINE_WAKE_Q does that already, and if it didn't, I'd wonder how I
> managed to boot with this patch. console_lock is usally terribly
> contented because thanks to fbcon we must do a full display modeset
> while holding it, which takes forever. As long as anyone printks
> meanwhile (guaranteed while loading drivers really) you have
> contention.
> -Daniel
You are right. It is initialized by DEFINE_WAKE_Q.
The patch looks correct to me then:
Reviewed-by: Petr Mladek
Best Regards,
Petr
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 14/21] gem_wsim: Engine map load balance command
On 15/05/2019 12:48, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-15 12:44:41)
On 10/05/2019 14:31, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-08 13:10:51)
From: Tvrtko Ursulin
A new workload command for enabling a load balanced context map (aka
Virtual Engine). Example usage:
B.1
This turns on load balancing for context one, assuming it has already been
configured with an engine map. Only DEFAULT engine specifier can be used
with load balanced engine maps.
Restriction makes sense for keeping linenoise^W file format simple.
Signed-off-by: Tvrtko Ursulin
---
@@ -1172,6 +1210,8 @@ prepare_workload(unsigned int id, struct workload *wrk,
unsigned int flags)
if (ctx->engine_map) {
I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
ctx->engine_map_count + 1);
+ I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
+
ctx->engine_map_count);
struct drm_i915_gem_context_param param = {
.ctx_id = ctx_id,
.param = I915_CONTEXT_PARAM_ENGINES,
@@ -1179,7 +1219,25 @@ prepare_workload(unsigned int id, struct workload *wrk,
unsigned int flags)
.value = to_user_pointer(&set_engines),
};
- set_engines.extensions = 0;
+ if (ctx->wants_balance) {
+ set_engines.extensions =
+ to_user_pointer(&load_balance);
+
+ memset(&load_balance, 0, sizeof(load_balance));
+ load_balance.base.name =
+ I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
+ load_balance.num_siblings =
+ ctx->engine_map_count;
+
+ for (j = 0; j < ctx->engine_map_count; j++) {
+ load_balance.engines[j].engine_class =
+ I915_ENGINE_CLASS_VIDEO; /*
FIXME */
+ load_balance.engines[j].engine_instance
=
+ ctx->engine_map[j] - VCS1; /*
FIXME */
Ok, more fallout from fixing ctx->engine_map[] first?
Not sure I understand the question.
The proliferation of FIXME, the assumption of CLASS_VIDEO and an
impedance mismatch between engine_map and class:instance. Basically
those FIXME raise the question of what do you intend this to look like?
Intention that implicit and explicit engine maps get populated by
available engines.
When "-b i915":
1.VCS.1000.0.0 -> implicit map of available vcs engines
M.1.VCS
B.1 \-> explicit map of available vcs engines
That would support Icelake vcs0+vcs2 SKUs. And explicit engine map wsims
would be more portable, like the original ones were.
Also, I am contemplating using VCS2 in wsim as meaning the 2nd VCS
engine, so logical instances. So:
M.1.VCS1|VCS2 -> also works on both SKL and ICL (two vcs SKUs)
B.1
Regards,
Tvrtko
___
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 HDR Metadata Parsing and handling in DRM layer (rev10)
== Series Details == Series: Add HDR Metadata Parsing and handling in DRM layer (rev10) URL : https://patchwork.freedesktop.org/series/25091/ State : success == Summary == CI Bug Log - changes from CI_DRM_6081_full -> Patchwork_13017_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13017_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_eio@in-flight-suspend: - shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +3 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-apl5/igt@gem_...@in-flight-suspend.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-apl3/igt@gem_...@in-flight-suspend.html * igt@gem_exec_suspend@basic-s3: - shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#110682]) +43 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb6/igt@gem_exec_susp...@basic-s3.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb5/igt@gem_exec_susp...@basic-s3.html * igt@kms_draw_crc@fill-fb: - shard-skl: [PASS][5] -> [FAIL][6] ([fdo#103184]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-skl1/igt@kms_draw_...@fill-fb.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-skl8/igt@kms_draw_...@fill-fb.html * igt@kms_flip@flip-vs-expired-vblank-interruptible: - shard-glk: [PASS][7] -> [FAIL][8] ([fdo#102887] / [fdo#105363]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-glk6/igt@kms_f...@flip-vs-expired-vblank-interruptible.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-glk1/igt@kms_f...@flip-vs-expired-vblank-interruptible.html * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite: - shard-iclb: [PASS][9] -> [FAIL][10] ([fdo#103167]) +5 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb6/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-shrfb-draw-pwrite.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb7/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-shrfb-draw-pwrite.html * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min: - shard-skl: [PASS][11] -> [FAIL][12] ([fdo#108145]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-skl7/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-min.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-skl1/igt@kms_plane_alpha_bl...@pipe-a-constant-alpha-min.html * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-mid: - shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#109052]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb6/igt@kms_plane_alpha_bl...@pipe-b-constant-alpha-mid.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb5/igt@kms_plane_alpha_bl...@pipe-b-constant-alpha-mid.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145] / [fdo#110403]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-skl7/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-skl1/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html * igt@kms_prop_blob@invalid-set-prop-any: - shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#110682]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb6/igt@kms_prop_b...@invalid-set-prop-any.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb5/igt@kms_prop_b...@invalid-set-prop-any.html * igt@kms_psr2_su@frontbuffer: - shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109642]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb2/igt@kms_psr2...@frontbuffer.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb7/igt@kms_psr2...@frontbuffer.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) +4 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-iclb7/igt@kms_psr@psr2_cursor_plane_onoff.html * igt@kms_setmode@basic: - shard-kbl: [PASS][23] -> [FAIL][24] ([fdo#99912]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard-kbl6/igt@kms_setm...@basic.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard-kbl6/igt@kms_setm...@basic.html * igt@kms_sysfs_edid_timing: - shard-iclb: [PASS][25] -> [FAIL][26] ([fdo#100047]) [25]: https://intel-gfx-ci.01.org/tre
[Intel-gfx] [PATCH 4/5] drm/i915: Downgrade NEWCLIENT to non-preemptive
Commit 1413b2bc0717 ("drm/i915: Trim NEWCLIENT boosting") had the
intended consequence of not allowing a sequence of work that merely
crossed into a new engine the privilege to be promoted to NEWCLIENT
status. It also had the unintended consequence of actually making
NEWCLIENT effective on heavily oversubscribed transcode machines and
impacting upon their throughput.
If we consider a client packet composed of (rcsA, rcsB, vcs) and 30 of
those clients, using the NEWCLIENT boost that will be scheduled as
rcsA x 30, (rcsB, vcs) x 30
where as before it would have been
(rcsA, rcsB, vcs) x 30
That is with NEWCLIENT only boosting the first request of each client,
we would execute all rcsA requests prior to running on the vcs engines;
acruing a lot of dead time as compared to the previous case where the
vcs engine would be started in parallel to processing the second client.
The previous patch has the effect of delaying submission until it is
required by a third party (either the user with an explicit wait, or by
another client/engine). We reduce the NEWCLIENT bump to a mere WAIT,
which has the effect of removing its preemptive grant and reducing it to
the same level as any other user interaction -- that it will not be
promoted above the interengine dependencies, and so preventing NEWCLIENTS
from starving other engines. This a large nerf to the rrul properties of
the current NEWCLIENT, but it still does give prioritised submission to
new requests from light workloads.
References: b16c765122f9 ("drm/i915: Priority boost for new clients")
Fixes: 1413b2bc0717 ("drm/i915: Trim NEWCLIENT boosting") # customer impact
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Dmitry Rogozhkin
Cc: Dmitry Ermilov
---
drivers/gpu/drm/i915/gt/intel_lrc.c| 2 +-
drivers/gpu/drm/i915/i915_priolist_types.h | 5 ++---
drivers/gpu/drm/i915/i915_request.c| 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e18623def282..b5e82171df8f 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -164,7 +164,7 @@
#define WA_TAIL_DWORDS 2
#define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
-#define ACTIVE_PRIORITY (I915_PRIORITY_NEWCLIENT | I915_PRIORITY_NOSEMAPHORE)
+#define ACTIVE_PRIORITY (I915_PRIORITY_NOSEMAPHORE)
static int execlists_context_deferred_alloc(struct intel_context *ce,
struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/i915_priolist_types.h
b/drivers/gpu/drm/i915/i915_priolist_types.h
index cc44ebd3b553..49709de69875 100644
--- a/drivers/gpu/drm/i915/i915_priolist_types.h
+++ b/drivers/gpu/drm/i915/i915_priolist_types.h
@@ -20,15 +20,14 @@ enum {
I915_PRIORITY_INVALID = INT_MIN
};
-#define I915_USER_PRIORITY_SHIFT 3
+#define I915_USER_PRIORITY_SHIFT 2
#define I915_USER_PRIORITY(x) ((x) << I915_USER_PRIORITY_SHIFT)
#define I915_PRIORITY_COUNT BIT(I915_USER_PRIORITY_SHIFT)
#define I915_PRIORITY_MASK (I915_PRIORITY_COUNT - 1)
#define I915_PRIORITY_WAIT ((u8)BIT(0))
-#define I915_PRIORITY_NEWCLIENT((u8)BIT(1))
-#define I915_PRIORITY_NOSEMAPHORE ((u8)BIT(2))
+#define I915_PRIORITY_NOSEMAPHORE ((u8)BIT(1))
#define __NO_PREEMPTION (I915_PRIORITY_WAIT)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 4899195e58c2..2fca0b59578d 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1189,7 +1189,7 @@ struct i915_request *__i915_request_commit(struct
i915_request *rq)
* the bulk clients. (FQ_CODEL)
*/
if (list_empty(&rq->sched.signalers_list))
- attr.priority |= I915_PRIORITY_NEWCLIENT;
+ attr.priority |= I915_PRIORITY_WAIT;
engine->schedule(rq, &attr);
}
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Truly bump ready tasks ahead of busywaits
In commit b7404c7ecb38 ("drm/i915: Bump ready tasks ahead of
busywaits"), I tried cutting a corner in order to not install a signal
for each of our dependencies, and only listened to requests on which we
were intending to busywait. The compromise that was made was that
instead of then being able to promite the request with a full
NOSEMAPHORE like its non-busywaiting brethren, as we had not ensured we
had cleared the semaphore chain, we settled for only using the NEWCLIENT
boost. With an over saturated system with multiple NEWCLIENTS in flight
at any time, this was found to be an inadequate promotion and left us
with a much poorer scheduling order than prior to using semaphores.
The outcome of this patch, is that all requests have NOSEMAPHORE
priority when they have no dependencies and are ready to run and not
busywait, restoring the pre-semaphore ordering on saturated systems.
We can demonstrate the effect of poor scheduling order by oversaturating
the system using gem_wsim on a system with multiple vcs engines
(i.e running the same workloads across more clients than required for
peak throughput, e.g. media_load_balance_17i7.wsim -c4 -b context):
x v5.1 (normalized)
+ tip
* fix
++
|x |
|x |
|x |
|x |
| %x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %#x |
| %#x |
| %#x |
| %#x |
| %#x |
| +%#xx |
| +%#xx |
| + %%#xx |
| + %%#xx |
| + %%#xx |
| + %%#xx |
| + %%##x |
| +++ %%##x |
| +++ %%##x |
| +++ %%##x |
| %%##x |
| %%##x |
| %%##xx |
| %###xx |
| %###xx |
| %###xx |
| %###xx |
| + %#O#xx |
| + %#O#xx |
|++ + %#O#xx |
| ++%OOOxxx|
| ++ + %#OOO#xx|
| + ++ ++++@@#xx|
| |A_| |
||__M___A| |
| |A_| |
++
N Min MaxMedian AvgStddev
x 120 0.99456 1.00628 0.85 1.0001545 0.0024387139
+ 120 0.873021 1.00037 0.884134
[Intel-gfx] [PATCH 3/5] drm/i915: Bump signaler priority on adding a waiter
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
v2: Limit the bump to external edges (as originally intended) i.e.
between contexts and out to the user.
Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 12
drivers/gpu/drm/i915/i915_request.c | 9 -
drivers/gpu/drm/i915/i915_scheduler.c | 11 +++
drivers/gpu/drm/i915/i915_scheduler_types.h | 3 ++-
4 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 4b042893dc0e..5b3d8e33f1cf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -98,12 +98,14 @@ static int live_busywait_preempt(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_unlock;
- ctx_hi->sched.priority = INT_MAX;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = INT_MIN;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj)) {
@@ -958,12 +960,14 @@ static int live_preempt_hang(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_spin_lo;
- ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
for_each_engine(engine, i915, id) {
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 2a1079e472e2..4899195e58c2 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -489,15 +489,6 @@ void __i915_request_unsubmit(struct i915_request *request)
/* We may be recursing from the signal callback of another i915 fence */
spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);
- /*
-* As we do not allow WAIT to preempt inflight requests,
-* once we have executed a request, along with triggering
-* any execution callbacks, we must preserve its ordering
-* within the non-preemptible FIFO.
-*/
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- request->sched.attr.priority |= __NO_PREEMPTION;
-
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
i915_request_cancel_breadcrumb(request);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c
b/drivers/gpu/drm/i915/i915_scheduler.c
index 5581c5004ff0..d215dcdf0b1e 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -387,6 +387,16 @@ bool __i915_sched_node_add_dependency(struct
i915_sched_node *node,
!node_started(signal))
node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+ /*
+* As we do not allow WAIT to preempt inflight requests,
+* once we have executed a request, along with triggering
+* any execution callbacks, we must preserve its ordering
+* within the non-preemptible FIFO.
+*/
+ BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
+
[Intel-gfx] [PATCH 1/5] drm/i915: Mark semaphores as complete on unsubmit out if payload was started
Avoid charging us for the presumed busywait if the request was preempted
after successfully using semaphores to reduce inter-engine latency.
v2: Bump the priority to reflect the lack of semaphores now required.
References: ca6e56f654e7 ("drm/i915: Disable semaphore busywaits on saturated
systems")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_request.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index bed213148cbb..11670774cd25 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -509,6 +509,12 @@ void __i915_request_unsubmit(struct i915_request *request)
/* Transfer back from the global per-engine timeline to per-context */
move_to_timeline(request, request->timeline);
+ /* We've already spun, don't charge on resubmitting. */
+ if (request->sched.semaphores && i915_request_started(request)) {
+ request->sched.attr.priority |= I915_PRIORITY_NOSEMAPHORE;
+ request->sched.semaphores = 0;
+ }
+
/*
* We don't need to wake_up any waiters on request->execute, they
* will get woken by any other event or us re-adding this request
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915/execlists: Drop promotion on unsubmit
With the disappearance of NEWCLIENT, we no longer need to provide the
priority boost on preemption in order to prevent repeated gazumping,
and we can remove the dead code.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 59 +
1 file changed, 10 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index b5e82171df8f..f263a8374273 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -164,8 +164,6 @@
#define WA_TAIL_DWORDS 2
#define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
-#define ACTIVE_PRIORITY (I915_PRIORITY_NOSEMAPHORE)
-
static int execlists_context_deferred_alloc(struct intel_context *ce,
struct intel_engine_cs *engine);
static void execlists_init_reg_state(u32 *reg_state,
@@ -189,23 +187,12 @@ static int effective_prio(const struct i915_request *rq)
/*
* On unwinding the active request, we give it a priority bump
-* equivalent to a freshly submitted request. This protects it from
-* being gazumped again, but it would be preferable if we didn't
-* let it be gazumped in the first place!
-*
-* See __unwind_incomplete_requests()
+* if it has completed waiting on any semaphore. If we know that
+* the request has already started, we can prevent an unwanted
+* preempt-to-idle cycle by taking that into account now.
*/
- if (~prio & ACTIVE_PRIORITY && __i915_request_has_started(rq)) {
- /*
-* After preemption, we insert the active request at the
-* end of the new priority level. This means that we will be
-* _lower_ priority than the preemptee all things equal (and
-* so the preemption is valid), so adjust our comparison
-* accordingly.
-*/
- prio |= ACTIVE_PRIORITY;
- prio--;
- }
+ if (__i915_request_has_started(rq))
+ prio |= I915_PRIORITY_NOSEMAPHORE;
/* Restrict mere WAIT boosts from triggering preemption */
return prio | __NO_PREEMPTION;
@@ -371,11 +358,11 @@ static void unwind_wa_tail(struct i915_request *rq)
}
static struct i915_request *
-__unwind_incomplete_requests(struct intel_engine_cs *engine, int boost)
+__unwind_incomplete_requests(struct intel_engine_cs *engine)
{
struct i915_request *rq, *rn, *active = NULL;
struct list_head *uninitialized_var(pl);
- int prio = I915_PRIORITY_INVALID | boost;
+ int prio = I915_PRIORITY_INVALID;
lockdep_assert_held(&engine->timeline.lock);
@@ -402,31 +389,6 @@ __unwind_incomplete_requests(struct intel_engine_cs
*engine, int boost)
active = rq;
}
- /*
-* The active request is now effectively the start of a new client
-* stream, so give it the equivalent small priority bump to prevent
-* it being gazumped a second time by another peer.
-*
-* Note we have to be careful not to apply a priority boost to a request
-* still spinning on its semaphores. If the request hasn't started, that
-* means it is still waiting for its dependencies to be signaled, and
-* if we apply a priority boost to this request, we will boost it past
-* its signalers and so break PI.
-*
-* One consequence of this preemption boost is that we may jump
-* over lesser priorities (such as I915_PRIORITY_WAIT), effectively
-* making those priorities non-preemptible. They will be moved forward
-* in the priority queue, but they will not gain immediate access to
-* the GPU.
-*/
- if (~prio & boost && __i915_request_has_started(active)) {
- prio |= boost;
- GEM_BUG_ON(active->sched.attr.priority >= prio);
- active->sched.attr.priority = prio;
- list_move_tail(&active->sched.link,
- i915_sched_lookup_priolist(engine, prio));
- }
-
return active;
}
@@ -436,7 +398,7 @@ execlists_unwind_incomplete_requests(struct
intel_engine_execlists *execlists)
struct intel_engine_cs *engine =
container_of(execlists, typeof(*engine), execlists);
- return __unwind_incomplete_requests(engine, 0);
+ return __unwind_incomplete_requests(engine);
}
static inline void
@@ -657,8 +619,7 @@ static void complete_preempt_context(struct
intel_engine_execlists *execlists)
execlists_cancel_port_requests(execlists);
__unwind_incomplete_requests(container_of(execlists,
struct intel_engine_cs,
- execlists),
-ACTIVE_PRIORITY);
+
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915: Mark semaphores as complete on unsubmit out if payload was started
== Series Details ==
Series: series starting with [1/5] drm/i915: Mark semaphores as complete on
unsubmit out if payload was started
URL : https://patchwork.freedesktop.org/series/60671/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
44f76ee8e2e2 drm/i915: Mark semaphores as complete on unsubmit out if payload
was started
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#12:
References: ca6e56f654e7 ("drm/i915: Disable semaphore busywaits on saturated
systems")
-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit ca6e56f654e7 ("drm/i915: Disable
semaphore busywaits on saturated systems")'
#12:
References: ca6e56f654e7 ("drm/i915: Disable semaphore busywaits on saturated
systems")
total: 1 errors, 1 warnings, 0 checks, 12 lines checked
2e6339d4939b drm/i915: Truly bump ready tasks ahead of busywaits
e8b241693441 drm/i915: Bump signaler priority on adding a waiter
29e2d059081e drm/i915: Downgrade NEWCLIENT to non-preemptive
-:37: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit b16c765122f9 ("drm/i915:
Priority boost for new clients")'
#37:
References: b16c765122f9 ("drm/i915: Priority boost for new clients")
total: 1 errors, 0 warnings, 0 checks, 33 lines checked
8737f6b9a62f drm/i915/execlists: Drop promotion on unsubmit
___
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/5] drm/i915: Mark semaphores as complete on unsubmit out if payload was started
== Series Details ==
Series: series starting with [1/5] drm/i915: Mark semaphores as complete on
unsubmit out if payload was started
URL : https://patchwork.freedesktop.org/series/60671/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6085 -> Patchwork_13020
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/
Known issues
Here are the changes found in Patchwork_13020 that come from known issues:
### IGT changes ###
Possible fixes
* igt@gem_busy@basic-busy-default:
- fi-icl-u2: [INCOMPLETE][1] ([fdo#107713]) -> [PASS][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/fi-icl-u2/igt@gem_b...@basic-busy-default.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/fi-icl-u2/igt@gem_b...@basic-busy-default.html
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [DMESG-FAIL][3] ([fdo#110235]) -> [PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
[fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
Participating hosts (52 -> 46)
--
Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan
fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6085 -> Patchwork_13020
CI_DRM_6085: 48d8cf5cc0aadd21924d05ad3e86b08d8e0e1c50 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4988: 2f6303d13e09b2457762540383c7f36cecd02bbf @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13020: 8737f6b9a62fa2d982870429e1ca43e5bffb273c @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8737f6b9a62f drm/i915/execlists: Drop promotion on unsubmit
29e2d059081e drm/i915: Downgrade NEWCLIENT to non-preemptive
e8b241693441 drm/i915: Bump signaler priority on adding a waiter
2e6339d4939b drm/i915: Truly bump ready tasks ahead of busywaits
44f76ee8e2e2 drm/i915: Mark semaphores as complete on unsubmit out if payload
was started
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Re-add enable_rc6 modparam
On Wed, 2019-05-15 at 06:43 +0100, Tvrtko Ursulin wrote: > On 15/05/2019 01:06, Rodrigo Vivi wrote: > > On Tue, May 14, 2019 at 06:32:01PM +, Summers, Stuart wrote: > > > On Tue, 2019-05-14 at 17:53 +0100, Chris Wilson wrote: > > > > Quoting Stuart Summers (2019-05-14 17:46:52) > > > > > To allow easier debug of platforms which do not fully support > > > > > power-saving render C-state 6, add back the module parameter > > > > > to allow RC6 flows to be disabled. Instead of directly > > > > > affecting > > > > > the RC6 states via a bitmask as done previously, use this > > > > > module > > > > > parameter to clear the has_rc6 field for these platforms. > > > > > > > > If you know which platforms don't support rc6, don't enable > > > > rc6. > > > > > > I'd really prefer to have this parameter in place for debug > > > purposes. > > > It seems more useful to allow quick testing by reloading i915 > > > than by > > > requiring a rebuild. > > > > > > Of course, once debug is complete and the platform is known to > > > either > > > not support the feature or has some cripling bug around this, I > > > agree, > > > rc6 shouldn't be enabled on that platform and i915 should be > > > updated. > > > > Exactly. We need the flexibility for debug that. > > unfortunately using debugfs doesn't look a solution. > > > > One possibility that just came to my mind now is, what if we make > > this only for platforms that are still protected by > > is_alpha_support=1 > > (soon becoming require_force_probe=1) > > > > But this is just one side of the coin... when product is out there > > and we want the user to debug the issue to see if it is a RC6 bug > > we have no way to verify that. :/ > > > > Please let's add this back one way or another. > > To throw a compromise option out there - add the modparam for debug > builds only (CONFIG_DRM_I915_DEBUG)? No issues here, and makes sense. If we have concensus, I'd be happy to add this and repost. -Stuart > > Regards, > > Tvrtko smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Add N & CTS values for 10/12 bit deep color
On Tue, 02 Apr 2019, Aditya Swarup wrote:
> Adding N & CTS values for 10/12 bit deep color from Appendix C
> table in HDMI 2.0 spec. The correct values for N is not chosen
> automatically by hardware for deep color modes.
>
> v2: Remove redundant code and make it generic.(Jani)
>
> Signed-off-by: Aditya Swarup
> Cc: Clint Taylor
> Cc: Ville Syrjälä
> Cc: Jani Nikula
> Cc: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_audio.c | 82 +-
> 1 file changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 502b57ce72ab..ad53b04fa5a2 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -70,6 +70,13 @@ struct dp_aud_n_m {
> u16 n;
> };
>
> +struct hdmi_aud_ncts_table {
The struct itself is not a table. Just make it struct hdmi_ncts or
something.
> + int sample_rate;
> + int clock;
> + int n;
> + int cts;
> +};
> +
> /* Values according to DP 1.4 Table 2-104 */
> static const struct dp_aud_n_m dp_aud_n_m[] = {
> { 32000, LC_162M, 1024, 10125 },
> @@ -146,12 +153,7 @@ static const struct {
> #define TMDS_594M 594000
> #define TMDS_593M 593407
>
> -static const struct {
> - int sample_rate;
> - int clock;
> - int n;
> - int cts;
> -} hdmi_aud_ncts[] = {
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_24bpp[] = {
> { 32000, TMDS_296M, 5824, 421875 },
> { 32000, TMDS_297M, 3072, 222750 },
> { 32000, TMDS_593M, 5824, 843750 },
> @@ -182,6 +184,49 @@ static const struct {
> { 192000, TMDS_594M, 24576, 594000 },
> };
>
> +/* Appendix C - N & CTS values for deep color from HDMI 2.0 spec*/
> +/* HDMI N/CTS table for 10 bit deep color(30 bpp)*/
> +#define TMDS_371M 371250
> +#define TMDS_370M 370878
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_30bpp[] = {
> + { 32000, TMDS_370M, 5824, 527344 },
N should be 11648?
> + { 32000, TMDS_371M, 6144, 556875 },
> + { 44100, TMDS_370M, 8918, 585938 },
N 17836
> + { 44100, TMDS_371M, 4704, 309375 },
> + { 88200, TMDS_370M, 17836, 585938 },
N 35672
> + { 88200, TMDS_371M, 9408, 309375 },
> + { 176400, TMDS_370M, 35672, 585938 },
N 71344
> + { 176400, TMDS_371M, 18816, 309375 },
> + { 48000, TMDS_370M, 11648, 703125 },
> + { 48000, TMDS_371M, 5120, 309375 },
> + { 96000, TMDS_370M, 23296, 703125 },
> + { 96000, TMDS_371M, 10240, 309375 },
> + { 192000, TMDS_370M, 46592, 703125 },
> + { 192000, TMDS_371M, 20480, 309375 },
> +};
> +
> +/* HDMI N/CTS table for 12 bit deep color(36 bpp)*/
> +#define TMDS_445_5M 445500
> +#define TMDS_445M 445054
> +
> +static const struct hdmi_aud_ncts_table hdmi_aud_ncts_36bpp[] = {
> + { 32000, TMDS_445M, 5824, 632813 },
> + { 32000, TMDS_445_5M, 4096, 445500 },
> + { 44100, TMDS_445M, 8918, 703125 },
> + { 44100, TMDS_445_5M, 4704, 371250 },
> + { 88200, TMDS_445M, 17836, 703125 },
> + { 88200, TMDS_445_5M, 9408, 371250 },
> + { 176400, TMDS_445M, 35672, 703125 },
> + { 176400, TMDS_445_5M, 18816, 371250 },
> + { 48000, TMDS_445M, 5824, 421875 },
> + { 48000, TMDS_445_5M, 5120, 371250 },
> + { 96000, TMDS_445M, 11648, 421875 },
> + { 96000, TMDS_445_5M, 10240, 371250 },
> + { 192000, TMDS_445M, 23296, 421875 },
> + { 192000, TMDS_445_5M, 20480, 371250 },
> +};
> +
> /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state
> *crtc_state)
> {
> @@ -210,16 +255,27 @@ static u32 audio_config_hdmi_pixel_clock(const struct
> intel_crtc_state *crtc_sta
> static int audio_config_hdmi_get_n(const struct intel_crtc_state *crtc_state,
> int rate)
> {
> - const struct drm_display_mode *adjusted_mode =
> - &crtc_state->base.adjusted_mode;
> - int i;
> + const struct hdmi_aud_ncts_table *hdmi_ncts_table;
> + int i, size = 0;
Unnecessary initialization.
> +
> + if (crtc_state->pipe_bpp == 36) {
> + hdmi_ncts_table = hdmi_aud_ncts_36bpp;
> + size = ARRAY_SIZE(hdmi_aud_ncts_36bpp);
> + } else if (crtc_state->pipe_bpp == 30) {
> + hdmi_ncts_table = hdmi_aud_ncts_30bpp;
> + size = ARRAY_SIZE(hdmi_aud_ncts_30bpp);
> + } else {
> + hdmi_ncts_table = hdmi_aud_ncts_24bpp;
> + size = ARRAY_SIZE(hdmi_aud_ncts_24bpp);
> + }
>
> - for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
> - if (rate == hdmi_aud_ncts[i].sample_rate &&
> - adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
> - return hdmi_aud_ncts[i].n;
> + for (i = 0; i < size; i++) {
^
Superfluous space.
> + if (rate == hdmi_ncts_table[i].sample_rate &&
> + crtc_state->port_clock == hdmi_ncts_table[i
Re: [Intel-gfx] [PATCH v5 07/11] drm/fb-helper: Move out commit code
Den 15.05.2019 11.09, skrev Sam Ravnborg: > Hi Noralf. > > On Mon, May 06, 2019 at 08:01:35PM +0200, Noralf Trønnes wrote: >> Move the modeset commit code to drm_client_modeset. >> No changes except exporting API. >> >> v2: Move to drm_client_modeset.c instead of drm_client.c >> >> Signed-off-by: Noralf Trønnes > Reviewed-by: Sam Ravnborg > > Assumes this is a one-to-one copy. I did not check it line-by-line. Yep, I've just cut and pasted over the code. > Should git be able to tell this with proper options? > No idea, git is a very useful and powerful tool, but I know only the basics. > Sam > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/11] drm/fb-helper: Remove drm_fb_helper_crtc
Hi Sam,
[looks like Thundebird decided to throw away my reply, so I'll try again]
Den 15.05.2019 11.04, skrev Sam Ravnborg:
> Hi Noralf.
>
> I have read through the cahnes a copuple of times not and feel confident
> to add my r-b if the comments are considered.
>
> On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
>> It now only contains the modeset so use that directly instead and attach
>> a modeset array to drm_client_dev. drm_fb_helper will use this array.
>> Code will later be moved to drm_client, so add code there in a new file
>> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
>
> The first part of this commit log could use some re-pharsing.
> What is "It" etc.
>
I could do this:
struct drm_fb_helper_crtc is now just a wrapper around drm_mode_set so
use that directly instead and attach it as a modeset array onto
drm_client_dev. drm_fb_helper will use this array to store its modesets
which means it will always initialize a drm_client, but it will not
register the client (callbacks) unless it's the generic fbdev emulation.
>> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct
>> drm_fb_helper *fb_helper)
>> DRM_MODE_ROTATE_0);
>> }
>>
>> -for (i = 0; i < fb_helper->crtc_count; i++) {
>> -struct drm_mode_set *mode_set =
>> &fb_helper->crtc_info[i].mode_set;
>> +drm_client_for_each_modeset(mode_set, client) {
>> struct drm_crtc *crtc = mode_set->crtc;
>>
>> if (crtc->funcs->cursor_set2) {
> This function requires modeset_mutex to be held. Maybe add comment?
>
drm_client_for_each_modeset() has a lockdep warn (courtesy of Daniel
Vetter):
#define drm_client_for_each_modeset(modeset, client) \
for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
modeset = (client)->modesets; modeset->crtc; modeset++)
>> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct fb_var_screeninfo
>> *var,
>>
>> pan_set(fb_helper, var->xoffset, var->yoffset);
>>
>> -ret = restore_fbdev_mode_atomic(fb_helper, true);
>> +ret = restore_fbdev_mode_force(fb_helper);
> This change looks alien compared to other changes.
> Does it belong to this patchset?
>
It's mentioned in the commit message:
In pan_display_atomic() restore_fbdev_mode_force() is used instead of
restore_fbdev_mode_atomic() because that one will later become internal
to drm_client_modeset.
Thanks for looking at this, I'll spin a new version.
Noralf.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Disable active links before rebooting
Certain monitors, e.g. Dell, do not like it when we reboot with an
active link, leaving them in a confused state where they refuse to
renegotiate the link after the reboot. If we hook into the reboot
notifier, we can switch off any active link before rebooting, leaving
everything in a consistent, hopefully happy, state.
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_pci.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 401eb6c71ae1..7b2dc8d66f35 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -26,6 +26,7 @@
#include
#include
+#include
#include
#include "i915_drv.h"
@@ -909,6 +910,9 @@ static void i915_pci_shutdown(struct pci_dev *pdev)
/* Cancel any outstanding rendering */
if (READ_ONCE(i915->gt.awake))
i915_gem_set_wedged(i915);
+
+ /* Disable active links to avoid confusing certain (Dell) monitors */
+ drm_atomic_helper_shutdown(&i915->drm);
}
static struct pci_driver i915_pci_driver = {
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Hook into the reboot notifier to cancel outstanding GPU operations
When the system is being shutdown, we no longer care about the results
of outstanding GPU operations, and so we can cancel them to speed up the
reboot. This helps most if we happen to be stuck waiting for a timeout
to declare the GPU hung.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_pci.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index e8119bcbf178..401eb6c71ae1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -898,11 +898,25 @@ static int i915_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
return 0;
}
+static void i915_pci_shutdown(struct pci_dev *pdev)
+{
+ struct drm_i915_private *i915;
+
+ i915 = pci_get_drvdata(pdev);
+ if (!i915) /* driver load aborted? */
+ return;
+
+ /* Cancel any outstanding rendering */
+ if (READ_ONCE(i915->gt.awake))
+ i915_gem_set_wedged(i915);
+}
+
static struct pci_driver i915_pci_driver = {
.name = DRIVER_NAME,
.id_table = pciidlist,
.probe = i915_pci_probe,
.remove = i915_pci_remove,
+ .shutdown = i915_pci_shutdown,
.driver.pm = &i915_pm_ops,
};
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Shutdown hooks
Janus, some old patches that may be of use for shutdown prior to kexec. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/11] drm/fb-helper: Remove drm_fb_helper_crtc
Hi Noralf.
On Wed, May 15, 2019 at 04:51:04PM +0200, Noralf Trønnes wrote:
> Hi Sam,
>
> [looks like Thundebird decided to throw away my reply, so I'll try again]
I guess it awaits moderator approval as the from address was different
from what you otherwise used - I got both mails.
>
> Den 15.05.2019 11.04, skrev Sam Ravnborg:
> > Hi Noralf.
> >
> > I have read through the cahnes a copuple of times not and feel confident
> > to add my r-b if the comments are considered.
> >
> > On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
> >> It now only contains the modeset so use that directly instead and attach
> >> a modeset array to drm_client_dev. drm_fb_helper will use this array.
> >> Code will later be moved to drm_client, so add code there in a new file
> >> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
> >
> > The first part of this commit log could use some re-pharsing.
> > What is "It" etc.
> >
>
> I could do this:
>
> struct drm_fb_helper_crtc is now just a wrapper around drm_mode_set so
> use that directly instead and attach it as a modeset array onto
> drm_client_dev. drm_fb_helper will use this array to store its modesets
> which means it will always initialize a drm_client, but it will not
> register the client (callbacks) unless it's the generic fbdev emulation.
This is much better - thanks.
>
> >> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct
> >> drm_fb_helper *fb_helper)
> >>DRM_MODE_ROTATE_0);
> >>}
> >>
> >> - for (i = 0; i < fb_helper->crtc_count; i++) {
> >> - struct drm_mode_set *mode_set =
> >> &fb_helper->crtc_info[i].mode_set;
> >> + drm_client_for_each_modeset(mode_set, client) {
> >>struct drm_crtc *crtc = mode_set->crtc;
> >>
> >>if (crtc->funcs->cursor_set2) {
> > This function requires modeset_mutex to be held. Maybe add comment?
> >
>
> drm_client_for_each_modeset() has a lockdep warn (courtesy of Daniel
> Vetter):
>
> #define drm_client_for_each_modeset(modeset, client) \
> for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
>modeset = (client)->modesets; modeset->crtc; modeset++)
>
> >> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct
> >> fb_var_screeninfo *var,
> >>
> >>pan_set(fb_helper, var->xoffset, var->yoffset);
> >>
> >> - ret = restore_fbdev_mode_atomic(fb_helper, true);
> >> + ret = restore_fbdev_mode_force(fb_helper);
> > This change looks alien compared to other changes.
> > Does it belong to this patchset?
> >
>
> It's mentioned in the commit message:
>
> In pan_display_atomic() restore_fbdev_mode_force() is used instead of
> restore_fbdev_mode_atomic() because that one will later become internal
> to drm_client_modeset.
Right, I forgot about this detail from the commit log.
Sam
___
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: Hook into the reboot notifier to cancel outstanding GPU operations
== Series Details ==
Series: series starting with [1/2] drm/i915: Hook into the reboot notifier to
cancel outstanding GPU operations
URL : https://patchwork.freedesktop.org/series/60676/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6086 -> Patchwork_13021
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/
Known issues
Here are the changes found in Patchwork_13021 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [PASS][1] -> [INCOMPLETE][2] ([fdo#108602] /
[fdo#108744])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
Possible fixes
* igt@gem_ctx_create@basic-files:
- {fi-icl-u3}:[INCOMPLETE][3] ([fdo#107713] / [fdo#109100]) ->
[PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-icl-u3/igt@gem_ctx_cre...@basic-files.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-icl-u3/igt@gem_ctx_cre...@basic-files.html
- {fi-icl-y}: [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) ->
[PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-icl-y/igt@gem_ctx_cre...@basic-files.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-icl-y/igt@gem_ctx_cre...@basic-files.html
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][7] ([fdo#107718]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-blb-e6850/igt@gem_exec_susp...@basic-s3.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-blb-e6850/igt@gem_exec_susp...@basic-s3.html
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [DMESG-FAIL][9] ([fdo#110235]) -> [PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
- fi-skl-gvtdvm: [DMESG-FAIL][11] ([fdo#110235]) -> [PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@kms_busy@basic-flip-c:
- fi-skl-6770hq: [SKIP][13] ([fdo#109271] / [fdo#109278]) ->
[PASS][14] +2 similar issues
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: [SKIP][15] ([fdo#109271]) -> [PASS][16] +23 similar
issues
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (54 -> 46)
--
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6086 -> Patchwork_13021
CI_DRM_6086: a15de3af1c8803492f380236f8494103a5d4a982 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4990: 24500748e2cc2c4c4e8f3dd037d2da06d4711d35 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13021: fd42b0510d7fe7f9b93c60a9f84643a0be18b5e3 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fd42b0510d7f drm/i915: Disable active links before rebooting
82013d07628d drm/i915: Hook into the reboot notifier to cancel outstanding GPU
operations
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freed
Re: [Intel-gfx] [PATCH] drm/i915/perf: Refactor oa object to better manage resources
On Wed, May 15, 2019 at 10:11:34AM +0100, Lionel Landwerlin wrote:
On 14/05/2019 19:14, Umesh Nerlige Ramappa wrote:
On Tue, May 14, 2019 at 10:34:49AM +0100, Lionel Landwerlin wrote:
Hi Umesh,
I just noticed this different between v1 & v2.
My understanding is that if destroy() is called, stream should be
the same as dev_priv->perf.exclusive_stream.
If it's not it sounds like a bug. So why change this?
v2 fixes only checkpatch warnings. it warned on use of BUG_ON.
BUG_ON is intended to crash the system in severe cases where the
driver/kernel is unusable. In this case, the mismatch between user
passed information and exclusive_stream may not require a crash.
This is called from i915_perf_release() which is attached to the
i915-perf file descriptor only in i915_perf.c.
If we managed to reach that function it must be because the file
descriptor given by userspace is associated to the i915-perf stream.
Having stream != dev_priv->perf.exclusive_stream means that we
probably screwed up the locking somewhere in this file.
So I would argue this is a kernel issue, not a user issue and that
using BUG_ON() is justified.
I see. Will change it back.
Thanks,
Umesh
Thanks,
-Lionel
-Lionel
On 03/05/2019 00:13, Umesh Nerlige Ramappa wrote:
static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
{
struct drm_i915_private *dev_priv = stream->dev_priv;
- BUG_ON(stream != dev_priv->perf.oa.exclusive_stream);
+ if (stream != dev_priv->perf.exclusive_stream) {
+ WARN_ON_ONCE(stream != dev_priv->perf.exclusive_stream);
+ return;
+ }
/*
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/perf: Refactor oa object to better manage resources
The oa object manages the oa buffer and must be allocated when the user
intends to read performance counter snapshots. This can be achieved by
making the oa object part of the stream object which is allocated when a
stream is opened by the user.
Attributes in the oa object that are gen-specific are moved to the perf
object so that they can be initialized on driver load.
The split provides a better separation of the objects used in perf
implementation of i915 driver so that resources are allocated and
initialized only when needed.
v2: Fix checkpatch warnings
v3: Addressed Lionel's review comment
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/gt/intel_sseu.c | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 4 +-
drivers/gpu/drm/i915/i915_drv.h | 219 +-
drivers/gpu/drm/i915/i915_oa_bdw.c| 30 +-
drivers/gpu/drm/i915/i915_oa_bxt.c| 30 +-
drivers/gpu/drm/i915/i915_oa_cflgt2.c | 30 +-
drivers/gpu/drm/i915/i915_oa_cflgt3.c | 30 +-
drivers/gpu/drm/i915/i915_oa_chv.c| 30 +-
drivers/gpu/drm/i915/i915_oa_cnl.c| 30 +-
drivers/gpu/drm/i915/i915_oa_glk.c| 30 +-
drivers/gpu/drm/i915/i915_oa_hsw.c| 30 +-
drivers/gpu/drm/i915/i915_oa_icl.c| 30 +-
drivers/gpu/drm/i915/i915_oa_kblgt2.c | 30 +-
drivers/gpu/drm/i915/i915_oa_kblgt3.c | 30 +-
drivers/gpu/drm/i915/i915_oa_sklgt2.c | 30 +-
drivers/gpu/drm/i915/i915_oa_sklgt3.c | 30 +-
drivers/gpu/drm/i915/i915_oa_sklgt4.c | 30 +-
drivers/gpu/drm/i915/i915_perf.c | 583 ++
18 files changed, 629 insertions(+), 599 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c
b/drivers/gpu/drm/i915/gt/intel_sseu.c
index 7f448f3bea0b..fa78df39521a 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -32,7 +32,7 @@ u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
* cases which disable slices for functional, apart for performance
* reasons. So in this case we select a known stable subset.
*/
- if (!i915->perf.oa.exclusive_stream) {
+ if (!i915->perf.exclusive_stream) {
ctx_sseu = *req_sseu;
} else {
ctx_sseu = intel_sseu_from_device_info(sseu);
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 7ae42f2ebfe8..878e71a927de 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -81,8 +81,8 @@ static void sr_oa_regs(struct intel_vgpu_workload *workload,
u32 *reg_state, bool save)
{
struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
- u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
- u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
+ u32 ctx_oactxctrl = dev_priv->perf.ctx_oactxctrl_offset;
+ u32 ctx_flexeu0 = dev_priv->perf.ctx_flexeu0_offset;
int i = 0;
u32 flex_mmio[] = {
i915_mmio_reg_offset(EU_PERF_CNTL0),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9a634ba57ff9..23778f6299de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1400,6 +1400,86 @@ struct i915_perf_stream {
* @oa_config: The OA configuration used by the stream.
*/
struct i915_oa_config *oa_config;
+
+ /**
+* The OA context specific information.
+*/
+ struct intel_context *pinned_ctx;
+ u32 specific_ctx_id;
+ u32 specific_ctx_id_mask;
+
+ struct hrtimer poll_check_timer;
+ wait_queue_head_t poll_wq;
+ bool pollin;
+
+ bool periodic;
+ int period_exponent;
+
+ /**
+* State of the OA buffer.
+*/
+ struct {
+ struct i915_vma *vma;
+ u8 *vaddr;
+ u32 last_ctx_id;
+ int format;
+ int format_size;
+ int size_exponent;
+
+ /**
+* Locks reads and writes to all head/tail state
+*
+* Consider: the head and tail pointer state needs to be read
+* consistently from a hrtimer callback (atomic context) and
+* read() fop (user context) with tail pointer updates happening
+* in atomic context and head updates in user context and the
+* (unlikely) possibility of read() errors needing to reset all
+* head/tail state.
+*
+* Note: Contention/performance aren't currently a significant
+* concern here considering the relatively low frequency of
+* hrtimer callbacks (5ms period) and that reads typically only
+* happen in response to a hrtimer event and likely complete
+* before the next callback.
+*
+* Note: This lock is not held *while* reading and copyin
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/perf: Refactor oa object to better manage resources (rev3)
== Series Details == Series: drm/i915/perf: Refactor oa object to better manage resources (rev3) URL : https://patchwork.freedesktop.org/series/60176/ State : warning == Summary == $ dim checkpatch origin/drm-tip 3c6e6cc91331 drm/i915/perf: Refactor oa object to better manage resources -:1522: WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() #1522: FILE: drivers/gpu/drm/i915/i915_perf.c:1366: + BUG_ON(stream != dev_priv->perf.exclusive_stream); total: 0 errors, 1 warnings, 0 checks, 2166 lines checked ___ 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/perf: Refactor oa object to better manage resources (rev3)
== Series Details == Series: drm/i915/perf: Refactor oa object to better manage resources (rev3) URL : https://patchwork.freedesktop.org/series/60176/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915/perf: Refactor oa object to better manage resources -O:drivers/gpu/drm/i915/i915_perf.c:1430:15: warning: memset with byte count of 16777216 -O:drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1435:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915: Mark semaphores as complete on unsubmit out if payload was started
== Series Details == Series: series starting with [1/5] drm/i915: Mark semaphores as complete on unsubmit out if payload was started URL : https://patchwork.freedesktop.org/series/60671/ State : failure == Summary == CI Bug Log - changes from CI_DRM_6085_full -> Patchwork_13020_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_13020_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_13020_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_13020_full: ### IGT changes ### Possible regressions * igt@gem_eio@in-flight-suspend: - shard-snb: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-snb1/igt@gem_...@in-flight-suspend.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-snb5/igt@gem_...@in-flight-suspend.html Known issues Here are the changes found in Patchwork_13020_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_eio@unwedge-stress: - shard-snb: [PASS][3] -> [FAIL][4] ([fdo#109661]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-snb7/igt@gem_...@unwedge-stress.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-snb1/igt@gem_...@unwedge-stress.html * igt@gem_softpin@noreloc-s3: - shard-skl: [PASS][5] -> [INCOMPLETE][6] ([fdo#104108] / [fdo#107773]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl10/igt@gem_soft...@noreloc-s3.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-skl8/igt@gem_soft...@noreloc-s3.html * igt@i915_pm_rpm@debugfs-read: - shard-skl: [PASS][7] -> [INCOMPLETE][8] ([fdo#107807]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl3/igt@i915_pm_...@debugfs-read.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-skl7/igt@i915_pm_...@debugfs-read.html * igt@i915_pm_rpm@i2c: - shard-iclb: [PASS][9] -> [DMESG-WARN][10] ([fdo#109982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb2/igt@i915_pm_...@i2c.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-iclb2/igt@i915_pm_...@i2c.html * igt@i915_suspend@fence-restore-tiled2untiled: - shard-apl: [PASS][11] -> [DMESG-WARN][12] ([fdo#108566]) +2 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-apl1/igt@i915_susp...@fence-restore-tiled2untiled.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-apl8/igt@i915_susp...@fence-restore-tiled2untiled.html * igt@kms_cursor_legacy@cursor-vs-flip-legacy: - shard-hsw: [PASS][13] -> [FAIL][14] ([fdo#103355]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-hsw8/igt@kms_cursor_leg...@cursor-vs-flip-legacy.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-hsw8/igt@kms_cursor_leg...@cursor-vs-flip-legacy.html * igt@kms_flip@flip-vs-suspend: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([fdo#109507]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-skl6/igt@kms_f...@flip-vs-suspend.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-skl9/igt@kms_f...@flip-vs-suspend.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt: - shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103167]) +4 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb5/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-iclb5/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html * igt@kms_psr@no_drrs: - shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#108341]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb8/igt@kms_psr@no_drrs.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-iclb1/igt@kms_psr@no_drrs.html * igt@kms_psr@psr2_cursor_plane_move: - shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13020/shard-iclb3/igt@kms_psr@psr2_cursor_plane_move.html * igt@kms_setmode@basic: - shard-kbl: [PASS][23] -> [FAIL][24] ([fdo#99912]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6085/shard-kbl2/igt@kms_setm...@basic.htm
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/perf: Refactor oa object to better manage resources (rev3)
== Series Details ==
Series: drm/i915/perf: Refactor oa object to better manage resources (rev3)
URL : https://patchwork.freedesktop.org/series/60176/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6086 -> Patchwork_13022
Summary
---
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/
Known issues
Here are the changes found in Patchwork_13022 that come from known issues:
### IGT changes ###
Issues hit
* igt@debugfs_test@read_all_entries:
- fi-ilk-650: [PASS][1] -> [DMESG-WARN][2] ([fdo#106387])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-ilk-650/igt@debugfs_test@read_all_entries.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-ilk-650/igt@debugfs_test@read_all_entries.html
Possible fixes
* igt@gem_ctx_create@basic-files:
- {fi-icl-u3}:[INCOMPLETE][3] ([fdo#107713] / [fdo#109100]) ->
[PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-icl-u3/igt@gem_ctx_cre...@basic-files.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-icl-u3/igt@gem_ctx_cre...@basic-files.html
- {fi-icl-y}: [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) ->
[PASS][6]
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-icl-y/igt@gem_ctx_cre...@basic-files.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-icl-y/igt@gem_ctx_cre...@basic-files.html
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][7] ([fdo#107718]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-blb-e6850/igt@gem_exec_susp...@basic-s3.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-blb-e6850/igt@gem_exec_susp...@basic-s3.html
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [DMESG-FAIL][9] ([fdo#110235]) -> [PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
- fi-skl-gvtdvm: [DMESG-FAIL][11] ([fdo#110235]) -> [PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@kms_busy@basic-flip-c:
- fi-skl-6770hq: [SKIP][13] ([fdo#109271] / [fdo#109278]) ->
[PASS][14] +2 similar issues
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-skl-6770hq/igt@kms_b...@basic-flip-c.html
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: [SKIP][15] ([fdo#109271]) -> [PASS][16] +23 similar
issues
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/fi-skl-6770hq/igt@kms_f...@basic-flip-vs-dpms.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (54 -> 46)
--
Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-
* Linux: CI_DRM_6086 -> Patchwork_13022
CI_DRM_6086: a15de3af1c8803492f380236f8494103a5d4a982 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4990: 24500748e2cc2c4c4e8f3dd037d2da06d4711d35 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13022: 3c6e6cc913317b60916e5bda88a8055f97356af9 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
3c6e6cc91331 drm/i915/perf: Refactor oa object to better manage resources
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 05/16] i915/gem_ctx_create: Basic checks for constructor properties
Quoting Tvrtko Ursulin (2019-05-14 13:27:38)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Check that the extended create interface accepts setparam.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/i915/gem_ctx_create.c | 225 ++--
> > 1 file changed, 213 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/i915/gem_ctx_create.c b/tests/i915/gem_ctx_create.c
> > index a664070db..9b4fddbe7 100644
> > --- a/tests/i915/gem_ctx_create.c
> > +++ b/tests/i915/gem_ctx_create.c
> > @@ -33,6 +33,7 @@
> > #include
> >
> > #include "igt_rand.h"
> > +#include "sw_sync.h"
> >
> > #define LOCAL_I915_EXEC_BSD_SHIFT (13)
> > #define LOCAL_I915_EXEC_BSD_MASK (3 << LOCAL_I915_EXEC_BSD_SHIFT)
> > @@ -45,12 +46,33 @@ static unsigned all_nengine;
> > static unsigned ppgtt_engines[16];
> > static unsigned ppgtt_nengine;
> >
> > -static int __gem_context_create_local(int fd, struct
> > drm_i915_gem_context_create *arg)
> > +static int create_ioctl(int fd, struct drm_i915_gem_context_create *arg)
> > {
> > - int ret = 0;
> > - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg))
> > - ret = -errno;
> > - return ret;
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg)) {
> > + err = -errno;
> > + igt_assert(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static int create_ext_ioctl(int i915,
> > + struct drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > + err = -errno;
> > + igt_assume(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > }
> >
> > static double elapsed(const struct timespec *start,
> > @@ -308,6 +330,187 @@ static void maximum(int fd, int ncpus, unsigned mode)
> > free(contexts);
> > }
> >
> > +static void basic_ext_param(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS
> > + };
> > + struct drm_i915_gem_context_param get;
> > +
> > + igt_require(create_ext_ioctl(i915, &create) == 0);
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + create.extensions = -1ull;
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -EFAULT);
> > +
> > + create.extensions = to_user_pointer(&ext);
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -EINVAL);
>
> I think this is the unknown param, right?
>
> Need another -EINVAL test for non-zero ext.ctx_id.
No, this is non-zero ctx_id.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 05/16] i915/gem_ctx_create: Basic checks for constructor properties
Quoting Chris Wilson (2019-05-15 20:06:45)
> Quoting Tvrtko Ursulin (2019-05-14 13:27:38)
> >
> > On 08/05/2019 11:09, Chris Wilson wrote:
> > > Check that the extended create interface accepts setparam.
> > >
> > > Signed-off-by: Chris Wilson
> > > ---
> > > tests/i915/gem_ctx_create.c | 225 ++--
> > > 1 file changed, 213 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/tests/i915/gem_ctx_create.c b/tests/i915/gem_ctx_create.c
> > > index a664070db..9b4fddbe7 100644
> > > --- a/tests/i915/gem_ctx_create.c
> > > +++ b/tests/i915/gem_ctx_create.c
> > > @@ -33,6 +33,7 @@
> > > #include
> > >
> > > #include "igt_rand.h"
> > > +#include "sw_sync.h"
> > >
> > > #define LOCAL_I915_EXEC_BSD_SHIFT (13)
> > > #define LOCAL_I915_EXEC_BSD_MASK (3 << LOCAL_I915_EXEC_BSD_SHIFT)
> > > @@ -45,12 +46,33 @@ static unsigned all_nengine;
> > > static unsigned ppgtt_engines[16];
> > > static unsigned ppgtt_nengine;
> > >
> > > -static int __gem_context_create_local(int fd, struct
> > > drm_i915_gem_context_create *arg)
> > > +static int create_ioctl(int fd, struct drm_i915_gem_context_create *arg)
> > > {
> > > - int ret = 0;
> > > - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg))
> > > - ret = -errno;
> > > - return ret;
> > > + int err;
> > > +
> > > + err = 0;
> > > + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg)) {
> > > + err = -errno;
> > > + igt_assert(err);
> > > + }
> > > +
> > > + errno = 0;
> > > + return err;
> > > +}
> > > +
> > > +static int create_ext_ioctl(int i915,
> > > + struct drm_i915_gem_context_create_ext *arg)
> > > +{
> > > + int err;
> > > +
> > > + err = 0;
> > > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > > + err = -errno;
> > > + igt_assume(err);
> > > + }
> > > +
> > > + errno = 0;
> > > + return err;
> > > }
> > >
> > > static double elapsed(const struct timespec *start,
> > > @@ -308,6 +330,187 @@ static void maximum(int fd, int ncpus, unsigned
> > > mode)
> > > free(contexts);
> > > }
> > >
> > > +static void basic_ext_param(int i915)
> > > +{
> > > + struct drm_i915_gem_context_create_ext_setparam ext = {
> > > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > > + };
> > > + struct drm_i915_gem_context_create_ext create = {
> > > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS
> > > + };
> > > + struct drm_i915_gem_context_param get;
> > > +
> > > + igt_require(create_ext_ioctl(i915, &create) == 0);
> > > + gem_context_destroy(i915, create.ctx_id);
> > > +
> > > + create.extensions = -1ull;
> > > + igt_assert_eq(create_ext_ioctl(i915, &create), -EFAULT);
> > > +
> > > + create.extensions = to_user_pointer(&ext);
> > > + igt_assert_eq(create_ext_ioctl(i915, &create), -EINVAL);
> >
> > I think this is the unknown param, right?
> >
> > Need another -EINVAL test for non-zero ext.ctx_id.
>
> No, this is non-zero ctx_id.
No, I read the wrong ctx_id.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 05/16] i915/gem_ctx_create: Basic checks for constructor properties
Quoting Tvrtko Ursulin (2019-05-14 11:15:12)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Check that the extended create interface accepts setparam.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/i915/gem_ctx_create.c | 225 ++--
> > 1 file changed, 213 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/i915/gem_ctx_create.c b/tests/i915/gem_ctx_create.c
> > index a664070db..9b4fddbe7 100644
> > --- a/tests/i915/gem_ctx_create.c
> > +++ b/tests/i915/gem_ctx_create.c
> > @@ -33,6 +33,7 @@
> > #include
> >
> > #include "igt_rand.h"
> > +#include "sw_sync.h"
> >
> > #define LOCAL_I915_EXEC_BSD_SHIFT (13)
> > #define LOCAL_I915_EXEC_BSD_MASK (3 << LOCAL_I915_EXEC_BSD_SHIFT)
> > @@ -45,12 +46,33 @@ static unsigned all_nengine;
> > static unsigned ppgtt_engines[16];
> > static unsigned ppgtt_nengine;
> >
> > -static int __gem_context_create_local(int fd, struct
> > drm_i915_gem_context_create *arg)
> > +static int create_ioctl(int fd, struct drm_i915_gem_context_create *arg)
> > {
> > - int ret = 0;
> > - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg))
> > - ret = -errno;
> > - return ret;
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, arg)) {
> > + err = -errno;
> > + igt_assert(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static int create_ext_ioctl(int i915,
> > + struct drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > + err = -errno;
> > + igt_assume(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > }
> >
> > static double elapsed(const struct timespec *start,
> > @@ -308,6 +330,187 @@ static void maximum(int fd, int ncpus, unsigned mode)
> > free(contexts);
> > }
> >
> > +static void basic_ext_param(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS
> > + };
> > + struct drm_i915_gem_context_param get;
> > +
> > + igt_require(create_ext_ioctl(i915, &create) == 0);
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + create.extensions = -1ull;
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -EFAULT);
> > +
> > + create.extensions = to_user_pointer(&ext);
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -EINVAL);
> > +
> > + ext.param.param = I915_CONTEXT_PARAM_PRIORITY;
> > + if (create_ext_ioctl(i915, &create) != -ENODEV) {
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + ext.base.next_extension = -1ull;
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -EFAULT);
> > + ext.base.next_extension = to_user_pointer(&ext);
> > + igt_assert_eq(create_ext_ioctl(i915, &create), -E2BIG);
> > + ext.base.next_extension = 0;
> > +
> > + ext.param.value = 32;
> > + igt_assert_eq(create_ext_ioctl(i915, &create), 0);
> > +
> > + memset(&get, 0, sizeof(get));
> > + get.ctx_id = create.ctx_id;
> > + get.param = I915_CONTEXT_PARAM_PRIORITY;
> > + gem_context_get_param(i915, &get);
> > + igt_assert_eq(get.value, ext.param.value);
> > +
> > + gem_context_destroy(i915, create.ctx_id);
> > + }
> > +}
> > +
> > +static void check_single_timeline(int i915, uint32_t ctx, int num_engines)
> > +{
> > +#define RCS_TIMESTAMP (0x2000 + 0x358)
> > + const int gen = intel_gen(intel_get_drm_devid(i915));
> > + const int has_64bit_reloc = gen >= 8;
> > + struct drm_i915_gem_exec_object2 results = { .handle =
> > gem_create(i915, 4096) };
> > + const uint32_t bbe = MI_BATCH_BUFFER_END;
> > + int timeline = sw_sync_timeline_create();
> > + uint32_t last, *map;
> > +
> > + {
> > + struct drm_i915_gem_execbuffer2 execbuf = {
> > + .buffers_ptr = to_user_pointer(&results),
> > + .buffer_count = 1,
> > + .rsvd1 = ctx,
> > + };
> > + gem_write(i915, results.handle, 0, &bbe, sizeof(bbe));
> > + gem_execbuf(i915, &execbuf);
> > + results.flags = EXEC_OBJECT_PINNED;
> > + }
> > +
> > + for (int i = 0; i < num_engines; i++) {
> > + struct drm_i915_gem_exec_object2 obj[2] = {
> > + results, /* write hazard lies! */
> > + { .handle = gem_create(i915, 4096) },
> > + };
> > + struct drm_i915_gem_execbuffer2
Re: [Intel-gfx] [v10 01/12] drm: Add HDR source metadata property
On Tue, May 14, 2019 at 11:06:23PM +0530, Uma Shankar wrote:
> This patch adds a blob property to get HDR metadata
> information from userspace. This will be send as part
> of AVI Infoframe to panel.
>
> It also implements get() and set() functions for HDR output
> metadata property.The blob data is received from userspace and
> saved in connector state, the same is returned as blob in get
> property call to userspace.
>
> v2: Rebase and modified the metadata structure elements
> as per Ville's POC changes.
>
> v3: No Change
>
> v4: Addressed Shashank's review comments
>
> v5: Rebase.
>
> v6: Addressed Brian Starkey's review comments, defined
> new structure with header for dynamic metadata scalability.
> Merge get/set property functions for metadata in this patch.
>
> v7: Addressed Jonas Karlman review comments and defined separate
> structure for infoframe to better align with CTA 861.G spec. Added
> Shashank's RB.
>
> v8: Addressed Ville's review comments. Moved sink metadata structure
> out of uapi headers as suggested by Jonas Karlman.
>
> v9: Rebase and addressed Jonas Karlman review comments.
>
> Signed-off-by: Uma Shankar
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_atomic.c | 2 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
> drivers/gpu/drm/drm_connector.c | 6 ++
> include/drm/drm_connector.h | 11 +++
> include/drm/drm_mode_config.h | 7 +++
> include/linux/hdmi.h | 26 ++
> include/uapi/drm/drm_mode.h | 23 +++
> 7 files changed, 88 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f4924cb..0d27195 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -925,6 +925,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
>
> drm_printf(p, "connector[%u]: %s\n", connector->base.id,
> connector->name);
> drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
> "(null)");
> + drm_printf(p, "\thdr_metadata_changed=%d\n",
> +state->hdr_metadata_changed);
>
> if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> if (state->writeback_job && state->writeback_job->fb)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 4131e66..4aa82c91 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> {
> struct drm_device *dev = connector->dev;
> struct drm_mode_config *config = &dev->mode_config;
> + bool replaced = false;
> + int ret;
>
> if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val);
> @@ -726,6 +728,14 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
>*/
> if (state->link_status != DRM_LINK_STATUS_GOOD)
> state->link_status = val;
> + } else if (property == config->hdr_output_metadata_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + &state->hdr_output_metadata,
> + val,
> + -1, sizeof(struct hdr_output_metadata),
Those function arguments are nonsesne for a blob that isn't an array.
> + &replaced);
> + state->hdr_metadata_changed |= replaced;
> + return ret;
> } else if (property == config->aspect_ratio_property) {
> state->picture_aspect_ratio = val;
> } else if (property == config->content_type_property) {
> @@ -814,6 +824,9 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> *val = state->colorspace;
> } else if (property == connector->scaling_mode_property) {
> *val = state->scaling_mode;
> + } else if (property == config->hdr_output_metadata_property) {
> + *val = state->hdr_output_metadata ?
> + state->hdr_output_metadata->base.id : 0;
> } else if (property == config->content_protection_property) {
> *val = state->content_protection;
> } else if (property == config->writeback_fb_id_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 11fcd25..c9ac8b9 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1051,6 +1051,12 @@ int drm_connector_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.non_desktop_property = prop;
>
> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
> +"HDR_OUTPUT_METADATA", 0);
> + if (!prop)
Re: [Intel-gfx] [PATCH i-g-t 07/16] i915: Add gem_ctx_clone
Quoting Tvrtko Ursulin (2019-05-14 13:41:13)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Exercise cloning contexts, an extension of merely creating one.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/Makefile.sources | 1 +
> > tests/i915/gem_ctx_clone.c | 460 +
> > tests/meson.build | 1 +
> > 3 files changed, 462 insertions(+)
> > create mode 100644 tests/i915/gem_ctx_clone.c
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 1a541d206..e1b7feeb2 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -21,6 +21,7 @@ TESTS_progs = \
> > drm_import_export \
> > drm_mm \
> > drm_read \
> > + i915/gem_ctx_clone \
> > i915/gem_vm_create \
> > kms_3d \
> > kms_addfb_basic \
> > diff --git a/tests/i915/gem_ctx_clone.c b/tests/i915/gem_ctx_clone.c
> > new file mode 100644
> > index 0..cdc5bf413
> > --- /dev/null
> > +++ b/tests/i915/gem_ctx_clone.c
> > @@ -0,0 +1,460 @@
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > next
> > + * paragraph) shall be included in all copies or substantial portions of
> > the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_gt.h"
> > +#include "i915/gem_vm.h"
> > +#include "i915_drm.h"
> > +
> > +static int ctx_create_ioctl(int i915, struct
> > drm_i915_gem_context_create_ext *arg)
> > +{
> > + int err;
> > +
> > + err = 0;
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > + err = -errno;
> > + igt_assume(err);
> > + }
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static bool has_ctx_clone(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .clone_id = -1,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > + return ctx_create_ioctl(i915, &create) == -ENOENT;
> > +}
> > +
> > +static void invalid_clone(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&ext),
> > + };
> > +
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), 0);
> > + gem_context_destroy(i915, create.ctx_id);
> > +
> > + ext.flags = -1; /* Hopefully we won't run out of flags */
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -EINVAL);
> > + ext.flags = 0;
> > +
> > + ext.base.next_extension = -1;
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -EFAULT);
> > + ext.base.next_extension = to_user_pointer(&ext);
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -E2BIG);
> > + ext.base.next_extension = 0;
> > +
> > + ext.clone_id = -1;
> > + igt_assert_eq(ctx_create_ioctl(i915, &create), -ENOENT);
> > + ext.clone_id = 0;
> > +}
> > +
> > +static void clone_flags(int i915)
> > +{
> > + struct drm_i915_gem_context_create_ext_setparam set = {
> > + { .name = I915_CONTEXT_CREATE_EXT_SETPARAM },
> > + { .param = I915_CONTEXT_PARAM_RECOVERABLE },
> > + };
> > + struct drm_i915_gem_context_create_ext_clone ext = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .flags = I915_CONTEXT_CLONE_FLAGS,
> > + };
> > + struct drm_i915_gem_context_create_ext create = {
> > + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .
Re: [Intel-gfx] [v10 04/12] drm: Enable HDR infoframe support
On Tue, May 14, 2019 at 11:06:26PM +0530, Uma Shankar wrote:
> Enable Dynamic Range and Mastering Infoframe for HDR
> content, which is defined in CEA 861.3 spec.
>
> The metadata will be computed based on blending
> policy in userspace compositors and passed as a connector
> property blob to driver. The same will be sent as infoframe
> to panel which support HDR.
>
> Added the const version of infoframe for DRM metadata
> for HDR.
>
> v2: Rebase and added Ville's POC changes.
>
> v3: No Change
>
> v4: Addressed Shashank's review comments and merged the
> patch making drm infoframe function arguments as constant.
>
> v5: Rebase
>
> v6: Fixed checkpatch warnings with --strict option. Addressed
> Shashank's review comments and added his RB.
>
> v7: Addressed Brian Starkey's review comments. Merged 2 patches
> into one.
>
> v8: Addressed Jonas Karlman review comments.
>
> v9: Addressed Jonas Karlman review comments.
>
> Signed-off-by: Uma Shankar
> Signed-off-by: Ville Syrjälä
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_edid.c | 43 +++
> drivers/video/hdmi.c | 187
> +
> include/drm/drm_edid.h | 5 ++
> include/linux/hdmi.h | 27 +++
> 4 files changed, 262 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 2e0b5be..73b7905 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4903,6 +4903,49 @@ static bool is_hdmi2_sink(struct drm_connector
> *connector)
> }
>
> /**
> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with
> + * HDR metadata from userspace
> + * @frame: HDMI DRM infoframe
> + * @hdr_metadata: hdr_source_metadata info from userspace
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int
> +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> + const struct hdr_output_metadata
> *hdr_metadata)
> +{
> + int err;
> +
> + if (!frame || !hdr_metadata)
> + return -EINVAL;
> +
> + err = hdmi_drm_infoframe_init(frame);
> + if (err < 0)
> + return err;
> +
> + DRM_DEBUG_KMS("type = %x\n", frame->type);
This debug seems pointless.
> +
> + frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
> + frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
> +
> + memcpy(&frame->display_primaries,
> +&hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
sizeof() can be used here.
Could also slap on a BUILD_BUG_ON() to make sure both are the same size.
> +
> + memcpy(&frame->white_point,
> +&hdr_metadata->hdmi_metadata_type1.white_point, 4);
ditto.
> +
> + frame->max_display_mastering_luminance =
> +
> hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
> + frame->min_display_mastering_luminance =
> +
> hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
> + frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
> + frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
> +
> +/**
> * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe
> with
> * data from a DRM display mode
> * @frame: HDMI AVI infoframe
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 799ae49..c5ecd16 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -650,6 +650,147 @@ ssize_t hdmi_vendor_infoframe_pack(struct
> hdmi_vendor_infoframe *frame,
> return 0;
> }
>
> +/**
> + * hdmi_drm_infoframe_init() - initialize an HDMI Dynaminc Range and
> + * mastering infoframe
> + * @frame: HDMI DRM infoframe
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
> +{
> + memset(frame, 0, sizeof(*frame));
> +
> + frame->type = HDMI_INFOFRAME_TYPE_DRM;
> + frame->version = 1;
> + frame->length = HDMI_DRM_INFOFRAME_SIZE;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(hdmi_drm_infoframe_init);
> +
> +static int hdmi_drm_infoframe_check_only(const struct hdmi_drm_infoframe
> *frame)
> +{
> + if (frame->type != HDMI_INFOFRAME_TYPE_DRM ||
> + frame->version != 1)
Missing the length check.
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
> + * @frame: HDMI DRM infoframe
> + *
> + * Validates that the infoframe is consistent.
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame)
> +{
> + return hdmi_drm_infoframe_check_only(frame);
> +}
> +EXPORT_SYMBOL(hdmi_drm_infoframe_
Re: [Intel-gfx] [v10 09/12] drm/i915:Enabled Modeset when HDR Infoframe changes
On Tue, May 14, 2019 at 11:06:31PM +0530, Uma Shankar wrote:
> This patch enables modeset whenever HDR metadata
> needs to be updated to sink.
>
> v2: Addressed Shashank's review comments.
>
> v3: Added Shashank's RB.
>
> v4: Addressed Ville's review comments.
>
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Uma Shankar
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 14 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 13 +
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 58b8049..6b985e8 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -105,6 +105,16 @@ int intel_digital_connector_atomic_set_property(struct
> drm_connector *connector,
> return -EINVAL;
> }
>
> +static bool blob_equal(const struct drm_property_blob *a,
> +const struct drm_property_blob *b)
> +{
> + if (a && b)
> + return a->length == b->length &&
> + !memcmp(a->data, b->data, a->length);
> +
> + return !a == !b;
> +}
> +
> int intel_digital_connector_atomic_check(struct drm_connector *conn,
>struct drm_connector_state *new_state)
> {
> @@ -132,7 +142,9 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> new_conn_state->base.colorspace != old_conn_state->base.colorspace
> ||
> new_conn_state->base.picture_aspect_ratio !=
> old_conn_state->base.picture_aspect_ratio ||
> new_conn_state->base.content_type !=
> old_conn_state->base.content_type ||
> - new_conn_state->base.scaling_mode !=
> old_conn_state->base.scaling_mode)
> + new_conn_state->base.scaling_mode !=
> old_conn_state->base.scaling_mode ||
> + !blob_equal(new_conn_state->base.hdr_output_metadata,
> + old_conn_state->base.hdr_output_metadata))
> crtc_state->mode_changed = true;
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index b80406b..e97bf6e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -806,6 +806,11 @@ void intel_read_infoframe(struct intel_encoder *encoder,
> return true;
> }
>
> +static inline bool is_eotf_supported(u8 output_eotf, u8 sink_eotf)
> +{
> + return sink_eotf & BIT(output_eotf);
> +}
> +
> static bool
> intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
>struct intel_crtc_state *crtc_state,
> @@ -814,6 +819,7 @@ void intel_read_infoframe(struct intel_encoder *encoder,
> struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm;
> struct hdr_output_metadata *hdr_metadata;
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct drm_connector *connector = conn_state->connector;
> int ret;
>
> if (!(INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)))
> @@ -828,6 +834,13 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>
> hdr_metadata = conn_state->hdr_output_metadata->data;
>
> + /* Sink EOTF is Bit map while infoframe is absolute values */
> + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
> + connector->hdr_sink_metadata.hdmi_type1.eotf)) {
> + DRM_ERROR("EOTF Not Supported\n");
> + return true;
> + }
I was going to say that this should probably be in the
drm_set_hdr_metdata() or whatever it was called.
But now I'm now wondering if we can even have this check
here. What happens if someone does a display switcheroo
while the machine is suspended? Depends on when we're going to
reprobe the displays I suppose. Hmm. Maybe it's fine. We already
have a similar issue after all wih the has_hdmi2_sink stuff.
Either way the user triggerable DRM_ERROR()s are at least a nono.
> +
> crtc_state->infoframes.enable |=
> intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
>
> --
> 1.9.1
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v10 12/12] drm/i915: Add state readout for DRM infoframe
On Tue, May 14, 2019 at 11:06:34PM +0530, Uma Shankar wrote: > Added state readout for DRM infoframe and enabled > state validation for DRM infoframe. > > v2: Addressed Ville's review comments and dropped the > unused drm infoframe read at intel_hdmi_init. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_ddi.c | 4 > drivers/gpu/drm/i915/intel_display.c | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 0af47f3..f574315 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3834,6 +3834,10 @@ void intel_ddi_get_config(struct intel_encoder > *encoder, > intel_read_infoframe(encoder, pipe_config, >HDMI_INFOFRAME_TYPE_VENDOR, >&pipe_config->infoframes.hdmi); > + if ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))) Silly extra parens. Actually, I think the check can be removed entirely since intel_read_infoframe() also checks infoframes.enable. > + intel_read_infoframe(encoder, pipe_config, > + HDMI_INFOFRAME_TYPE_DRM, > + &pipe_config->infoframes.drm); > } > > static enum intel_output_type > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index e35ba8d..c89b214 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12274,6 +12274,7 @@ static bool fastboot_enabled(struct drm_i915_private > *dev_priv) > PIPE_CONF_CHECK_INFOFRAME(avi); > PIPE_CONF_CHECK_INFOFRAME(spd); > PIPE_CONF_CHECK_INFOFRAME(hdmi); > + PIPE_CONF_CHECK_INFOFRAME(drm); > > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > -- > 1.9.1 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling in DRM layer (rev10)
On Wed, May 15, 2019 at 08:59:37AM +, Shankar, Uma wrote: > > > >-Original Message- > >From: Patchwork [mailto:patchw...@emeril.freedesktop.org] > >Sent: Wednesday, May 15, 2019 6:54 AM > >To: Shankar, Uma > >Cc: intel-gfx@lists.freedesktop.org > >Subject: ✗ Fi.CI.IGT: failure for Add HDR Metadata Parsing and handling in > >DRM layer > >(rev10) > > > >== Series Details == > > > >Series: Add HDR Metadata Parsing and handling in DRM layer (rev10) > >URL : https://patchwork.freedesktop.org/series/25091/ > >State : failure > > > >== Summary == > > > >CI Bug Log - changes from CI_DRM_6081_full -> Patchwork_13017_full > > > > > >Summary > >--- > > > > **FAILURE** > > > > Serious unknown changes coming with Patchwork_13017_full absolutely need > > to be > > verified manually. > > > > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_13017_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_13017_full: > > > >### IGT changes ### > > > > Possible regressions > > > > * igt@gem_exec_suspend@basic-s3: > >- shard-iclb: [PASS][1] -> [SKIP][2] +43 similar issues > > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard- > >iclb6/igt@gem_exec_susp...@basic-s3.html > > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard- > >iclb5/igt@gem_exec_susp...@basic-s3.html > > > > * igt@kms_prop_blob@invalid-set-prop-any: > >- shard-iclb: [PASS][3] -> [FAIL][4] > > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6081/shard- > >iclb6/igt@kms_prop_b...@invalid-set-prop-any.html > > [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13017/shard- > >iclb5/igt@kms_prop_b...@invalid-set-prop-any.html > > > > Hi Martin, > These issues are unrelated to the changes made in this series. Can you please > have a look > and confirm. The kms_prop fails at least are real. Probably due to the bogus function arguements to the replace_blob() thing I pointed out. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v10 01/12] drm: Add HDR source metadata property
On 2019-05-15 21:10, Ville Syrjälä wrote:
> On Tue, May 14, 2019 at 11:06:23PM +0530, Uma Shankar wrote:
>> This patch adds a blob property to get HDR metadata
>> information from userspace. This will be send as part
>> of AVI Infoframe to panel.
>>
>> It also implements get() and set() functions for HDR output
>> metadata property.The blob data is received from userspace and
>> saved in connector state, the same is returned as blob in get
>> property call to userspace.
>>
>> v2: Rebase and modified the metadata structure elements
>> as per Ville's POC changes.
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> v5: Rebase.
>>
>> v6: Addressed Brian Starkey's review comments, defined
>> new structure with header for dynamic metadata scalability.
>> Merge get/set property functions for metadata in this patch.
>>
>> v7: Addressed Jonas Karlman review comments and defined separate
>> structure for infoframe to better align with CTA 861.G spec. Added
>> Shashank's RB.
>>
>> v8: Addressed Ville's review comments. Moved sink metadata structure
>> out of uapi headers as suggested by Jonas Karlman.
>>
>> v9: Rebase and addressed Jonas Karlman review comments.
>>
>> Signed-off-by: Uma Shankar
>> Reviewed-by: Shashank Sharma
>> ---
>> drivers/gpu/drm/drm_atomic.c | 2 ++
>> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
>> drivers/gpu/drm/drm_connector.c | 6 ++
>> include/drm/drm_connector.h | 11 +++
>> include/drm/drm_mode_config.h | 7 +++
>> include/linux/hdmi.h | 26 ++
>> include/uapi/drm/drm_mode.h | 23 +++
>> 7 files changed, 88 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index f4924cb..0d27195 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -925,6 +925,8 @@ static void drm_atomic_connector_print_state(struct
>> drm_printer *p,
>>
>> drm_printf(p, "connector[%u]: %s\n", connector->base.id,
>> connector->name);
>> drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
>> "(null)");
>> +drm_printf(p, "\thdr_metadata_changed=%d\n",
>> + state->hdr_metadata_changed);
>>
>> if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>> if (state->writeback_job && state->writeback_job->fb)
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 4131e66..4aa82c91 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
>> {
>> struct drm_device *dev = connector->dev;
>> struct drm_mode_config *config = &dev->mode_config;
>> +bool replaced = false;
>> +int ret;
>>
>> if (property == config->prop_crtc_id) {
>> struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val);
>> @@ -726,6 +728,14 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
>> */
>> if (state->link_status != DRM_LINK_STATUS_GOOD)
>> state->link_status = val;
>> +} else if (property == config->hdr_output_metadata_property) {
>> +ret = drm_atomic_replace_property_blob_from_id(dev,
>> +&state->hdr_output_metadata,
>> +val,
>> +-1, sizeof(struct hdr_output_metadata),
> Those function arguments are nonsesne for a blob that isn't an array.
>
>> +&replaced);
>> +state->hdr_metadata_changed |= replaced;
>> +return ret;
>> } else if (property == config->aspect_ratio_property) {
>> state->picture_aspect_ratio = val;
>> } else if (property == config->content_type_property) {
>> @@ -814,6 +824,9 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
>> *val = state->colorspace;
>> } else if (property == connector->scaling_mode_property) {
>> *val = state->scaling_mode;
>> +} else if (property == config->hdr_output_metadata_property) {
>> +*val = state->hdr_output_metadata ?
>> +state->hdr_output_metadata->base.id : 0;
>> } else if (property == config->content_protection_property) {
>> *val = state->content_protection;
>> } else if (property == config->writeback_fb_id_property) {
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c
>> index 11fcd25..c9ac8b9 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1051,6 +1051,12 @@ int drm_connector_create_standard_properties(struct
>> drm_device *dev)
>> return -ENOMEM;
>> dev->mode_config.non_desktop_property = prop;
>>
>> +prop = drm_property_c
Re: [Intel-gfx] [PATCH i-g-t 08/16] i915: Exercise creating context with shared GTT
Quoting Tvrtko Ursulin (2019-05-15 07:37:18)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > v2: Test each shared context is its own timeline and allows request
> > reordering between shared contexts.
> >
> > Signed-off-by: Chris Wilson
> > Cc: Joonas Lahtinen
> > Cc: Tvrtko Ursulin
> > Cc: Mika Kuoppala
> > Cc: Michal Wajdeczko
> > ---
> > lib/i915/gem_context.c| 68 +++
> > lib/i915/gem_context.h| 13 +
> > tests/Makefile.sources| 1 +
> > tests/i915/gem_ctx_shared.c | 856 ++
> > tests/i915/gem_exec_whisper.c | 32 +-
> > tests/meson.build | 1 +
> > 6 files changed, 962 insertions(+), 9 deletions(-)
> > create mode 100644 tests/i915/gem_ctx_shared.c
> >
> > diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
> > index f94d89cb4..8fb8984d1 100644
> > --- a/lib/i915/gem_context.c
> > +++ b/lib/i915/gem_context.c
> > @@ -272,6 +272,74 @@ void gem_context_set_priority(int fd, uint32_t ctx_id,
> > int prio)
> > igt_assert_eq(__gem_context_set_priority(fd, ctx_id, prio), 0);
> > }
> >
> > +int
> > +__gem_context_clone(int i915,
> > + uint32_t src, unsigned int share,
> > + unsigned int flags,
> > + uint32_t *out)
> > +{
> > + struct drm_i915_gem_context_create_ext_clone clone = {
> > + { .name = I915_CONTEXT_CREATE_EXT_CLONE },
> > + .clone_id = src,
> > + .flags = share,
> > + };
> > + struct drm_i915_gem_context_create_ext arg = {
> > + .flags = flags | I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > + .extensions = to_user_pointer(&clone),
> > + };
> > + int err = 0;
> > +
> > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, &arg))
> > + err = -errno;
> > +
> > + *out = arg.ctx_id;
> > +
> > + errno = 0;
> > + return err;
> > +}
> > +
> > +static bool __gem_context_has(int i915, uint32_t share, unsigned int flags)
> > +{
> > + uint32_t ctx;
> > +
> > + __gem_context_clone(i915, 0, share, flags, &ctx);
> > + if (ctx)
> > + gem_context_destroy(i915, ctx);
> > +
> > + errno = 0;
> > + return ctx;
> > +}
> > +
> > +bool gem_contexts_has_shared_gtt(int i915)
> > +{
> > + return __gem_context_has(i915, I915_CONTEXT_CLONE_VM, 0);
> > +}
> > +
> > +bool gem_has_queues(int i915)
> > +{
> > + return __gem_context_has(i915,
> > + I915_CONTEXT_CLONE_VM,
> > + I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE);
> > +}
> > +
> > +uint32_t gem_context_clone(int i915,
> > +uint32_t src, unsigned int share,
> > +unsigned int flags)
> > +{
> > + uint32_t ctx;
> > +
> > + igt_assert_eq(__gem_context_clone(i915, src, share, flags, &ctx), 0);
> > +
> > + return ctx;
> > +}
> > +
> > +uint32_t gem_queue_create(int i915)
> > +{
> > + return gem_context_clone(i915, 0,
> > + I915_CONTEXT_CLONE_VM,
> > + I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE);
> > +}
> > +
> > bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
> > {
> > struct drm_i915_gem_exec_object2 exec = {};
> > diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
> > index a052714d4..8043c3401 100644
> > --- a/lib/i915/gem_context.h
> > +++ b/lib/i915/gem_context.h
> > @@ -29,6 +29,19 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
> > void gem_context_destroy(int fd, uint32_t ctx_id);
> > int __gem_context_destroy(int fd, uint32_t ctx_id);
> >
> > +int __gem_context_clone(int i915,
> > + uint32_t src, unsigned int share,
> > + unsigned int flags,
> > + uint32_t *out);
> > +uint32_t gem_context_clone(int i915,
> > +uint32_t src, unsigned int share,
> > +unsigned int flags);
> > +
> > +uint32_t gem_queue_create(int i915);
> > +
> > +bool gem_contexts_has_shared_gtt(int i915);
> > +bool gem_has_queues(int i915);
> > +
> > bool gem_has_contexts(int fd);
> > void gem_require_contexts(int fd);
> > void gem_context_require_bannable(int fd);
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index e1b7feeb2..3552e895b 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -22,6 +22,7 @@ TESTS_progs = \
> > drm_mm \
> > drm_read \
> > i915/gem_ctx_clone \
> > + i915/gem_ctx_shared \
> > i915/gem_vm_create \
> > kms_3d \
> > kms_addfb_basic \
> > diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c
> > new file mode 100644
> > index 0..0076f5e9d
> > --- /dev/null
> > +++ b/tests/i915/gem_ctx_shared.c
> > @@ -0,0 +1,856 @@
> > +/*
> > + * Copyright © 2017 Intel Corporation
>
> 2019
Nah, that would imply I put any thought into t
Re: [Intel-gfx] [PATCH i-g-t 10/16] i915/gem_exec_whisper: Fork all-engine tests one-per-engine
Quoting Tvrtko Ursulin (2019-05-14 13:57:26)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Add a new mode for some more stress, submit the all-engines tests
> > simultaneously, a stream per engine.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/i915/gem_exec_whisper.c | 27 ++-
> > 1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/i915/gem_exec_whisper.c b/tests/i915/gem_exec_whisper.c
> > index d3e0b0ba2..d5afc8119 100644
> > --- a/tests/i915/gem_exec_whisper.c
> > +++ b/tests/i915/gem_exec_whisper.c
> > @@ -88,6 +88,7 @@ static void verify_reloc(int fd, uint32_t handle,
> > #define SYNC 0x40
> > #define PRIORITY 0x80
> > #define QUEUES 0x100
> > +#define ALL 0x200
> >
> > struct hang {
> > struct drm_i915_gem_exec_object2 obj;
> > @@ -199,6 +200,7 @@ static void whisper(int fd, unsigned engine, unsigned
> > flags)
> > uint64_t old_offset;
> > int i, n, loc;
> > int debugfs;
> > + int nchild;
> >
> > if (flags & PRIORITY) {
> > igt_require(gem_scheduler_enabled(fd));
> > @@ -215,6 +217,7 @@ static void whisper(int fd, unsigned engine, unsigned
> > flags)
> > engines[nengine++] = engine;
> > }
> > } else {
> > + igt_assert(!(flags & ALL));
> > igt_require(gem_has_ring(fd, engine));
> > igt_require(gem_can_store_dword(fd, engine));
> > engines[nengine++] = engine;
> > @@ -233,11 +236,22 @@ static void whisper(int fd, unsigned engine, unsigned
> > flags)
> > if (flags & HANG)
> > init_hang(&hang);
> >
> > + nchild = 1;
> > + if (flags & FORKED)
> > + nchild *= sysconf(_SC_NPROCESSORS_ONLN);
> > + if (flags & ALL)
> > + nchild *= nengine;
> > +
> > intel_detect_and_clear_missed_interrupts(fd);
> > gpu_power_read(&power, &sample[0]);
> > - igt_fork(child, flags & FORKED ? sysconf(_SC_NPROCESSORS_ONLN) : 1) {
> > + igt_fork(child, nchild) {
> > unsigned int pass;
> >
> > + if (flags & ALL) {
> > + engines[0] = engines[child % nengine];
>
> Relying on PIDs being sequential feels fragile but suggesting pipes or
> shared memory would be overkill. How about another loop:
Where are you getting pid_t from? child is an integer [0, nchild).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v10 03/12] drm: Parse HDR metadata info from EDID
On Tue, May 14, 2019 at 11:06:25PM +0530, Uma Shankar wrote:
> HDR metadata block is introduced in CEA-861.3 spec.
> Parsing the same to get the panel's HDR metadata.
>
> v2: Rebase and added Ville's POC changes to the patch.
>
> v3: No Change
>
> v4: Addressed Shashank's review comments
>
> v5: Addressed Shashank's comment and added his RB.
>
> v6: Addressed Jonas Karlman review comments.
>
> v7: Adressed Ville's review comments and fixed the issue
> with length handling.
>
> Signed-off-by: Uma Shankar
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_edid.c | 49
> ++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 852bdd8..2e0b5be 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2852,6 +2852,7 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> #define VIDEO_BLOCK 0x02
> #define VENDOR_BLOCK0x03
> #define SPEAKER_BLOCK0x04
> +#define HDR_STATIC_METADATA_BLOCK0x6
> #define USE_EXTENDED_TAG 0x07
> #define EXT_VIDEO_CAPABILITY_BLOCK 0x00
> #define EXT_VIDEO_DATA_BLOCK_420 0x0E
> @@ -3834,6 +3835,52 @@ static void fixup_detailed_cea_mode_clock(struct
> drm_display_mode *mode)
> mode->clock = clock;
> }
>
> +static bool cea_db_is_hdmi_hdr_metadata_block(const u8 *db)
> +{
> + if (cea_db_tag(db) != USE_EXTENDED_TAG)
> + return false;
> +
> + if (db[1] != HDR_STATIC_METADATA_BLOCK)
> + return false;
> +
> + return true;
> +}
> +
> +static uint8_t eotf_supported(const u8 *edid_ext)
> +{
> + return edid_ext[2] &
> + (BIT(HDMI_EOTF_TRADITIONAL_GAMMA_SDR) |
> + BIT(HDMI_EOTF_TRADITIONAL_GAMMA_HDR) |
> + BIT(HDMI_EOTF_SMPTE_ST2084));
> +}
> +
> +static uint8_t hdr_metadata_type(const u8 *edid_ext)
> +{
> + return edid_ext[3] &
> + BIT(HDMI_STATIC_METADATA_TYPE1);
> +}
> +
> +static void
> +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db)
> +{
> + u16 len;
> +
> + len = cea_db_payload_len(db);
> + if (len >= 3) {
I believe in other cases we've put the length check for the
mandatory bytes into the cea_db_is_foo() function. Would be
good to follow the path laid out by existing code here too.
> + connector->hdr_sink_metadata.hdmi_type1.eotf =
> + eotf_supported(db);
> + connector->hdr_sink_metadata.hdmi_type1.metadata_type =
> + hdr_metadata_type(db);
> + }
> +
> + if (len >= 4)
> + connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4];
> + if (len >= 5)
> + connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
> + if (len >= 6)
> + connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
> +}
> +
> static void
> drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
> {
> @@ -4461,6 +4508,8 @@ static void drm_parse_cea_ext(struct drm_connector
> *connector,
> drm_parse_y420cmdb_bitmap(connector, db);
> if (cea_db_is_vcdb(db))
> drm_parse_vcdb(connector, db);
> + if (cea_db_is_hdmi_hdr_metadata_block(db))
> + drm_parse_hdr_metadata_block(connector, db);
> }
> }
>
> --
> 1.9.1
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v10 01/12] drm: Add HDR source metadata property
On Wed, May 15, 2019 at 07:33:10PM +, Jonas Karlman wrote:
> On 2019-05-15 21:10, Ville Syrjälä wrote:
> > On Tue, May 14, 2019 at 11:06:23PM +0530, Uma Shankar wrote:
> >> This patch adds a blob property to get HDR metadata
> >> information from userspace. This will be send as part
> >> of AVI Infoframe to panel.
> >>
> >> It also implements get() and set() functions for HDR output
> >> metadata property.The blob data is received from userspace and
> >> saved in connector state, the same is returned as blob in get
> >> property call to userspace.
> >>
> >> v2: Rebase and modified the metadata structure elements
> >> as per Ville's POC changes.
> >>
> >> v3: No Change
> >>
> >> v4: Addressed Shashank's review comments
> >>
> >> v5: Rebase.
> >>
> >> v6: Addressed Brian Starkey's review comments, defined
> >> new structure with header for dynamic metadata scalability.
> >> Merge get/set property functions for metadata in this patch.
> >>
> >> v7: Addressed Jonas Karlman review comments and defined separate
> >> structure for infoframe to better align with CTA 861.G spec. Added
> >> Shashank's RB.
> >>
> >> v8: Addressed Ville's review comments. Moved sink metadata structure
> >> out of uapi headers as suggested by Jonas Karlman.
> >>
> >> v9: Rebase and addressed Jonas Karlman review comments.
> >>
> >> Signed-off-by: Uma Shankar
> >> Reviewed-by: Shashank Sharma
> >> ---
> >> drivers/gpu/drm/drm_atomic.c | 2 ++
> >> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
> >> drivers/gpu/drm/drm_connector.c | 6 ++
> >> include/drm/drm_connector.h | 11 +++
> >> include/drm/drm_mode_config.h | 7 +++
> >> include/linux/hdmi.h | 26 ++
> >> include/uapi/drm/drm_mode.h | 23 +++
> >> 7 files changed, 88 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index f4924cb..0d27195 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -925,6 +925,8 @@ static void drm_atomic_connector_print_state(struct
> >> drm_printer *p,
> >>
> >>drm_printf(p, "connector[%u]: %s\n", connector->base.id,
> >> connector->name);
> >>drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
> >> "(null)");
> >> + drm_printf(p, "\thdr_metadata_changed=%d\n",
> >> + state->hdr_metadata_changed);
> >>
> >>if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> >>if (state->writeback_job && state->writeback_job->fb)
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 4131e66..4aa82c91 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct
> >> drm_connector *connector,
> >> {
> >>struct drm_device *dev = connector->dev;
> >>struct drm_mode_config *config = &dev->mode_config;
> >> + bool replaced = false;
> >> + int ret;
> >>
> >>if (property == config->prop_crtc_id) {
> >>struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val);
> >> @@ -726,6 +728,14 @@ static int drm_atomic_connector_set_property(struct
> >> drm_connector *connector,
> >> */
> >>if (state->link_status != DRM_LINK_STATUS_GOOD)
> >>state->link_status = val;
> >> + } else if (property == config->hdr_output_metadata_property) {
> >> + ret = drm_atomic_replace_property_blob_from_id(dev,
> >> + &state->hdr_output_metadata,
> >> + val,
> >> + -1, sizeof(struct hdr_output_metadata),
> > Those function arguments are nonsesne for a blob that isn't an array.
> >
> >> + &replaced);
> >> + state->hdr_metadata_changed |= replaced;
> >> + return ret;
> >>} else if (property == config->aspect_ratio_property) {
> >>state->picture_aspect_ratio = val;
> >>} else if (property == config->content_type_property) {
> >> @@ -814,6 +824,9 @@ static int drm_atomic_connector_set_property(struct
> >> drm_connector *connector,
> >>*val = state->colorspace;
> >>} else if (property == connector->scaling_mode_property) {
> >>*val = state->scaling_mode;
> >> + } else if (property == config->hdr_output_metadata_property) {
> >> + *val = state->hdr_output_metadata ?
> >> + state->hdr_output_metadata->base.id : 0;
> >>} else if (property == config->content_protection_property) {
> >>*val = state->content_protection;
> >>} else if (property == config->writeback_fb_id_property) {
> >> diff --git a/drivers/gpu/drm/drm_connector.c
> >> b/drivers/gpu/drm/drm_connector.c
> >> index 11fcd25..c9ac8b9 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @
Re: [Intel-gfx] [v10 01/12] drm: Add HDR source metadata property
On 2019-05-15 21:45, Ville Syrjälä wrote:
> On Wed, May 15, 2019 at 07:33:10PM +, Jonas Karlman wrote:
>> On 2019-05-15 21:10, Ville Syrjälä wrote:
>>> On Tue, May 14, 2019 at 11:06:23PM +0530, Uma Shankar wrote:
This patch adds a blob property to get HDR metadata
information from userspace. This will be send as part
of AVI Infoframe to panel.
It also implements get() and set() functions for HDR output
metadata property.The blob data is received from userspace and
saved in connector state, the same is returned as blob in get
property call to userspace.
v2: Rebase and modified the metadata structure elements
as per Ville's POC changes.
v3: No Change
v4: Addressed Shashank's review comments
v5: Rebase.
v6: Addressed Brian Starkey's review comments, defined
new structure with header for dynamic metadata scalability.
Merge get/set property functions for metadata in this patch.
v7: Addressed Jonas Karlman review comments and defined separate
structure for infoframe to better align with CTA 861.G spec. Added
Shashank's RB.
v8: Addressed Ville's review comments. Moved sink metadata structure
out of uapi headers as suggested by Jonas Karlman.
v9: Rebase and addressed Jonas Karlman review comments.
Signed-off-by: Uma Shankar
Reviewed-by: Shashank Sharma
---
drivers/gpu/drm/drm_atomic.c | 2 ++
drivers/gpu/drm/drm_atomic_uapi.c | 13 +
drivers/gpu/drm/drm_connector.c | 6 ++
include/drm/drm_connector.h | 11 +++
include/drm/drm_mode_config.h | 7 +++
include/linux/hdmi.h | 26 ++
include/uapi/drm/drm_mode.h | 23 +++
7 files changed, 88 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f4924cb..0d27195 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -925,6 +925,8 @@ static void drm_atomic_connector_print_state(struct
drm_printer *p,
drm_printf(p, "connector[%u]: %s\n", connector->base.id,
connector->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
"(null)");
+ drm_printf(p, "\thdr_metadata_changed=%d\n",
+ state->hdr_metadata_changed);
if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
if (state->writeback_job && state->writeback_job->fb)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 4131e66..4aa82c91 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -676,6 +676,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
{
struct drm_device *dev = connector->dev;
struct drm_mode_config *config = &dev->mode_config;
+ bool replaced = false;
+ int ret;
if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, file_priv, val);
@@ -726,6 +728,14 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
*/
if (state->link_status != DRM_LINK_STATUS_GOOD)
state->link_status = val;
+ } else if (property == config->hdr_output_metadata_property) {
+ ret = drm_atomic_replace_property_blob_from_id(dev,
+ &state->hdr_output_metadata,
+ val,
+ -1, sizeof(struct hdr_output_metadata),
>>> Those function arguments are nonsesne for a blob that isn't an array.
>>>
+ &replaced);
+ state->hdr_metadata_changed |= replaced;
+ return ret;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
} else if (property == config->content_type_property) {
@@ -814,6 +824,9 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
*val = state->colorspace;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
+ } else if (property == config->hdr_output_metadata_property) {
+ *val = state->hdr_output_metadata ?
+ state->hdr_output_metadata->base.id : 0;
} else if (property == config->content_protection_property) {
*val = state->content_protection;
} else if (property == config->writeback_fb_id_property) {
diff --git a/drivers/gpu/drm/drm_connector.c
b/drivers/gpu/drm/drm_connector.c
index 11fcd25..c9ac8b9 100644
--- a/drivers/gpu/drm/drm_connector.c
Re: [Intel-gfx] [PATCH i-g-t 13/16] i915: Add gem_exec_balancer
Quoting Tvrtko Ursulin (2019-05-15 11:49:45)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > Exercise the in-kernel load balancer checking that we can distribute
> > batches across the set of ctx->engines to avoid load.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/Makefile.am |1 +
> > tests/Makefile.sources |1 +
> > tests/i915/gem_exec_balancer.c | 1050
> > tests/meson.build |7 +
> > 4 files changed, 1059 insertions(+)
> > create mode 100644 tests/i915/gem_exec_balancer.c
> >
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 5097debf6..c6af0aeaf 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -96,6 +96,7 @@ gem_close_race_LDADD = $(LDADD) -lpthread
> > gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> > gem_ctx_thrash_LDADD = $(LDADD) -lpthread
> > gem_ctx_sseu_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
> > +i915_gem_exec_balancer_LDADD = $(LDADD) $(top_builddir)/lib/libigt_perf.la
> > gem_exec_capture_LDADD = $(LDADD) -lz
> > gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> > gem_exec_parallel_LDADD = $(LDADD) -lpthread
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index e7ee27e81..323b625aa 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -24,6 +24,7 @@ TESTS_progs = \
> > i915/gem_ctx_clone \
> > i915/gem_ctx_engines \
> > i915/gem_ctx_shared \
> > + i915/gem_exec_balancer \
> > i915/gem_vm_create \
> > kms_3d \
> > kms_addfb_basic \
> > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > new file mode 100644
> > index 0..25195d478
> > --- /dev/null
> > +++ b/tests/i915/gem_exec_balancer.c
> > @@ -0,0 +1,1050 @@
> > +/*
> > + * Copyright © 2018 Intel Corporation
>
> 2019 I guess, even though work was started in 2018?
>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > next
> > + * paragraph) shall be included in all copies or substantial portions of
> > the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include
> > +
> > +#include "igt.h"
> > +#include "igt_perf.h"
> > +#include "i915/gem_ring.h"
> > +#include "sw_sync.h"
> > +
> > +IGT_TEST_DESCRIPTION("Exercise in-kernel load-balancing");
> > +
> > +#define INSTANCE_COUNT (1 << I915_PMU_SAMPLE_INSTANCE_BITS)
>
> Hmm.. this is a strange surrogate but I guess it works.
>
> > +
> > +static bool has_class_instance(int i915, uint16_t class, uint16_t instance)
> > +{
> > + int fd;
> > +
> > + fd = perf_i915_open(I915_PMU_ENGINE_BUSY(class, instance));
>
> More work for Andi to replace with real engine discovery. :)
>
> > + if (fd != -1) {
> > + close(fd);
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static struct i915_engine_class_instance *
> > +list_engines(int i915, uint32_t class_mask, unsigned int *out)
> > +{
> > + unsigned int count = 0, size = 64;
> > + struct i915_engine_class_instance *engines;
> > +
> > + engines = malloc(size * sizeof(*engines));
> > + if (!engines) {
> > + *out = 0;
> > + return NULL;
> > + }
> > +
> > + for (enum drm_i915_gem_engine_class class = I915_ENGINE_CLASS_RENDER;
> > + class_mask;
> > + class++, class_mask >>= 1) {
> > + if (!(class_mask & 1))
> > + continue;
> > +
> > + for (unsigned int instance = 0;
> > + instance < INSTANCE_COUNT;
> > + instance++) {
> > + if (!has_class_instance(i915, class, instance))
> > + continue;
> > +
> > + if (count == size) {
> > + struct i915_engine_class_instance *e;
> > +
> > + size *= 2;
> > +
Re: [Intel-gfx] [PATCH i-g-t 14/16] i915/gem_exec_balancer: Exercise bonded pairs
Quoting Tvrtko Ursulin (2019-05-15 11:58:20)
>
> On 08/05/2019 11:09, Chris Wilson wrote:
> > The submit-fence + load_balancing apis allow for us to execute a named
> > pair of engines in parallel; that this by submitting a request to one
> > engine, we can then use the generated submit-fence to submit a second
> > request to another engine and have it execute at the same time.
> > Furthermore, by specifying bonded pairs, we can direct the virtual
> > engine to use a particular engine in parallel to the first request.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/i915/gem_exec_balancer.c | 234 +++--
> > 1 file changed, 224 insertions(+), 10 deletions(-)
> >
> > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > index 25195d478..20ad66727 100644
> > --- a/tests/i915/gem_exec_balancer.c
> > +++ b/tests/i915/gem_exec_balancer.c
> > @@ -98,9 +98,35 @@ list_engines(int i915, uint32_t class_mask, unsigned int
> > *out)
> > return engines;
> > }
> >
> > +static int __set_engines(int i915, uint32_t ctx,
> > + const struct i915_engine_class_instance *ci,
> > + unsigned int count)
> > +{
> > + I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, count);
> > + struct drm_i915_gem_context_param p = {
> > + .ctx_id = ctx,
> > + .param = I915_CONTEXT_PARAM_ENGINES,
> > + .size = sizeof(engines),
> > + .value = to_user_pointer(&engines)
> > + };
> > +
> > + engines.extensions = 0;
> > + memcpy(engines.engines, ci, sizeof(engines.engines));
> > +
> > + return __gem_context_set_param(i915, &p);
> > +}
> > +
> > +static void set_engines(int i915, uint32_t ctx,
> > + const struct i915_engine_class_instance *ci,
> > + unsigned int count)
> > +{
> > + igt_assert_eq(__set_engines(i915, ctx, ci, count), 0);
> > +}
> > +
> > static int __set_load_balancer(int i915, uint32_t ctx,
> > const struct i915_engine_class_instance *ci,
> > -unsigned int count)
> > +unsigned int count,
> > +void *ext)
> > {
> > I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(balancer, count);
> > I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1 + count);
> > @@ -113,6 +139,7 @@ static int __set_load_balancer(int i915, uint32_t ctx,
> >
> > memset(&balancer, 0, sizeof(balancer));
> > balancer.base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
> > + balancer.base.next_extension = to_user_pointer(ext);
> >
> > igt_assert(count);
> > balancer.num_siblings = count;
> > @@ -131,9 +158,10 @@ static int __set_load_balancer(int i915, uint32_t ctx,
> >
> > static void set_load_balancer(int i915, uint32_t ctx,
> > const struct i915_engine_class_instance *ci,
> > - unsigned int count)
> > + unsigned int count,
> > + void *ext)
> > {
> > - igt_assert_eq(__set_load_balancer(i915, ctx, ci, count), 0);
> > + igt_assert_eq(__set_load_balancer(i915, ctx, ci, count, ext), 0);
> > }
> >
> > static uint32_t load_balancer_create(int i915,
> > @@ -143,7 +171,7 @@ static uint32_t load_balancer_create(int i915,
> > uint32_t ctx;
> >
> > ctx = gem_context_create(i915);
> > - set_load_balancer(i915, ctx, ci, count);
> > + set_load_balancer(i915, ctx, ci, count, NULL);
> >
> > return ctx;
> > }
> > @@ -288,6 +316,74 @@ static void invalid_balancer(int i915)
> > }
> > }
> >
> > +static void invalid_bonds(int i915)
> > +{
> > + I915_DEFINE_CONTEXT_ENGINES_BOND(bonds[16], 1);
> > + I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1);
> > + struct drm_i915_gem_context_param p = {
> > + .ctx_id = gem_context_create(i915),
> > + .param = I915_CONTEXT_PARAM_ENGINES,
> > + .value = to_user_pointer(&engines),
> > + .size = sizeof(engines),
> > + };
> > + uint32_t handle;
> > + void *ptr;
> > +
> > + memset(&engines, 0, sizeof(engines));
> > + gem_context_set_param(i915, &p);
> > +
> > + memset(bonds, 0, sizeof(bonds));
> > + for (int n = 0; n < ARRAY_SIZE(bonds); n++) {
> > + bonds[n].base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> > + bonds[n].base.next_extension =
> > + n ? to_user_pointer(&bonds[n - 1]) : 0;
> > + bonds[n].num_bonds = 1;
> > + }
> > + engines.extensions = to_user_pointer(&bonds);
> > + gem_context_set_param(i915, &p);
> > +
> > + bonds[0].base.next_extension = -1ull;
> > + igt_assert_eq(__gem_context_set_param(i915, &p), -EFAULT);
> > +
> > + bonds[0].base.next_extension = to_user_pointer(&bonds[0]);
> > + igt_assert_eq(__gem_context_set_param(i915, &p), -E2BIG);
>
[Intel-gfx] [PULL] drm-misc-next-fixes
Hi Da.*, Some more fixes for -next. As predicted more panfrost cleanup. We also have 4 msm patches that Rob and I figured should just go through -misc instead of spinning up an msm-fixes branch. If we pooled together all msm fixes for 5.2, maybe the -fixes branch would have been warranted, but we've been boiling this frog for a while so why not continue. Anyways, hopefully this is it until we move back to -misc-fixes. drm-misc-next-fixes-2019-05-15: - A couple new panfrost fixes - Fix the low refresh rate register in adv7511 - A handful of msm fixes that fell out of 5.1 bringup on SDM845 - Fix spinlock initialization in pl111 Cheers, Sean The following changes since commit eb85d03e01c3e9f3b0ba7282b2e3515a635decb2: Merge tag 'drm-misc-next-fixes-2019-05-08' of git://anongit.freedesktop.org/drm/drm-misc into drm-next (2019-05-09 11:04:00 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-05-15 for you to fetch changes up to 2b117451277140b782f3e4e56a4296c0a165e7ae: drm/msm: Upgrade gxpd checks to IS_ERR_OR_NULL (2019-05-15 16:03:08 -0400) - A couple new panfrost fixes - Fix the low refresh rate register in adv7511 - A handful of msm fixes that fell out of 5.1 bringup on SDM845 - Fix spinlock initialization in pl111 Boris Brezillon (1): drm/panfrost: Add missing _fini() calls in panfrost_device_fini() Brian Masney (2): drm/msm: remove resv fields from msm_gem_object struct drm/msm: correct attempted NULL pointer dereference in debugfs Guenter Roeck (1): drm/pl111: Initialize clock spinlock early Matt Redfearn (1): drm/bridge: adv7511: Fix low refresh rate selection Sabyasachi Gupta (1): drm/msm/dpu: Remove duplicate header Sean Paul (1): drm/msm: Upgrade gxpd checks to IS_ERR_OR_NULL Tomeu Vizoso (1): drm/panfrost: Only put sync_out if non-NULL drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 +++--- drivers/gpu/drm/msm/adreno/a6xx_gmu.c| 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c| 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 4 +--- drivers/gpu/drm/msm/msm_atomic.c | 4 +--- drivers/gpu/drm/msm/msm_gem.c| 3 ++- drivers/gpu/drm/msm/msm_gem.h| 4 drivers/gpu/drm/panfrost/panfrost_device.c | 4 drivers/gpu/drm/panfrost/panfrost_drv.c | 3 ++- drivers/gpu/drm/pl111/pl111_display.c| 5 +++-- 10 files changed, 19 insertions(+), 21 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 14/16] i915/gem_exec_balancer: Exercise bonded pairs
Quoting Chris Wilson (2019-05-15 20:57:18) > Quoting Tvrtko Ursulin (2019-05-15 11:58:20) > > > > On 08/05/2019 11:09, Chris Wilson wrote: > > > + igt_assert_f(load > 0.90, > > > + "engine %d (class:instance %d:%d) was > > > found to be only %.1f%% busy\n", > > > + n, siblings[n].engine_class, > > > siblings[n].engine_instance, > > > + load*100); > > > > Master also needs to be checked I think. You have the infrastructure to > > open two pmus in the previous patch so should be easy. > > Haven't we checked precisely that in earlier tests? What would perhaps > be fairer here would be to verify the other engine was idle, otherwise > we could say we fluked it. Furthermore, we should repeat a few times > with say (0, 1), (0, 1), (1, 0), (1, 0) to further rule out flukes, and > then to finish with a random smoketest of some description. > > Perhaps even a test is closer to the typical workload would involve > semaphore communication across the bond. But I don't know a way in which > I can determine which engine I am on in order to record that from the > GPU itself. To remind myself, the importance here is on uABI stressing, it's is much easier to prove the relationship in the kernel and that is where we do. -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 series starting with [1/2] drm/i915: Hook into the reboot notifier to cancel outstanding GPU operations
== Series Details == Series: series starting with [1/2] drm/i915: Hook into the reboot notifier to cancel outstanding GPU operations URL : https://patchwork.freedesktop.org/series/60676/ State : success == Summary == CI Bug Log - changes from CI_DRM_6086_full -> Patchwork_13021_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_13021_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_tiled_swapping@non-threaded: - shard-hsw: [PASS][1] -> [FAIL][2] ([fdo#108686]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-hsw2/igt@gem_tiled_swapp...@non-threaded.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-hsw8/igt@gem_tiled_swapp...@non-threaded.html * igt@i915_pm_rpm@modeset-lpsp-stress: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([fdo#107807]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl1/igt@i915_pm_...@modeset-lpsp-stress.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-skl5/igt@i915_pm_...@modeset-lpsp-stress.html * igt@kms_fbcon_fbt@psr-suspend: - shard-skl: [PASS][5] -> [INCOMPLETE][6] ([fdo#104108] / [fdo#107773]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl6/igt@kms_fbcon_...@psr-suspend.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-skl5/igt@kms_fbcon_...@psr-suspend.html * igt@kms_flip@2x-plain-flip-ts-check: - shard-hsw: [PASS][7] -> [INCOMPLETE][8] ([fdo#103540]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-hsw2/igt@kms_f...@2x-plain-flip-ts-check.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-hsw8/igt@kms_f...@2x-plain-flip-ts-check.html * igt@kms_flip@plain-flip-ts-check-interruptible: - shard-skl: [PASS][9] -> [FAIL][10] ([fdo#100368]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl8/igt@kms_f...@plain-flip-ts-check-interruptible.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-skl9/igt@kms_f...@plain-flip-ts-check-interruptible.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render: - shard-iclb: [PASS][11] -> [FAIL][12] ([fdo#103167]) +4 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb1/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-cur-indfb-draw-render.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-iclb5/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-cur-indfb-draw-render.html * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes: - shard-snb: [PASS][13] -> [DMESG-WARN][14] ([fdo#102365]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-snb7/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-a-planes.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-snb2/igt@kms_pl...@plane-panning-bottom-right-suspend-pipe-a-planes.html * igt@kms_plane_lowres@pipe-a-tiling-x: - shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103166]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb3/igt@kms_plane_low...@pipe-a-tiling-x.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-iclb1/igt@kms_plane_low...@pipe-a-tiling-x.html * igt@kms_psr@psr2_cursor_mmap_cpu: - shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-iclb6/igt@kms_psr@psr2_cursor_mmap_cpu.html * igt@prime_busy@hang-vebox: - shard-apl: [PASS][19] -> [FAIL][20] ([fdo#108807]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-apl4/igt@prime_b...@hang-vebox.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-apl3/igt@prime_b...@hang-vebox.html Possible fixes * igt@i915_pm_rpm@pm-caching: - shard-skl: [INCOMPLETE][21] ([fdo#107807]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl7/igt@i915_pm_...@pm-caching.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-skl2/igt@i915_pm_...@pm-caching.html * igt@i915_pm_rpm@system-suspend: - shard-skl: [INCOMPLETE][23] ([fdo#104108] / [fdo#107773] / [fdo#107807]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl10/igt@i915_pm_...@system-suspend.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13021/shard-skl10/igt@i915_pm_...@system-suspend.html * igt@i915_suspend@debugfs-reader: - shard-skl: [INCOMPLETE][
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/perf: Refactor oa object to better manage resources (rev3)
== Series Details ==
Series: drm/i915/perf: Refactor oa object to better manage resources (rev3)
URL : https://patchwork.freedesktop.org/series/60176/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6086_full -> Patchwork_13022_full
Summary
---
**SUCCESS**
No regressions found.
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_13022_full:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@kms_cursor_crc@pipe-a-cursor-suspend}:
- shard-skl: [PASS][1] -> [INCOMPLETE][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl9/igt@kms_cursor_...@pipe-a-cursor-suspend.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-skl1/igt@kms_cursor_...@pipe-a-cursor-suspend.html
Known issues
Here are the changes found in Patchwork_13022_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@gem_eio@reset-stress:
- shard-snb: [PASS][3] -> [INCOMPLETE][4] ([fdo#105411])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-snb6/igt@gem_...@reset-stress.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-snb1/igt@gem_...@reset-stress.html
* igt@gem_eio@wait-10ms:
- shard-glk: [PASS][5] -> [FAIL][6] ([fdo#105957])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-glk8/igt@gem_...@wait-10ms.html
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-glk2/igt@gem_...@wait-10ms.html
* igt@gem_workarounds@suspend-resume:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#108566])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-apl7/igt@gem_workarou...@suspend-resume.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-apl5/igt@gem_workarou...@suspend-resume.html
* igt@i915_pm_rpm@i2c:
- shard-iclb: [PASS][9] -> [DMESG-WARN][10] ([fdo#109982])
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb1/igt@i915_pm_...@i2c.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-iclb2/igt@i915_pm_...@i2c.html
* igt@kms_flip@flip-vs-suspend-interruptible:
- shard-hsw: [PASS][11] -> [INCOMPLETE][12] ([fdo#103540])
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-hsw2/igt@kms_f...@flip-vs-suspend-interruptible.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-hsw6/igt@kms_f...@flip-vs-suspend-interruptible.html
* igt@kms_flip_tiling@flip-y-tiled:
- shard-skl: [PASS][13] -> [FAIL][14] ([fdo#108303])
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl5/igt@kms_flip_til...@flip-y-tiled.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-skl9/igt@kms_flip_til...@flip-y-tiled.html
* igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-cpu:
- shard-snb: [PASS][15] -> [SKIP][16] ([fdo#109271]) +1 similar
issue
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-snb1/igt@kms_frontbuffer_track...@fbc-rgb565-draw-mmap-cpu.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-snb6/igt@kms_frontbuffer_track...@fbc-rgb565-draw-mmap-cpu.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103167]) +5 similar
issues
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb3/igt@kms_frontbuffer_track...@fbcpsr-1p-pri-indfb-multidraw.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-iclb2/igt@kms_frontbuffer_track...@fbcpsr-1p-pri-indfb-multidraw.html
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403])
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-skl6/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html
[20]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-skl3/igt@kms_plane_alpha_bl...@pipe-c-coverage-7efc.html
* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar
issues
[21]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
[22]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13022/shard-iclb6/igt@kms_psr@psr2_cursor_mmap_cpu.html
* igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
- shard-iclb: [PASS][23] -> [INCOMPLETE][24] ([fdo#107713] /
[fdo#110026])
[23]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6086/shard-iclb6/igt@kms_rotation_...@multiplane-rotation-cropping-bottom.html
[24]:
htt
Re: [Intel-gfx] IOMMU RMRR for Intel graphic device
Hi Daniel, On 5/10/19 4:34 PM, Daniel Vetter wrote: On Fri, May 10, 2019 at 01:21:59PM +0800, Lu Baolu wrote: Forward to i915 mailing list and post the question again so people knows what are the key concerns. The background is that Linux community is going to collect and report the reserved memory ranges of an assigned device to VFIO driver, and any mapped address will be checked against the reserved ranges. If there's any conflict, vifo will refuse the map request. Unfortunately, when it comes Intel graphic device, the conflict happens. That means address range mapped through vfio conflicts with the rmrr for graphic device. https://lkml.org/lkml/2018/6/5/760 The questions are 1) will the rmrr for graphic device still needs to be reserved for BIOS or firmware use, when a device is going to be assigned to user level? 2) if no, what's the check point, after which the rmrr is unnecessary anymore? The gfx RMRR isn't for the bios, it's for the driver. It covers stolen memory, and we need it. There's various piles of hacks to disable use of stolen, but they're all somewhat fragile, and afaiui for huc/guc we need it, and huc is part of the uapi exposed by the driver/device combo on gen9+. Until our hw folks come up with a better design I think we're just stuck on this, iow you can't pass-thru modern intel igfx because of this. -Daniel Thanks for the explanation. It's clear to me now. Best regards, Lu Baolu Best regards, Lu Baolu On 5/9/19 5:14 PM, Zhang, Tina wrote: Hi Baolu, +Xiong I think the root cause is that guest i915 needs to access RMRR. Xiong cooked a patch to disable the RMRR use in i915 guest, however that patch didn't get landed into the i915 upstream repo due to some concerns from i915 maintainers. Xiong can give us more backgrounds. So agreed, need to ask the i915 folk for this. BR, Tina -Original Message- From: Yuan, Hang Sent: Thursday, May 9, 2019 4:07 PM To: Lu Baolu ; Tian, Kevin ; Zhenyu Wang ; Zhang, Tina ; Lu, Baolu ; Liu, Yi L Subject: RE: IOMMU RMRR for Intel graphic device Hi Baolu, as Kevin suggested, would you like to ask i915 people in their mailing list intel-gfx@lists.freedesktop.org? Regards, Henry -Original Message- From: Lu Baolu [mailto:baolu...@linux.intel.com] Sent: Thursday, May 9, 2019 2:42 PM To: Tian, Kevin ; Zhenyu Wang ; Zhang, Tina ; Yuan, Hang ; Lu, Baolu ; Liu, Yi L Cc: baolu...@linux.intel.com Subject: Re: IOMMU RMRR for Intel graphic device Hi, +Tina and Henry and cc more people The background is that Linux community is going to collect and report the reserved memory ranges of an assigned device to VFIO driver, and any mapped address will be checked against the reserved ranges. If there's any conflict, vifo will refuse the map request. Unfortunately, when it comes Intel graphic device, the conflict happens. That means address range mapped through vfio conflicts with the rmrr for graphic device. https://lkml.org/lkml/2018/6/5/760 The questions are 1) will the rmrr for graphic device still needs to be reserved for BIOS or firmware use, when a device is going to be assigned to user level? 2) if no, what's the check point, after which the rmrr is unnecessary anymore? Best regards, Lu Baolu On 5/6/19 2:16 PM, Tian, Kevin wrote: this should better be asked to i915 guys, since it's not virtualization related. :-) One caveat, iirc, i915 driver tries to reuse stolen memory (covered by RMRR) even after boot time. take it as if another type of memory resource. If true I'm afraid this might be a gap to your proposal. Since nothing confidential, possibly you can directly discuss in community. From: Lu Baolu [mailto:baolu...@linux.intel.com] Sent: Thursday, May 2, 2019 2:45 PM Ping... Any comments? This has been postponed in the community for a long time. We need to response this as soon as possible. Best regards, Lu Baolu On 4/29/19 1:19 PM, Lu Baolu wrote: Hi Zhenyu, As we discussed, BIOS always exports IOMMU reserved memory regions for (a.k.a. RMRR in vt-d spec) Intel integrated graphic device. This caused some problems when we pass-through such graphic devices to user level. I am about to propose something to the community so that a RMRR for graphic devices could be explicitly canceled as long as the driver (i915 or vfio) knows that the RMRR will never be used by BIOS anymore. The same story happens for USB host controller devices. And since we know that BIOS will stop using that memory region as soon as the driver clears the SMI bits. So the question is, can graphic driver know when the RMRR for graphic could be canceled? Best regards, Lu Baolu ___ 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
[Intel-gfx] [PATCH 0/1] Reg: igt@kms_pipe_crc_basic@* CRC mismatch test failures
This patch fixes the igt@kms_pipe_crc_basic@* test failures due to CRC mismatch on CI BYT squawks chromebook as reported in: https://bugs.freedesktop.org/show_bug.cgi?id=103191 These CRC mismatch test failures happen consistently on BYT squawks chromebook These test failures happen very intermittently on shard-SKL and CML-U systems. This patch fixes these test failures on the BYT platform. The reason for these CRC test failures happening on SKL and CML-U platforms is still unknown and is being investigated. Thanks Harish. Harish Chegondi (1): drm/i915: skip the second CRC even for GEN 7 GPUs drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/1] drm/i915: skip the second CRC even for GEN 7 GPUs
display_pipe_crc_irq_handler() skips the first CRC for all GPUs and the
second CRC for GEN8+ GPUs. The second CRC is invalid even for BYT which
is a GEN7 GPU. So, skip the second CRC even for GEN7 GPUs.
Cc: Jani Nikula
Cc: Tomi Sarvela
Cc: Petri Latvala
Cc: Ville Syrjälä
Cc: Maarten Lankhorst
Signed-off-by: Harish Chegondi
References: https://bugs.freedesktop.org/show_bug.cgi?id=103191
---
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 233211fde0ea..3809e9f7fae2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1775,11 +1775,11 @@ static void display_pipe_crc_irq_handler(struct
drm_i915_private *dev_priv,
* bonkers. So let's just wait for the next vblank and read
* out the buggy result.
*
-* On GEN8+ sometimes the second CRC is bonkers as well, so
+* On GEN7+ sometimes the second CRC is bonkers as well, so
* don't trust that one either.
*/
if (pipe_crc->skipped <= 0 ||
- (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
+ (INTEL_GEN(dev_priv) >= 7 && pipe_crc->skipped == 1)) {
pipe_crc->skipped++;
spin_unlock(&pipe_crc->lock);
return;
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Shutdown hooks
On Wednesday, May 15, 2019 5:00:40 PM CEST Chris Wilson wrote: > Janus, some old patches that may be of use for shutdown prior to kexec. > -Chris Hi Chris, Thanks for sharing. I'm only not sure why you mentioned kexec. I have an impression someone else was talking about kexec recently so maybe I was not the intended recipient. But anyway, those patches look to me like they may be helpful by hotunplug so I'm going to give them a try with the hotunplug test. Thanks, Janusz Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Reg: igt@kms_pipe_crc_basic@* CRC mismatch test failures
== Series Details == Series: Reg: igt@kms_pipe_crc_basic@* CRC mismatch test failures URL : https://patchwork.freedesktop.org/series/60697/ State : success == Summary == CI Bug Log - changes from CI_DRM_6089 -> Patchwork_13023 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/ Known issues Here are the changes found in Patchwork_13023 that come from known issues: ### IGT changes ### Issues hit * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-blb-e6850: [PASS][1] -> [INCOMPLETE][2] ([fdo#107718]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6089/fi-blb-e6850/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/fi-blb-e6850/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html Possible fixes * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-7500u: [FAIL][3] ([fdo#109485]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6089/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/fi-kbl-7500u/igt@kms_chamel...@hdmi-hpd-fast.html Warnings * igt@i915_selftest@live_hangcheck: - fi-apl-guc: [DMESG-FAIL][5] ([fdo#110620]) -> [INCOMPLETE][6] ([fdo#103927] / [fdo#110624]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6089/fi-apl-guc/igt@i915_selftest@live_hangcheck.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/fi-apl-guc/igt@i915_selftest@live_hangcheck.html * igt@runner@aborted: - fi-apl-guc: [FAIL][7] ([fdo#110622]) -> [FAIL][8] ([fdo#110624]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6089/fi-apl-guc/igt@run...@aborted.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/fi-apl-guc/igt@run...@aborted.html [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485 [fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620 [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622 [fdo#110624]: https://bugs.freedesktop.org/show_bug.cgi?id=110624 Participating hosts (52 -> 45) -- Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * Linux: CI_DRM_6089 -> Patchwork_13023 CI_DRM_6089: c7a46913f77868d15ade90846526d7e4cb910637 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4991: 61dd8eec659062eac67823ccd9f2189c3aa36201 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_13023: ea7bd1c53ffed5ca2cf77f2a236ff031fcd71ba7 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ea7bd1c53ffe drm/i915: skip the second CRC even for GEN 7 GPUs == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13023/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] CI: Revert "net/sch_generic: Shut up noise"
This reverts commit a9f840bdd2fd4cb07a669f1c3112b804218b4aba.
But instead tune it down to an error, hopefully that way we can
perhaps capture something ...
Cc: Martin Peres
---
net/sched/sch_generic.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index cf96149d7b5e..5b42dad94665 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -456,13 +456,8 @@ static void dev_watchdog(struct timer_list *t)
}
}
- /* The noise is pissing off our CI and upstream doesn't
-* move on the bug report:
-*
-* https://bugzilla.kernel.org/show_bug.cgi?id=196399
-*/
- if (some_queue_timedout && 0) {
- WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s
(%s): transmit queue %u timed out\n",
+ if (some_queue_timedout) {
+ pr_err("NETDEV WATCHDOG: %s (%s): transmit
queue %u timed out\n",
dev->name, netdev_drivername(dev), i);
dev->netdev_ops->ndo_tx_timeout(dev);
}
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for CI: Revert "net/sch_generic: Shut up noise"
== Series Details == Series: CI: Revert "net/sch_generic: Shut up noise" URL : https://patchwork.freedesktop.org/series/60699/ State : warning == Summary == $ dim checkpatch origin/drm-tip 8fbf6ed5c78a CI: Revert "net/sch_generic: Shut up noise" -:32: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 0 checks, 15 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
