Re: [Intel-gfx] [PATCH 2/8] drm/i915/icl: implement icl_digital_port_connected()
On Wed, Jul 11, 2018 at 02:59:03PM -0700, Paulo Zanoni wrote:
> Do like the other functions and check for the ISR bits. We have plans
> to add a few more checks in this code in the next patches, that's why
> it's a little more verbose than it could be.
>
> v2: Rebase.
>
> Cc: Animesh Manna
> Reviewed-by: Lucas De Marchi (v1)
> Signed-off-by: Paulo Zanoni
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4 +++
> drivers/gpu/drm/i915/intel_dp.c | 57
> +
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b95bab7a3d24..c1f350469ff6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7198,6 +7198,7 @@ enum {
> #define GEN11_TC3_HOTPLUG (1 << 18)
> #define GEN11_TC2_HOTPLUG (1 << 17)
> #define GEN11_TC1_HOTPLUG (1 << 16)
> +#define GEN11_TC_HOTPLUG(tc_port) (1 << ((tc_port) + 16))
> #define GEN11_DE_TC_HOTPLUG_MASK(GEN11_TC4_HOTPLUG | \
>GEN11_TC3_HOTPLUG | \
>GEN11_TC2_HOTPLUG | \
> @@ -7206,6 +7207,7 @@ enum {
> #define GEN11_TBT3_HOTPLUG (1 << 2)
> #define GEN11_TBT2_HOTPLUG (1 << 1)
> #define GEN11_TBT1_HOTPLUG (1 << 0)
> +#define GEN11_TBT_HOTPLUG(tc_port) (1 << (tc_port))
> #define GEN11_DE_TBT_HOTPLUG_MASK (GEN11_TBT4_HOTPLUG | \
>GEN11_TBT3_HOTPLUG | \
>GEN11_TBT2_HOTPLUG | \
> @@ -7578,6 +7580,8 @@ enum {
> #define SDE_GMBUS_ICP(1 << 23)
> #define SDE_DDIB_HOTPLUG_ICP (1 << 17)
> #define SDE_DDIA_HOTPLUG_ICP (1 << 16)
> +#define SDE_TC_HOTPLUG_ICP(tc_port) (1 << ((tc_port) + 24))
> +#define SDE_DDI_HOTPLUG_ICP(port)(1 << ((port) + 16))
> #define SDE_DDI_MASK_ICP (SDE_DDIB_HOTPLUG_ICP | \
>SDE_DDIA_HOTPLUG_ICP)
> #define SDE_TC_MASK_ICP (SDE_TC4_HOTPLUG_ICP | \
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5be07e1d816d..f2197dea02d0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4744,6 +4744,61 @@ static bool bxt_digital_port_connected(struct
> intel_encoder *encoder)
> return I915_READ(GEN8_DE_PORT_ISR) & bit;
> }
>
> +static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
> + struct intel_digital_port *intel_dig_port)
> +{
> + enum port port = intel_dig_port->base.port;
> +
> + return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> +}
> +
> +static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
> + struct intel_digital_port *intel_dig_port)
> +{
> + enum port port = intel_dig_port->base.port;
> + enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> + u32 legacy_bit = SDE_TC_HOTPLUG_ICP(tc_port);
> + u32 typec_bit = GEN11_TC_HOTPLUG(tc_port);
> + u32 tbt_bit = GEN11_TBT_HOTPLUG(tc_port);
> + bool is_legacy = false, is_typec = false, is_tbt = false;
> + u32 cpu_isr;
> +
> + if (I915_READ(SDEISR) & legacy_bit)
> + is_legacy = true;
> +
> + cpu_isr = I915_READ(GEN11_DE_HPD_ISR);
> + if (cpu_isr & typec_bit)
> + is_typec = true;
> + if (cpu_isr & tbt_bit)
> + is_tbt = true;
> +
> + WARN_ON(is_legacy + is_typec + is_tbt > 1);
> + if (!is_legacy && !is_typec && !is_tbt)
> + return false;
what about something like below?
+{
+ enum port port = intel_dig_port->base.port;
+ enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
+ bool is_legacy = false, is_typec = false, is_tbt = false;
+ u32 cpu_isr;
+
+ is_legacy = I915_READ(SDEISR) & legacy_bit;
+
+ cpu_isr = I915_READ(GEN11_DE_HPD_ISR);
+ is_typec = cpu_isr & typec_bit;
+ is_tbt = cpu_isr & tbt_bit;
+
+ WARN_ON(is_legacy && (is_typec || is_tbt));
+ return (is_legacy || is_typec || is_tbt);
+}
> +
> +static bool icl_digital_port_connected(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(>base);
> +
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_A:
> + case HPD_PORT_B:
> + return icl_combo_port_connected(dev_priv, dig_port);
> + case HPD_PORT_C:
> + case HPD_PORT_D:
> + case HPD_PORT_E:
> + case HPD_PORT_F:
> + return icl_tc_port_connected(dev_priv, dig_port);
> + default:
> + MISSING_CASE(encoder->hpd_pin);
> + return false;
> + }
Re: [Intel-gfx] [PATCH v5] drm/i915: Use crtc_state->has_psr instead of CAN_PSR for pipe update
On Wed, 2018-07-11 at 22:33 -0700, Tarun Vyas wrote:
> In commit "drm/i915: Wait for PSR exit before checking for vblank
> evasion", the idea was to limit the PSR IDLE checks when PSR is
> actually supported. While CAN_PSR does do that check, it doesn't
> applies on a per-crtc basis. crtc_state->has_psr is a more granular
> check that only applies to pipe(s) that have PSR enabled.
>
> Without the has_psr check, we end up waiting on the eDP transcoder's
> PSR_STATUS register irrespective of whether the pipe being updated is
> driving it or not.
>
> Fixes: a608987970b9 ("drm/i915: Wait for PSR exit before checking for
> vblank evasion")
>
Pushed this to -queued after checking v4's shards results (only commit
message changed between v5 and v4)
-DK
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
On Thu, 2018-07-12 at 16:26 -0700, Rodrigo Vivi wrote:
> On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> >
> > This allows to read i915_edp_psr_status from tests without
> > triggering
> > any AUX communication. Take this opportunity to move this under the
> > eDP-1 connector directory as the status we print is of the sink.
> >
> > Cc: Rodrigo Vivi
> > Cc: José Roberto de Souza
> > Suggested-by: Rodrigo Vivi
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 69 +--
> > --
> > 1 file changed, 39 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index f6142d78ede4..5069d5dedafe 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2592,6 +2592,41 @@ static const struct file_operations
> > i915_guc_log_relay_fops = {
> > .release = i915_guc_log_relay_release,
> > };
> >
> > +static int i915_psr_sink_status_show(struct seq_file *m, void
> > *data)
> > +{
> > + u8 val;
> > + static const char * const sink_status[] = {
> > + "inactive",
> > + "transition to active, capture and display",
> > + "active, display from RFB",
> > + "active, capture and display on sink device
> > timings",
> > + "transition to inactive, capture and display,
> > timing re-sync",
> > + "reserved",
> > + "reserved",
> > + "sink internal error",
> > + };
> > + struct drm_connector *connector = m->private;
> > + struct intel_dp *intel_dp =
> > + enc_to_intel_dp(_attached_encoder(connector)
> > ->base);
> > +
> > + if (connector->status != connector_status_connected)
> > + return -ENODEV;
> > +
> > + if (drm_dp_dpcd_readb(_dp->aux, DP_PSR_STATUS, )
> > == 1) {
> > + const char *str = "unknown";
> > +
> > + val &= DP_PSR_SINK_STATE_MASK;
> > + if (val < ARRAY_SIZE(sink_status))
> > + str = sink_status[val];
> > + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> > str);
> > + } else {
> > + DRM_ERROR("dpcd read (at %u) failed\n",
> > DP_PSR_STATUS);
> > + }
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > +
> > static void
> > psr_source_status(struct drm_i915_private *dev_priv, struct
> > seq_file *m)
> > {
> > @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private
> > *dev_priv, struct seq_file *m)
> > seq_printf(m, "Source PSR status: 0x%x [%s]\n",
> > psr_status, "unknown");
> > }
> >
> > -static const char *psr_sink_status(u8 val)
> > -{
> > - static const char * const sink_status[] = {
> > - "inactive",
> > - "transition to active, capture and display",
> > - "active, display from RFB",
> > - "active, capture and display on sink device
> > timings",
> > - "transition to inactive, capture and display,
> > timing re-sync",
> > - "reserved",
> > - "reserved",
> > - "sink internal error"
> > - };
> > -
> > - val &= DP_PSR_SINK_STATE_MASK;
> > - if (val < ARRAY_SIZE(sink_status))
> > - return sink_status[val];
> > -
> > - return "unknown";
> > -}
> > -
> > static int i915_edp_psr_status(struct seq_file *m, void *data)
> > {
> > struct drm_i915_private *dev_priv = node_to_i915(m-
> > >private);
> > @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> > }
> >
> > psr_source_status(dev_priv, m);
> > -
> > - if (dev_priv->psr.enabled) {
> > - struct drm_dp_aux *aux = _priv->psr.enabled-
> > >aux;
> > - u8 val;
> > -
> > - if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, ) ==
> > 1)
> > - seq_printf(m, "Sink PSR status: 0x%x
> > [%s]\n", val,
> > - psr_sink_status(val));
> > - }
> > mutex_unlock(_priv->psr.lock);
> >
> > if (READ_ONCE(dev_priv->psr.debug)) {
> > @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct
> > drm_connector *connector)
> > debugfs_create_file("i915_dpcd", S_IRUGO, root,
> > connector, _dpcd_fops);
> >
> > - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> > + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > debugfs_create_file("i915_panel_timings", S_IRUGO,
> > root,
> > connector, _panel_fops);
> > + debugfs_create_file("i915_psr_sink_status",
> > S_IRUGO, root,
> > + connector,
> > _psr_sink_status_fops);
> my OCD on names here about i915_psr vs i915_edp_psr is still a real
> thing although
> they are on different places ;)
>
> We could probably remove "_edp" from the others, because psr is an
> edp only after all
>
> Anyways let's
Re: [Intel-gfx] [PATCH 3/6] drm/i915/gtt: Disable read-only support under GVT
On 2018.07.12 20:36:03 +, Bloomfield, Jon wrote: > > -Original Message- > > From: Chris Wilson > > Sent: Thursday, July 12, 2018 11:53 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Chris Wilson ; Zhenyu Wang > > ; Bloomfield, Jon ; > > Joonas Lahtinen ; Matthew Auld > > > > Subject: [PATCH 3/6] drm/i915/gtt: Disable read-only support under GVT > > > > GVT is not propagating the PTE bits, and is always setting the > > read-write bit, thus breaking read-only support. > > > > Signed-off-by: Chris Wilson > > Cc: Zhenyu Wang > > Cc: Jon Bloomfield > > Cc: Joonas Lahtinen > > Cc: Matthew Auld > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 6c0b438afe46..8e70a45b8a90 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -1662,8 +1662,12 @@ static struct i915_hw_ppgtt > > *gen8_ppgtt_create(struct drm_i915_private *i915) > > 1ULL << 48 : > > 1ULL << 32; > > > > - /* From bdw, there is support for read-only pages in the PPGTT */ > > - ppgtt->vm.has_read_only = true; > > + /* > > +* From bdw, there is support for read-only pages in the PPGTT. > > +* > > +* XXX GVT is not setting honouring the PTE bits. > > +*/ > > + ppgtt->vm.has_read_only = !intel_vgpu_active(i915); > > > > i915_address_space_init(>vm, i915); > > > > -- > > 2.18.0 > > Is there a blocker that prevents gvt respecting this bit? I can't think of an > obvious > reason why it would be a bad thing to support. I don't think any blocker for gvt support, we can respect that bit when shadowing. But we need capability check on host gvt when that support is ready. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for linux-next: build failure after merge of the drm-intel tree (rev2)
== Series Details == Series: linux-next: build failure after merge of the drm-intel tree (rev2) URL : https://patchwork.freedesktop.org/series/42839/ State : failure == Summary == Applying: linux-next: build failure after merge of the drm-intel tree Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/gvt/kvmgt.c Falling back to patching base and 3-way merge... No changes -- Patch already applied. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: build failure after merge of the drm-intel tree
Hi all,
[Dave cc'd because this will probably turn up in the drm tree soon.]
After merging the drm-intel tree, today's linux-next build (x86_64
allmodconfig) failed like this:
drivers/gpu/drm/i915/gvt/kvmgt.c: In function 'gvt_dma_map_page':
drivers/gpu/drm/i915/gvt/kvmgt.c:188:17: error: 'pfn' undeclared (first use in
this function); did you mean 'gfn'?
if (!pfn_valid(pfn)) {
^~~
Caused by commit
79e542f5af79 ("drm/i915/kvmgt: Support setting dma map for huge pages")
interacting with commit
39b4cbadb9a9 ("drm/i915/kvmgt: Check the pfn got from vfio_pin_pages")
from Linus' tree (v4.18-rc1).
I added the following merge fix patch:
From: Stephen Rothwell
Date: Fri, 13 Jul 2018 11:48:41 +1000
Subject: [PATCH] drm/i915/kvmgt: merge fixup for "Check the pfn got from
vfio_pin_pages"
Signed-off-by: Stephen Rothwell
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 718ab307a500..4d2f53ae9f0f 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -185,12 +185,6 @@ static int gvt_dma_map_page(struct intel_vgpu *vgpu,
unsigned long gfn,
if (ret)
return ret;
- if (!pfn_valid(pfn)) {
- gvt_vgpu_err("pfn 0x%lx is not mem backed\n", pfn);
- vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), , 1);
- return -EINVAL;
- }
-
/* Setup DMA mapping. */
*dma_addr = dma_map_page(dev, page, 0, size, PCI_DMA_BIDIRECTIONAL);
ret = dma_mapping_error(dev, *dma_addr);
--
2.18.0
--
Cheers,
Stephen Rothwell
pgpTpnUiuSRT4.pgp
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/kvmgt: Fix compilation error
On 2018.07.12 17:53:30 +0200, Micha?? Winiarski wrote:
> gvt_pin_guest_page extracted some of the gvt_dma_map_page functionality:
> commit 79e542f5af79 ("drm/i915/kvmgt: Support setting dma map for huge pages")
>
> And yet, part of it was reintroduced in:
> commit 39b4cbadb9a9 ("drm/i915/kvmgt: Check the pfn got from vfio_pin_pages")
>
> Causing kvmgt part to no longer build. Let's remove it.
>
yeah, this change is correct. I found it too when merging against
upstreams for gvt staging tree, not caught by conflict though. Once
we backmerge upstream gvt requires this fix.
Acked-by: Zhenyu Wang
Thanks!
> Reported-by: Tomasz Lis
> Signed-off-by: Micha?? Winiarski
> Cc: Changbin Du
> Cc: Zhenyu Wang
> ---
> drivers/gpu/drm/i915/gvt/kvmgt.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 718ab307a500..4d2f53ae9f0f 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -185,12 +185,6 @@ static int gvt_dma_map_page(struct intel_vgpu *vgpu,
> unsigned long gfn,
> if (ret)
> return ret;
>
> - if (!pfn_valid(pfn)) {
> - gvt_vgpu_err("pfn 0x%lx is not mem backed\n", pfn);
> - vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), , 1);
> - return -EINVAL;
> - }
> -
> /* Setup DMA mapping. */
> *dma_addr = dma_map_page(dev, page, 0, size, PCI_DMA_BIDIRECTIONAL);
> ret = dma_mapping_error(dev, *dma_addr);
> --
> 2.17.1
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 5/9] trace.pl: Improved key/colours
On 7/12/2018 3:59 AM, Tvrtko Ursulin wrote:
From: John Harrison
Improve the timeline legend to show actual context colours.
v2: (Tvrtko Ursulin)
* Commit msg.
* Tweak layout for more compactness and more readability.
Signed-off-by: Tvrtko Ursulin
Cc: John Harrison
Cc: Tvrtko Ursulin
---
scripts/trace.pl | 69 ++--
1 file changed, 43 insertions(+), 26 deletions(-)
diff --git a/scripts/trace.pl b/scripts/trace.pl
index 7cafb3f52ba4..bd3039511f5d 100755
--- a/scripts/trace.pl
+++ b/scripts/trace.pl
@@ -738,9 +738,35 @@ say sprintf('GPU: %.2f%% idle, %.2f%% busy',
$flat_busy{'gpu-idle'}, $flat_busy{'gpu-busy'}) unless $html;
my $timeline_text = $colour_contexts ?
- 'Per context coloured shading like:' : 'Box shading like:';
+ 'per context coloured shading like' : 'box shading like';
my %ctx_colours;
+my $ctx_table;
+
+sub generate_ctx_table
+{
+ my @states = ('queue', 'ready', 'execute', 'ctxsave', 'incomplete');
+ my @ctxts;
+
+ if( $colour_contexts ) {
+ @ctxts = sort keys %ctxdb;
+ } else {
+ @ctxts = ($min_ctx);
+ }
+
+ $ctx_table .= '';
+
+ foreach my $ctx (@ctxts) {
+ $ctx_table .= "\n";
+ $ctx_table .= " Context $ctx\n" if $colour_contexts;
+ foreach my $state (@states) {
+ $ctx_table .= " " . uc($state) .
"\n";
+ }
+ $ctx_table .= "\n";
As mentioned in the email, I would definitely add something like:
my $i = 0;
...
if( $i++ > 5) {
$ctx_table .= "etc...";
last;
}
I've had to analyse traces with hundreds of contexts in them before now.
Which would produce an HTML file swamped by a huge legend (albeit with a
beautifully smoothly shaded rainbow!).
With that...
Signed-off-by: John Harrison
Reviewed-by: John Harrison
+ }
+
+ $ctx_table .= '';
+}
sub generate_ctx_colours
{
@@ -754,12 +780,7 @@ sub generate_ctx_colours
generate_ctx_colours() if $html and $colour_contexts;
-
-my $queued_style = box_style($min_ctx, 'queue');
-my $ready_style = box_style($min_ctx, 'ready');
-my $execute_style = box_style($min_ctx, 'execute');
-my $ctxsave_style = box_style($min_ctx, 'ctxsave');
-my $incomplete_style = box_style($min_ctx, 'incomplete');
+generate_ctx_table() if $html;
print <
@@ -778,35 +799,27 @@ print <
-Timeline request view:
+Timeline request view is $timeline_text:
-$timeline_text
-QUEUED
-READY
-EXECUTE
-CTXSAVE
-
-
-
-(INCOMPLETE)
-
-
-
-
-
-QUEUED = requests executing on the GPU
+
+$ctx_table
+
+
+QUEUE = requests executing on the GPU
READY = runnable requests waiting for a slot on GPU
EXECUTE = requests waiting on fences and dependencies before they are
runnable
CTXSAVE = GPU saving the context image
-
-
+INCOMPLETE = request of unknown completion time
Boxes are in format 'ctx-id/seqno'.
Use Ctrl+scroll-action to zoom-in/out and scroll-action or dragging to move
around the timeline.
+
+
+
GPU idle: $flat_busy{'gpu-idle'}%
@@ -975,20 +988,24 @@ sub box_style
{
my ($ctx, $stage) = @_;
my $deg;
+ my $text_col = 'white';
if ($stage eq 'queue') {
$deg = 90;
+ $text_col = 'black' if $colour_contexts;
} elsif ($stage eq 'ready') {
$deg = 45;
} elsif ($stage eq 'execute') {
$deg = 0;
+ $text_col = 'black' if $colour_contexts;
} elsif ($stage eq 'ctxsave') {
$deg = 105;
+ $text_col = 'black' if $colour_contexts;
} elsif ($stage eq 'incomplete') {
$deg = 0;
}
- return 'color: black; background: repeating-linear-gradient(' .
+ return "color: $text_col; background: repeating-linear-gradient(" .
$deg . 'deg, ' .
ctx_colour($ctx, $stage, 1.0) . ', ' .
ctx_colour($ctx, $stage, 1.0) . ' 10px, ' .
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 3/9] trace.pl: Improve readability of graphical timeline representation
On 7/12/2018 3:59 AM, Tvrtko Ursulin wrote: From: Tvrtko Ursulin We add stripes for different stages of request execution so it is easier to follow one context in the multi-colour mode. Vertical stripe pattern indicates pipeline "blockages" - requests waiting for dependencies before they are runnable. Diagonal stripes indicate runnable requests waiting for GPU time. Horizontal strips are requests executing on the GPU. Also use this new multi-coloured mode from media-bench.pl. v2: John Harrison: * Mention media-bench.pl in the commit. * Fix HTML for single colour mode. v3: * Rebase. * Apply stripes to legacy colouring as well. v4: John Harrison: * Use per context colours for ctxsave and incomplete boxes. * Clearer timeline legend. Signed-off-by: Tvrtko Ursulin Cc: John Harrison Reviewed-by: John Harrison ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 4/9] trace.pl: Improve context colouring for large context id's
On 7/12/2018 3:59 AM, Tvrtko Ursulin wrote: From: Tvrtko Ursulin John reports that on a long runnning systems the huge disparity between kernel context and user context id's causes all interesting colours to be clustered too close together. Fix this by assigning colours to seen contexts instead of basing purely on context id's. Signed-off-by: Tvrtko Ursulin Suggested-by: John Harrison Cc: John Harrison --- Reviewed-by: John Harrison ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/9] trace.pl: Scale timeline for better precision
On 7/12/2018 3:59 AM, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
vis library has a limited precision compared to our trace data which
prevents zooming into the timeline and seeing the fine detail.
Scale the HTML view by a thousand to work around it.
v2: Rebase for time axis changes.
Signed-off-by: Tvrtko Ursulin
Suggested-by: John Harrison
Cc: John Harrison
Reviewed-by: John Harrison # v1
---
scripts/trace.pl | 46 +++---
1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/scripts/trace.pl b/scripts/trace.pl
index 89491125490d..726c90d44547 100755
--- a/scripts/trace.pl
+++ b/scripts/trace.pl
@@ -40,6 +40,7 @@ my $trace = 0;
my $avg_delay_stats = 0;
my $gpu_timeline = 0;
my $colour_contexts = 0;
+my $no_timeline_scaling = 0;
my @args;
@@ -281,6 +282,18 @@ sub arg_colour_contexts
return @_;
}
+sub arg_no_timeline_scaling
+{
+ return unless scalar(@_);
+
+ if ($_[0] eq '--no-timeline-scaling') {
+ shift @_;
+ $no_timeline_scaling = 1;
+ }
+
+ return @_;
+}
+
@args = @ARGV;
while (@args) {
my $left = scalar(@args);
@@ -296,6 +309,7 @@ while (@args) {
@args = arg_ignore_ring(@args);
@args = arg_skip_box(@args);
@args = arg_colour_contexts(@args);
+ @args = arg_no_timeline_scaling(@args);
last if $left == scalar(@args);
}
@@ -334,6 +348,8 @@ sub ts
my ($us) = @_;
my ($y, $mo, $d, $h, $m, $s);
+ $us *= 1000 unless $no_timeline_scaling;
+
$s = int($us / 100);
$us = $us % 100;
@@ -1001,11 +1017,28 @@ print <
]);
var last_major_ms;
+ENDHTML
+
+if ($html) {
+ if ($no_timeline_scaling) {
+ say " timeDiv = 1;";
+ } else {
+ say " timeDiv = 1000;";
+ }
+}
+print <
function majorAxis(date, scale, step) {
- var s = date / 1000;
+ var s = date / 1000 / timeDiv;
+
+ last_major_ms = date / timeDiv;
- last_major_ms = date;
+ if (timeDiv == 1000) {
+ if (scale == 'second')
+ scale = 'millisecond';
+ else if (scale == 'minute')
+ scale = 'second'
+ }
if (scale == 'millisecond')
return s.toFixed(6) + "s";
@@ -1016,13 +1049,20 @@ print <
function minorAxis(date, scale, step) {
- var ms = date;
+ var ms = date / timeDiv;
ms -= last_major_ms;
if (ms < 0)
return '';
+ if (timeDiv > 1) {
+ if (scale == 'second')
+ scale = 'millisecond';
+ else if (scale == 'minute')
+ scale = 'second';
+ }
+
if (scale == 'millisecond')
return "+" + ms.toFixed(3) + "ms";
else if (scale == 'second')
Reviewed-by: John Harrison
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/i915/icl: compute the TBT PLL registers
On Wed, Jul 11, 2018 at 02:59:02PM -0700, Paulo Zanoni wrote:
> Use the hardcoded tables provided by our spec.
>
> v2:
> - SSC stays disabled.
> - Use intel_port_is_tc().
>
> Cc: Anusha Srivatsa
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 22 +-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index b51ad2917dbe..7e5e6eb5dfe2 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2452,6 +2452,16 @@ static const struct skl_wrpll_params
> icl_dp_combo_pll_19_2MHz_values[] = {
> .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> };
>
> +static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> + .dco_integer = 0x151, .dco_fraction = 0x4000,
I see 0x151 is the most common for 24MHz, but not the only one.
I also see 0x168 and 0x195, depending on the link rate.
So, what am I missing?
> + .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0,
> +};
> +
> +static const struct skl_wrpll_params icl_tbt_pll_19_2MHz_values = {
> + .dco_integer = 0x1A5, .dco_fraction = 0x7000,
> + .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0,
> +};
> +
> static bool icl_calc_dp_combo_pll(struct drm_i915_private *dev_priv, int
> clock,
> struct skl_wrpll_params *pll_params)
> {
> @@ -2494,6 +2504,14 @@ static bool icl_calc_dp_combo_pll(struct
> drm_i915_private *dev_priv, int clock,
> return true;
> }
>
> +static bool icl_calc_tbt_pll(struct drm_i915_private *dev_priv, int clock,
> + struct skl_wrpll_params *pll_params)
> +{
> + *pll_params = dev_priv->cdclk.hw.ref == 24000 ?
> + icl_tbt_pll_24MHz_values : icl_tbt_pll_19_2MHz_values;
> + return true;
> +}
> +
> static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
> struct intel_encoder *encoder, int clock,
> struct intel_dpll_hw_state *pll_state)
> @@ -2503,7 +2521,9 @@ static bool icl_calc_dpll_state(struct intel_crtc_state
> *crtc_state,
> struct skl_wrpll_params pll_params = { 0 };
> bool ret;
>
> - if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> + if (intel_port_is_tc(dev_priv, encoder->port))
> + ret = icl_calc_tbt_pll(dev_priv, clock, _params);
> + else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> ret = cnl_ddi_calculate_wrpll(clock, dev_priv, _params);
> else
> ret = icl_calc_dp_combo_pll(dev_priv, clock, _params);
> --
> 2.14.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/icl: Add remaining registers and bitfields for MG PHY DDI
Em Qui, 2018-06-28 às 15:35 -0700, Manasi Navare escreveu:
> This patch adds the remaining register definitions and bit fields
> required for MG PHy DDI buffer initializations and voltage
> swing programming for MG PHy DDI ports.
>
> While at it this patch also fixes the naming for previously defined
> MG PHY registers in original commit id (c92f47b5ec977a "drm/i915/icl:
> Add register defs for voltage swing sequences for MG PHY DDI").
> Since the MG PHY registers are first defined in ICL platform, there
> is no need for _ICL prefix.
IMHO drive-by "do this other trivial thing"s are fine when it's only 1
or 2 small chunks affected. In this case it's half of the patch, so I
would prefer having it in a separate patch. But let's leave it as-is
here since at least the registers being touched are not used.
More below.
>
> v2:
> * Change the MG_TX_DRVCTL registers names to match the spec (Anusha)
>
> Signed-off-by: Manasi Navare
> Cc: Paulo Zanoni
> Cc: James Ausmus
> ---
> drivers/gpu/drm/i915/i915_reg.h | 246 +++---
> --
> 1 file changed, 145 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index c30cfcd..6119acc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1872,121 +1872,165 @@ enum i915_power_well_id {
> #define N_SCALAR(x)((x) << 24)
> #define N_SCALAR_MASK (0x7F << 24)
>
> -#define _ICL_MG_PHY_PORT_LN(port, ln, ln0p1, ln0p2, ln1p1) \
> +#define MG_PHY_PORT_LN(port, ln, ln0p1, ln0p2, ln1p1) \
> _MMIO(_PORT((port) - PORT_C, ln0p1, ln0p2) + (ln) * ((ln1p1)
> - (ln0p1)))
>
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT1 0x16812C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT1 0x16852C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT2 0x16912C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT2 0x16952C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT3 0x16A12C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT3 0x16A52C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT4 0x16B12C
> -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT4 0x16B52C
> -#define ICL_PORT_MG_TX1_LINK_PARAMS(port, ln) \
> - _ICL_MG_PHY_PORT_LN(port, ln,
> _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT1, \
> - _ICL_MG_TX_LINK_PARAMS_TX1LN0_
> PORT2, \
> - _ICL_MG_TX_LINK_PARAMS_TX1LN1_
> PORT1)
> -
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT1 0x1680AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT1 0x1684AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT2 0x1690AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT2 0x1694AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT3 0x16A0AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT3 0x16A4AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT4 0x16B0AC
> -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT4 0x16B4AC
> -#define ICL_PORT_MG_TX2_LINK_PARAMS(port, ln) \
> - _ICL_MG_PHY_PORT_LN(port, ln,
> _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT1, \
> - _ICL_MG_TX_LINK_PARAMS_TX2LN0_
> PORT2, \
> - _ICL_MG_TX_LINK_PARAMS_TX2LN1_
> PORT1)
> +#define MG_TX_LINK_PARAMS_TX1LN0_PORT1 0x16812C
> +#define MG_TX_LINK_PARAMS_TX1LN1_PORT1 0x16852C
> +#define MG_TX_LINK_PARAMS_TX1LN0_PORT2 0x16912C
> +#define MG_TX_LINK_PARAMS_TX1LN1_PORT2 0x16952C
> +#define MG_TX_LINK_PARAMS_TX1LN0_PORT3 0x16A12C
> +#define MG_TX_LINK_PARAMS_TX1LN1_PORT3 0x16A52C
> +#define MG_TX_LINK_PARAMS_TX1LN0_PORT4 0x16B12C
> +#define _MG_TX_LINK_PARAMS_TX1LN1_PORT4 0x16B52C
You left an underline on the line above.
> +#define MG_TX1_LINK_PARAMS(port, ln) \
> + MG_PHY_PORT_LN(port, ln, MG_TX_LINK_PARAMS_TX1LN0_PORT1, \
> + MG_TX_LINK_PARAMS_TX1LN0_PORT2, \
> + MG_TX_LINK_PARAMS_TX1LN1_PORT1)
> +
> +#define MG_TX_LINK_PARAMS_TX2LN0_PORT1 0x1680AC
> +#define MG_TX_LINK_PARAMS_TX2LN1_PORT1 0x1684AC
> +#define MG_TX_LINK_PARAMS_TX2LN0_PORT2 0x1690AC
> +#define MG_TX_LINK_PARAMS_TX2LN1_PORT2 0x1694AC
> +#define MG_TX_LINK_PARAMS_TX2LN0_PORT3 0x16A0AC
> +#define MG_TX_LINK_PARAMS_TX2LN1_PORT3 0x16A4AC
> +#define MG_TX_LINK_PARAMS_TX2LN0_PORT4 0x16B0AC
> +#define MG_TX_LINK_PARAMS_TX2LN1_PORT4 0x16B4AC
> +#define MG_TX2_LINK_PARAMS(port, ln) \
> + MG_PHY_PORT_LN(port, ln, MG_TX_LINK_PARAMS_TX2LN0_PORT1, \
> + MG_TX_LINK_PARAMS_TX2LN0_PORT2, \
> + MG_TX_LINK_PARAMS_TX2LN1_PORT1)
> #define CRI_USE_FS32 (1 << 5)
>
> -#define
Re: [Intel-gfx] [PATCH v2] drm/i915: kill resource streamer
On Thu, Jul 12, 2018 at 11:28:25PM +, De Marchi, Lucas wrote:
> On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote:
> > On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote:
> > > After disabling resource streamer on ICL (due to it actually not
> > > existing there), I got feedback that there have been some experimental
> > > patches for mesa to use it, but nothing ever landed nor shipped.
> > >
> > > This is a tentative to remove it from kernel keeping the uapi defines
> > > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
> > > some platforms - Daniele mentioned there's possible performance regression
> > > in gem_latency if this bit is not set. On this patch, I'm just removing
> > > it in order to get proper numbers.
> >
> > Since I see below 3 occurrences of
> > - .has_resource_streamer = 1
> >
> > I feel this commit message lacks of the reason why we are removing
> > this features for the platforms that already had it.
>
> /me confused. Isn't the first paragraph in the commit message exactly that?
/me confused now... For me the first paragraph tells that after removing
from ICL mesa might want to use it on ICL.
second paragraph tells that you are removing the support but keeping the uapi.
what I couldn't find is the explanation why exactly you are removing this
feature. Is this useless? No body using it? It is buggy? It is deprecated? why?
>
> Lucas De Marchi
>
> >
> > >
> > > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
> > > - don't bother trying to document removed params on uapi header:
> > > applications should know that from the query.
> > > (from Chris)
> > >
> > > Signed-off-by: Lucas De Marchi
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c| 2 +-
> > > drivers/gpu/drm/i915/i915_drv.h| 2 --
> > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
> > > drivers/gpu/drm/i915/i915_pci.c| 4
> > > drivers/gpu/drm/i915/intel_device_info.h | 1 -
> > > drivers/gpu/drm/i915/intel_lrc.c | 7 ++-
> > > drivers/gpu/drm/i915/intel_ringbuffer.c| 4 +---
> > > drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
> > > 8 files changed, 6 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 3eba3d1ab5b8..10423e2c1176 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> > > void *data,
> > > value = 2;
> > > break;
> > > case I915_PARAM_HAS_RESOURCE_STREAMER:
> > > - value = HAS_RESOURCE_STREAMER(dev_priv);
> > > + value = 0;
> > > break;
> > > case I915_PARAM_HAS_POOLED_EU:
> > > value = HAS_POOLED_EU(dev_priv);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 01dd29837233..dddb886fa06c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > #define USES_GUC_SUBMISSION(dev_priv)intel_uc_is_using_guc_submis
> > > sion()
> > > #define USES_HUC(dev_priv) intel_uc_is_using_huc()
> > >
> > > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)-
> > > >info.has_resource_streamer)
> > > -
> > > #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
> > >
> > > #define INTEL_PCH_DEVICE_ID_MASK 0xff80
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index 1932bc227942..ca21a08b2be9 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> > > if (!eb.engine)
> > > return -EINVAL;
> > >
> > > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> > > - if (!HAS_RESOURCE_STREAMER(eb.i915)) {
> > > - DRM_DEBUG("RS is only allowed for Haswell and
> > > Gen8 - Gen10\n");
> > > - return -EINVAL;
> > > - }
> > > - if (eb.engine->id != RCS) {
> > > - DRM_DEBUG("RS is not available on %s\n",
> > > - eb.engine->name);
> > > - return -EINVAL;
> > > - }
> > > -
> > > - eb.batch_flags |= I915_DISPATCH_RS;
> > > - }
> > > + if (args->flags & I915_EXEC_RESOURCE_STREAMER)
> > > + return -EINVAL;
> > >
> > > if (args->flags & I915_EXEC_FENCE_IN) {
> > > in_fence = sync_file_get_fence(lower_32_bits(args-
> > > >rsvd2));
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > b/drivers/gpu/drm/i915/i915_pci.c
> > > index c03ba0fe0845..7af19097dce8 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
[Intel-gfx] ✗ Fi.CI.BAT: failure for i915/intel_tv_get_modes: fix strncpy truncation warning (rev2)
== Series Details == Series: i915/intel_tv_get_modes: fix strncpy truncation warning (rev2) URL : https://patchwork.freedesktop.org/series/46314/ State : failure == Summary == Applying: i915/intel_tv_get_modes: fix strncpy truncation warning Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/intel_tv.c Falling back to patching base and 3-way merge... No changes -- Patch already applied. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: kill resource streamer
On Thu, 2018-07-12 at 16:06 -0700, Rodrigo Vivi wrote:
> On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote:
> > After disabling resource streamer on ICL (due to it actually not
> > existing there), I got feedback that there have been some experimental
> > patches for mesa to use it, but nothing ever landed nor shipped.
> >
> > This is a tentative to remove it from kernel keeping the uapi defines
> > around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
> > some platforms - Daniele mentioned there's possible performance regression
> > in gem_latency if this bit is not set. On this patch, I'm just removing
> > it in order to get proper numbers.
>
> Since I see below 3 occurrences of
> - .has_resource_streamer = 1
>
> I feel this commit message lacks of the reason why we are removing
> this features for the platforms that already had it.
/me confused. Isn't the first paragraph in the commit message exactly that?
Lucas De Marchi
>
> >
> > v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
> > - don't bother trying to document removed params on uapi header:
> > applications should know that from the query.
> > (from Chris)
> >
> > Signed-off-by: Lucas De Marchi
> > ---
> > drivers/gpu/drm/i915/i915_drv.c| 2 +-
> > drivers/gpu/drm/i915/i915_drv.h| 2 --
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
> > drivers/gpu/drm/i915/i915_pci.c| 4
> > drivers/gpu/drm/i915/intel_device_info.h | 1 -
> > drivers/gpu/drm/i915/intel_lrc.c | 7 ++-
> > drivers/gpu/drm/i915/intel_ringbuffer.c| 4 +---
> > drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
> > 8 files changed, 6 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 3eba3d1ab5b8..10423e2c1176 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> > void *data,
> > value = 2;
> > break;
> > case I915_PARAM_HAS_RESOURCE_STREAMER:
> > - value = HAS_RESOURCE_STREAMER(dev_priv);
> > + value = 0;
> > break;
> > case I915_PARAM_HAS_POOLED_EU:
> > value = HAS_POOLED_EU(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 01dd29837233..dddb886fa06c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > #define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submis
> > sion()
> > #define USES_HUC(dev_priv) intel_uc_is_using_huc()
> >
> > -#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)-
> > >info.has_resource_streamer)
> > -
> > #define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu)
> >
> > #define INTEL_PCH_DEVICE_ID_MASK 0xff80
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 1932bc227942..ca21a08b2be9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> > if (!eb.engine)
> > return -EINVAL;
> >
> > - if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> > - if (!HAS_RESOURCE_STREAMER(eb.i915)) {
> > - DRM_DEBUG("RS is only allowed for Haswell and
> > Gen8 - Gen10\n");
> > - return -EINVAL;
> > - }
> > - if (eb.engine->id != RCS) {
> > - DRM_DEBUG("RS is not available on %s\n",
> > -eb.engine->name);
> > - return -EINVAL;
> > - }
> > -
> > - eb.batch_flags |= I915_DISPATCH_RS;
> > - }
> > + if (args->flags & I915_EXEC_RESOURCE_STREAMER)
> > + return -EINVAL;
> >
> > if (args->flags & I915_EXEC_FENCE_IN) {
> > in_fence = sync_file_get_fence(lower_32_bits(args-
> > >rsvd2));
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index c03ba0fe0845..7af19097dce8 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -360,7 +360,6 @@ static const struct intel_device_info
> > intel_valleyview_info = {
> > .has_ddi = 1, \
> > .has_fpga_dbg = 1, \
> > .has_psr = 1, \
> > - .has_resource_streamer = 1, \
> > .has_dp_mst = 1, \
> > .has_rc6p = 0 /* RC6p removed-by HSW */, \
> > .has_runtime_pm = 1
> > @@ -433,7 +432,6 @@ static const struct intel_device_info
> > intel_cherryview_info = {
> > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> > .has_64bit_reloc = 1,
> > .has_runtime_pm = 1,
Re: [Intel-gfx] [PATCH] drm/i915/psr: Split sink status into a separate debugfs node
On Wed, Jul 04, 2018 at 05:31:21PM -0700, Dhinakaran Pandiyan wrote:
> This allows to read i915_edp_psr_status from tests without triggering
> any AUX communication. Take this opportunity to move this under the
> eDP-1 connector directory as the status we print is of the sink.
>
> Cc: Rodrigo Vivi
> Cc: José Roberto de Souza
> Suggested-by: Rodrigo Vivi
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 69
> +
> 1 file changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index f6142d78ede4..5069d5dedafe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2592,6 +2592,41 @@ static const struct file_operations
> i915_guc_log_relay_fops = {
> .release = i915_guc_log_relay_release,
> };
>
> +static int i915_psr_sink_status_show(struct seq_file *m, void *data)
> +{
> + u8 val;
> + static const char * const sink_status[] = {
> + "inactive",
> + "transition to active, capture and display",
> + "active, display from RFB",
> + "active, capture and display on sink device timings",
> + "transition to inactive, capture and display, timing re-sync",
> + "reserved",
> + "reserved",
> + "sink internal error",
> + };
> + struct drm_connector *connector = m->private;
> + struct intel_dp *intel_dp =
> + enc_to_intel_dp(_attached_encoder(connector)->base);
> +
> + if (connector->status != connector_status_connected)
> + return -ENODEV;
> +
> + if (drm_dp_dpcd_readb(_dp->aux, DP_PSR_STATUS, ) == 1) {
> + const char *str = "unknown";
> +
> + val &= DP_PSR_SINK_STATE_MASK;
> + if (val < ARRAY_SIZE(sink_status))
> + str = sink_status[val];
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val, str);
> + } else {
> + DRM_ERROR("dpcd read (at %u) failed\n", DP_PSR_STATUS);
> + }
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> +
> static void
> psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> {
> @@ -2643,26 +2678,6 @@ psr_source_status(struct drm_i915_private *dev_priv,
> struct seq_file *m)
> seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "unknown");
> }
>
> -static const char *psr_sink_status(u8 val)
> -{
> - static const char * const sink_status[] = {
> - "inactive",
> - "transition to active, capture and display",
> - "active, display from RFB",
> - "active, capture and display on sink device timings",
> - "transition to inactive, capture and display, timing re-sync",
> - "reserved",
> - "reserved",
> - "sink internal error"
> - };
> -
> - val &= DP_PSR_SINK_STATE_MASK;
> - if (val < ARRAY_SIZE(sink_status))
> - return sink_status[val];
> -
> - return "unknown";
> -}
> -
> static int i915_edp_psr_status(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -2706,15 +2721,6 @@ static int i915_edp_psr_status(struct seq_file *m,
> void *data)
> }
>
> psr_source_status(dev_priv, m);
> -
> - if (dev_priv->psr.enabled) {
> - struct drm_dp_aux *aux = _priv->psr.enabled->aux;
> - u8 val;
> -
> - if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, ) == 1)
> - seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> -psr_sink_status(val));
> - }
> mutex_unlock(_priv->psr.lock);
>
> if (READ_ONCE(dev_priv->psr.debug)) {
> @@ -4971,9 +4977,12 @@ int i915_debugfs_connector_add(struct drm_connector
> *connector)
> debugfs_create_file("i915_dpcd", S_IRUGO, root,
> connector, _dpcd_fops);
>
> - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> debugfs_create_file("i915_panel_timings", S_IRUGO, root,
> connector, _panel_fops);
> + debugfs_create_file("i915_psr_sink_status", S_IRUGO, root,
> + connector, _psr_sink_status_fops);
my OCD on names here about i915_psr vs i915_edp_psr is still a real thing
although
they are on different places ;)
We could probably remove "_edp" from the others, because psr is an edp only
after all
Anyways let's not block the progress here ;)
Reviewed-by: Rodrigo Vivi
> + }
>
> return 0;
> }
> --
> 2.14.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
>
Re: [Intel-gfx] [PATCH v2] drm/i915: kill resource streamer
On Thu, Jul 12, 2018 at 02:02:51PM -0700, Lucas De Marchi wrote:
> After disabling resource streamer on ICL (due to it actually not
> existing there), I got feedback that there have been some experimental
> patches for mesa to use it, but nothing ever landed nor shipped.
>
> This is a tentative to remove it from kernel keeping the uapi defines
> around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
> some platforms - Daniele mentioned there's possible performance regression
> in gem_latency if this bit is not set. On this patch, I'm just removing
> it in order to get proper numbers.
Since I see below 3 occurrences of
- .has_resource_streamer = 1
I feel this commit message lacks of the reason why we are removing
this features for the platforms that already had it.
>
> v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
> - don't bother trying to document removed params on uapi header:
> applications should know that from the query.
> (from Chris)
>
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/i915_drv.c| 2 +-
> drivers/gpu/drm/i915/i915_drv.h| 2 --
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
> drivers/gpu/drm/i915/i915_pci.c| 4
> drivers/gpu/drm/i915/intel_device_info.h | 1 -
> drivers/gpu/drm/i915/intel_lrc.c | 7 ++-
> drivers/gpu/drm/i915/intel_ringbuffer.c| 4 +---
> drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
> 8 files changed, 6 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3eba3d1ab5b8..10423e2c1176 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev,
> void *data,
> value = 2;
> break;
> case I915_PARAM_HAS_RESOURCE_STREAMER:
> - value = HAS_RESOURCE_STREAMER(dev_priv);
> + value = 0;
> break;
> case I915_PARAM_HAS_POOLED_EU:
> value = HAS_POOLED_EU(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 01dd29837233..dddb886fa06c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define USES_GUC_SUBMISSION(dev_priv)
> intel_uc_is_using_guc_submission()
> #define USES_HUC(dev_priv) intel_uc_is_using_huc()
>
> -#define HAS_RESOURCE_STREAMER(dev_priv)
> ((dev_priv)->info.has_resource_streamer)
> -
> #define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
>
> #define INTEL_PCH_DEVICE_ID_MASK 0xff80
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 1932bc227942..ca21a08b2be9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> if (!eb.engine)
> return -EINVAL;
>
> - if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> - if (!HAS_RESOURCE_STREAMER(eb.i915)) {
> - DRM_DEBUG("RS is only allowed for Haswell and Gen8 -
> Gen10\n");
> - return -EINVAL;
> - }
> - if (eb.engine->id != RCS) {
> - DRM_DEBUG("RS is not available on %s\n",
> - eb.engine->name);
> - return -EINVAL;
> - }
> -
> - eb.batch_flags |= I915_DISPATCH_RS;
> - }
> + if (args->flags & I915_EXEC_RESOURCE_STREAMER)
> + return -EINVAL;
>
> if (args->flags & I915_EXEC_FENCE_IN) {
> in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2));
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index c03ba0fe0845..7af19097dce8 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -360,7 +360,6 @@ static const struct intel_device_info
> intel_valleyview_info = {
> .has_ddi = 1, \
> .has_fpga_dbg = 1, \
> .has_psr = 1, \
> - .has_resource_streamer = 1, \
> .has_dp_mst = 1, \
> .has_rc6p = 0 /* RC6p removed-by HSW */, \
> .has_runtime_pm = 1
> @@ -433,7 +432,6 @@ static const struct intel_device_info
> intel_cherryview_info = {
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> .has_64bit_reloc = 1,
> .has_runtime_pm = 1,
> - .has_resource_streamer = 1,
> .has_rc6 = 1,
> .has_logical_ring_contexts = 1,
> .has_gmch_display = 1,
> @@ -506,7 +504,6 @@ static const struct intel_device_info
> intel_skylake_gt4_info = {
> .has_runtime_pm = 1, \
> .has_pooled_eu = 0, \
> .has_csr =
Re: [Intel-gfx] [PATCH] drm/i915/guc: Protect against NULL client dereference in error path
On Thu, Jul 12, 2018 at 09:20:27PM +0100, Chris Wilson wrote: > After aborting a module load, we may try and disable guc before we have > finished setting it. Long term plan is to ensure perfect onion unwind, > but in the short term we want to fix the oops to re-enable > drv_module_reload. > > [ 317.401239] BUG: unable to handle kernel NULL pointer dereference at > 0030 > [ 317.401279] Oops: [#1] PREEMPT SMP PTI > [ 317.401294] CPU: 5 PID: 4275 Comm: drv_module_relo Tainted: G U > 4.18.0-rc4-CI-CI_DRM_4476+ #1 > [ 317.401317] Hardware name: System manufacturer System Product > Name/Z170M-PLUS, BIOS 3610 03/29/2018 > [ 317.401440] RIP: 0010:unreserve_doorbell+0x0/0x80 [i915] > [ 317.401454] Code: bb e0 48 8b 35 21 4d 18 00 49 c7 c0 a8 e5 62 a0 b9 cc 00 > 00 00 48 c7 c2 d8 41 5f a0 48 c7 c7 c9 f6 53 a0 e8 a2 3d c2 e0 0f 0b <0f> b7 > 47 30 66 3d 00 01 74 20 48 8b 57 18 48 0f a3 82 40 05 00 00 > [ 317.401602] RSP: 0018:c93d3da0 EFLAGS: 00010246 > [ 317.401619] RAX: 8223b300 RBX: RCX: > > [ 317.401636] RDX: 001fffc0 RSI: 880219f115f0 RDI: > > [ 317.401654] RBP: 880219f11838 R08: R09: > > [ 317.401671] R10: R11: R12: > 880219f11300 > [ 317.401689] R13: 880219f17770 R14: 88022c1daef8 R15: > a06ae950 > [ 317.401707] FS: 7febf77a9980() GS:880236d4() > knlGS: > [ 317.401727] CS: 0010 DS: ES: CR0: 80050033 > [ 317.401743] CR2: 0030 CR3: 000222072003 CR4: > 003606e0 > [ 317.401761] DR0: DR1: DR2: > > [ 317.401779] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 317.401796] Call Trace: > [ 317.401894] guc_client_free+0x9/0x130 [i915] > [ 317.401993] intel_guc_submission_fini+0x50/0x90 [i915] > [ 317.402092] intel_uc_fini+0x34/0xd0 [i915] > [ 317.402179] i915_gem_fini+0x5c/0x100 [i915] > [ 317.402249] i915_driver_unload+0xd2/0x110 [i915] > [ 317.402321] i915_pci_remove+0x10/0x20 [i915] > [ 317.402341] pci_device_remove+0x36/0xb0 > [ 317.402357] device_release_driver_internal+0x185/0x250 > [ 317.402374] driver_detach+0x35/0x70 > [ 317.402390] bus_remove_driver+0x53/0xd0 > [ 317.402404] pci_unregister_driver+0x25/0xa0 > [ 317.402423] __se_sys_delete_module+0x162/0x210 > [ 317.402439] ? do_syscall_64+0xd/0x190 > [ 317.402454] do_syscall_64+0x55/0x190 > [ 317.402470] entry_SYSCALL_64_after_hwframe+0x49/0xbe > [ 317.402485] RIP: 0033:0x7febf6e5d1b7 > [ 317.402496] Code: 73 01 c3 48 8b 0d d1 8c 2c 00 f7 d8 64 89 01 48 83 c8 ff > c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 8b 0d a1 8c 2c 00 f7 d8 64 89 01 48 > [ 317.402646] RSP: 002b:7fffb5e72798 EFLAGS: 0206 ORIG_RAX: > 00b0 > [ 317.402667] RAX: ffda RBX: RCX: > 7febf6e5d1b7 > [ 317.402686] RDX: RSI: 0800 RDI: > 562da1addd98 > [ 317.402703] RBP: 562da1addd30 R08: 562da1addd9c R09: > 7fffb5e727d8 > [ 317.402721] R10: 7fffb5e71794 R11: 0206 R12: > 562da0ff6470 > > Testcase: igt/drv_module_reload/basic-reload-inject > Signed-off-by: Chris Wilson > Cc: Michał Winiarski > Cc: Michal Wajdeczko Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c > b/drivers/gpu/drm/i915/intel_guc_submission.c > index cd51be8ff025..22367131d6a1 100644 > --- a/drivers/gpu/drm/i915/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/intel_guc_submission.c > @@ -1128,7 +1128,8 @@ static void guc_clients_destroy(struct intel_guc *guc) > guc_client_free(client); > > client = fetch_and_zero(>execbuf_client); > - guc_client_free(client); > + if (client) > + guc_client_free(client); > } > > /* > -- > 2.18.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: kill resource streamer
On 12/07/18 14:02, Lucas De Marchi wrote:
After disabling resource streamer on ICL (due to it actually not
existing there), I got feedback that there have been some experimental
patches for mesa to use it, but nothing ever landed nor shipped.
This is a tentative to remove it from kernel keeping the uapi defines
around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
some platforms - Daniele mentioned there's possible performance regression
in gem_latency if this bit is not set. On this patch, I'm just removing
it in order to get proper numbers.
I've re-run gem_latency on the same machine I saw the regression on with
and without this patch applied. I've tried both with 2 and 10 contexts
and the regression I saw with a similar patch a few months back seems to
not be happening. The average numbers with the patch applied came out
very slightly worse (10 iterations of 60s, ~1% extra latency average)
but I'm assuming that's within the noise (what I saw at the time was
~20% latency increase).
Acked-by: Daniele Ceraolo Spurio
Daniele
v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
- don't bother trying to document removed params on uapi header:
applications should know that from the query.
(from Chris)
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/i915_drv.c| 2 +-
drivers/gpu/drm/i915/i915_drv.h| 2 --
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
drivers/gpu/drm/i915/i915_pci.c| 4
drivers/gpu/drm/i915/intel_device_info.h | 1 -
drivers/gpu/drm/i915/intel_lrc.c | 7 ++-
drivers/gpu/drm/i915/intel_ringbuffer.c| 4 +---
drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
8 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3eba3d1ab5b8..10423e2c1176 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void
*data,
value = 2;
break;
case I915_PARAM_HAS_RESOURCE_STREAMER:
- value = HAS_RESOURCE_STREAMER(dev_priv);
+ value = 0;
break;
case I915_PARAM_HAS_POOLED_EU:
value = HAS_POOLED_EU(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01dd29837233..dddb886fa06c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
#define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission()
#define USES_HUC(dev_priv)intel_uc_is_using_huc()
-#define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer)
-
#define HAS_POOLED_EU(dev_priv) ((dev_priv)->info.has_pooled_eu)
#define INTEL_PCH_DEVICE_ID_MASK 0xff80
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1932bc227942..ca21a08b2be9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (!eb.engine)
return -EINVAL;
- if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
- if (!HAS_RESOURCE_STREAMER(eb.i915)) {
- DRM_DEBUG("RS is only allowed for Haswell and Gen8 -
Gen10\n");
- return -EINVAL;
- }
- if (eb.engine->id != RCS) {
- DRM_DEBUG("RS is not available on %s\n",
-eb.engine->name);
- return -EINVAL;
- }
-
- eb.batch_flags |= I915_DISPATCH_RS;
- }
+ if (args->flags & I915_EXEC_RESOURCE_STREAMER)
+ return -EINVAL;
if (args->flags & I915_EXEC_FENCE_IN) {
in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2));
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index c03ba0fe0845..7af19097dce8 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -360,7 +360,6 @@ static const struct intel_device_info intel_valleyview_info
= {
.has_ddi = 1, \
.has_fpga_dbg = 1, \
.has_psr = 1, \
- .has_resource_streamer = 1, \
.has_dp_mst = 1, \
.has_rc6p = 0 /* RC6p removed-by HSW */, \
.has_runtime_pm = 1
@@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info
= {
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.has_64bit_reloc = 1,
.has_runtime_pm = 1,
- .has_resource_streamer = 1,
.has_rc6 = 1,
.has_logical_ring_contexts = 1,
.has_gmch_display = 1,
@@ -506,7
Re: [Intel-gfx] [PATCH] i915/intel_tv_get_modes: fix strncpy truncation warning
Ville Syrjälä wrote on Thu, Jul 12, 2018: > On Thu, Jul 12, 2018 at 03:55:26PM +0200, Dominique Martinet wrote: > > This could either be 'this commit' as a whole or if you look only at the > > commit message 'this strncpy fix' from the title (which is arguably the > > same), and both interpretations sound fairly understandable in the > > context of the title line without seeing the patch to me... Although > > I'll admit this is difficult to judge of that as the author. > > The patch subject is not part of the commit message body though. This is > made all the more clear when I'm editing the response in vim that doesn't > even show the mail subject to me. Hence I'm always left in the dark by > commit messages that aren't fully self contained. Ah, that is a fair point - I thought you were referring to the patch itself, not the subject. My mail client does include the subject in the editor so I hadn't considered that, but I understand where you come from now and agree. I will be more mindful of that as the v2 has the same problem. > > Yes and no, I gave it for referrence but when you update to gcc 8 you > > will literally see it all over the place. > > The words "strncpy truncation warning" is really precise once you've > > seen them a few times and there are litteraly hundred of these warnings > > in the kernel, some have already been fixed taking a glance at the git > > log, some with and without the warning message. > > I don't think it's worth polluting the git log with this many > > warnings... Which leads to... > > I disagree. Without knowing what exactly is fixed how can you judge > whether the patch even makes sense? And later you may get another > report of the same warning and then you would want to look through > the git log to see if there's a patch that already fixed it. Quite > hard to do without the exact warning in the log. I might just be tired of this specific warning; I've fixed it countless times in different projects these past few months and it's coming out of my eyes at this point. I definitely agree in general, just -Wstringop-truncation has been showing up everywhere and it's always the same, with many occurences I don't consider to be bugs (like here because we forcefully terminate the last byte of the string afterwards), so it's really lost value to me. I included it as a comment precisely for your first point (so you can tell the patch makes sense now) but I do not feel any regret not recording it, and I still stand by what I said: if all you want is examples of patches that already fix it, I've just had a look at git log in drm trees and there already have been many fixes, most of which provided a warning similar to the one I got. Attaching the full warning messages makes sense if the warning is new/rare but if it's the same as 5 other commits in semi-recent history I do not see much point. Anyway, I would be enclined to add it just to comply now but it looks like Chris already picked the v2 up, so there is not much point in arguing, sorry for disagreeing. -- Dominique Martinet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915/intel_tv_get_modes: fix strncpy truncation warning
Ville Syrjälä wrote on Thu, Jul 12, 2018: > On Wed, Jul 11, 2018 at 09:46:15AM +0200, Dominique Martinet wrote: > > This is effectively no-op as the next line writes a nul at the final > > What is "This". Please write self contained commit messages. This could either be 'this commit' as a whole or if you look only at the commit message 'this strncpy fix' from the title (which is arguably the same), and both interpretations sound fairly understandable in the context of the title line without seeing the patch to me... Although I'll admit this is difficult to judge of that as the author. Thanksfully, the v2 of the patch didn't use this wording but while I agree the message could be better I do not think it was horrible. > > drivers/gpu/drm/i915/intel_tv.c: In function ‘intel_tv_get_modes’: > > drivers/gpu/drm/i915/intel_tv.c:1358:3: error: ‘strncpy’ specified bound 32 > > equals destination size [-Werror=stringop-truncation] > >strncpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN); > >^~ > > cc1: all warnings being treated as errors > > That warning should be in the actual commit message. Yes and no, I gave it for referrence but when you update to gcc 8 you will literally see it all over the place. The words "strncpy truncation warning" is really precise once you've seen them a few times and there are litteraly hundred of these warnings in the kernel, some have already been fixed taking a glance at the git log, some with and without the warning message. I don't think it's worth polluting the git log with this many warnings... Which leads to... > This same pattern is used all over drm. Can you go and fix them all up? > One might even consider writing a cocci patch for it ;) Now this is something I can agree with. This patch really was just a stop-gap measure because I could not build the kernel at all without it, but yes I did consider having a look at others. Unfortunately coccinelle does not run on fedora 28 (and doesn't look like it will fix itself any time soon, there is a bug report[1] open since February that didn't get much love lately - I was just looking at it a few days ago) I think in this case it might actually be faster to look at gcc warnings and s/strncpy/strlcpy/, but I am curious about Coccinelle so this is a good excuse to look at it, I'll report back in a bit after poking at that bug report and figuring out how coccinelle works. I do not guarantee speed however, if anyone sees this and feels put off from donig it themselves, please go ahead and just drop me a word. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1544204 Thanks, and sorry for the mail longer than I originally intended, -- Dominique Martinet ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] i915/intel_tv_get_modes: fix strncpy truncation warning
Change it to use strlcpy instead Signed-off-by: Dominique Martinet --- drivers/gpu/drm/i915/intel_tv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index b55b5c157e38..25fee7dba7e2 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1355,8 +1355,7 @@ intel_tv_get_modes(struct drm_connector *connector) mode_ptr = drm_mode_create(connector->dev); if (!mode_ptr) continue; - strncpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN); - mode_ptr->name[DRM_DISPLAY_MODE_LEN - 1] = '\0'; + strlcpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN); mode_ptr->hdisplay = hactive_s; mode_ptr->hsync_start = hactive_s + 1; -- 2.17.1 ___ 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 1/9] trace.pl: Improve time axis labels
On 7/12/2018 5:42 AM, Tvrtko Ursulin wrote:
On 12/07/2018 11:59, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
It is possible to customize the axis display so change it to display
timestamps in seconds on the major axis (with six decimal spaces) and
relative millisecond offsets on the minor axis.
Signed-off-by: Tvrtko Ursulin
Suggested-by: Chris Wilson
Cc: Chris Wilson
Cc: John Harrison
---
scripts/trace.pl | 32
1 file changed, 32 insertions(+)
diff --git a/scripts/trace.pl b/scripts/trace.pl
index fc1713e4f9a7..89491125490d 100755
--- a/scripts/trace.pl
+++ b/scripts/trace.pl
@@ -1000,6 +1000,37 @@ $first_ts = ts($first_ts);
print <
This logic is unfortunately a bit dodgy. It looked like major label is
getting called before the minor ones, so I thought I could calculate
the relative offset for the minor for a more readable display. But
after scrolling and zooming around timelines some more I can see cases
where that doesn't seem to be the case.
Please have a play and if you think it is bad they only option will be
to absolute time for minor labels as well.
Something is completely broken for me. I am getting massively out of
whack timeline values. For example, a log file entry of 761846.983987 is
being displayed as -59196268.216013s! Even ignoring that, I'm not
convinced about the relative offset thing. It makes it much harder to
correlate the graph with the trace file entries, e.g. if trying to work
out exactly what happened when something went peculiar in a test. Would
it be possible to make it dynamically switchable between raw absolute
values and the funky change-as-you-scroll relative ones?
Also, it doesn't seem especially useful to have the values be relative
to the left most edge of the visible portion of the timeline. That means
that when you scroll sideways along the timeline, the time-stamp of any
given point wanders up and down accordingly. That just seems confusing
to me! Maybe it makes sense for some use cases but I would have thought
that having the values be relative to the start of the trace would be
more use. Or even have clickable anchor points so that you can set the
origin to be whatever exciting event you are interested in? Although
that might be way too complex to be worth the effort of implementing :).
Regards,
Tvrtko
+
+ if (ms < 0)
+ return '';
+
+ if (scale == 'millisecond')
+ return "+" + ms.toFixed(3) + "ms";
+ else if (scale == 'second')
+ return ms.toFixed(3) + "s";
+ else
+ return ms.toFixed(0) + "s";
+ }
+
// Configuration for the Timeline
var options = { groupOrder: 'content',
horizontalScroll: true,
@@ -1007,6 +1038,7 @@ print <
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Kill resource streamer (rev2)
== Series Details ==
Series: Kill resource streamer (rev2)
URL : https://patchwork.freedesktop.org/series/46224/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9640 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46224/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9640:
=== IGT changes ===
Possible regressions
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
{fi-cfl-8109u}: PASS -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_9640 that come from known issues:
=== IGT changes ===
Possible fixes
igt@drv_module_reload@basic-reload:
fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2
igt@kms_chamelium@hdmi-hpd-fast:
fi-kbl-7500u: FAIL (fdo#103841, fdo#102672) -> SKIP
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: INCOMPLETE (fdo#107139) -> DMESG-WARN (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (46 -> 42) ==
Additional (1): fi-byt-j1900
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4479 -> Patchwork_9640
CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9640: 2997ade936fa14aa75840c30d62237891853f586 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
2997ade936fa drm/i915: kill resource streamer
a668fe24a019 drm/i915/icl: move has_resource_streamer to GEN11_FEATURES
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9640/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Kill resource streamer (rev2)
== Series Details == Series: Kill resource streamer (rev2) URL : https://patchwork.freedesktop.org/series/46224/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/icl: move has_resource_streamer to GEN11_FEATURES Okay! Commit: drm/i915: kill resource streamer -drivers/gpu/drm/i915/selftests/../i915_drv.h:3655:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
Em Sex, 2018-06-08 às 00:49 +0100, Chris Wilson escreveu:
> Quoting Paulo Zanoni (2018-06-08 00:07:00)
> > static void
> > skl_print_wm_changes(const struct drm_atomic_state *state)
> > {
> > @@ -5381,7 +5370,10 @@ static void skl_initial_wm(struct
> > intel_atomic_state *state,
> > if (cstate->base.active_changed)
> > skl_atomic_update_crtc_wm(state, cstate);
> >
> > - skl_copy_ddb_for_pipe(hw_vals, results, pipe);
> > + memcpy(hw_vals->ddb.uv_plane[pipe], results-
> > >ddb.uv_plane[pipe],
> > + sizeof(hw_vals->ddb.uv_plane[pipe]));
>
> I must be seeing things.
>
> ddb.uv_plane[pipe] must be a pointer, right?
No, it's an array of structs.
> Therefore sizeof(ddb.uv_plane[pipe]) must the size of that pointer
> and
> not of the struct.
It is the size of the array, which is 20 (both before and after the
patch). Each member has size 4.
> Do you not mean sizeof(*ddb.uv_plane[pipe]) ?
What's written above means "size of the first element of
ddb.uv_plane[pipe]". This sizeof operation returns 4 instead of 20. We
don't want to copy just the first element of uv_plane[pipe].
>
> Definitely time to stop reading code...
> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: kill resource streamer
After disabling resource streamer on ICL (due to it actually not
existing there), I got feedback that there have been some experimental
patches for mesa to use it, but nothing ever landed nor shipped.
This is a tentative to remove it from kernel keeping the uapi defines
around for compatibility. We may need to keep CTX_CTRL_RS_CTX_ENABLE on
some platforms - Daniele mentioned there's possible performance regression
in gem_latency if this bit is not set. On this patch, I'm just removing
it in order to get proper numbers.
v2: - re-add the inadvertent removal of CTX_CTRL_INHIBIT_SYN_CTX_SWITCH
- don't bother trying to document removed params on uapi header:
applications should know that from the query.
(from Chris)
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/i915_drv.c| 2 +-
drivers/gpu/drm/i915/i915_drv.h| 2 --
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++-
drivers/gpu/drm/i915/i915_pci.c| 4
drivers/gpu/drm/i915/intel_device_info.h | 1 -
drivers/gpu/drm/i915/intel_lrc.c | 7 ++-
drivers/gpu/drm/i915/intel_ringbuffer.c| 4 +---
drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
8 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3eba3d1ab5b8..10423e2c1176 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -373,7 +373,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void
*data,
value = 2;
break;
case I915_PARAM_HAS_RESOURCE_STREAMER:
- value = HAS_RESOURCE_STREAMER(dev_priv);
+ value = 0;
break;
case I915_PARAM_HAS_POOLED_EU:
value = HAS_POOLED_EU(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01dd29837233..dddb886fa06c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2609,8 +2609,6 @@ intel_info(const struct drm_i915_private *dev_priv)
#define USES_GUC_SUBMISSION(dev_priv) intel_uc_is_using_guc_submission()
#define USES_HUC(dev_priv) intel_uc_is_using_huc()
-#define HAS_RESOURCE_STREAMER(dev_priv)
((dev_priv)->info.has_resource_streamer)
-
#define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu)
#define INTEL_PCH_DEVICE_ID_MASK 0xff80
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1932bc227942..ca21a08b2be9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -2221,19 +2221,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (!eb.engine)
return -EINVAL;
- if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
- if (!HAS_RESOURCE_STREAMER(eb.i915)) {
- DRM_DEBUG("RS is only allowed for Haswell and Gen8 -
Gen10\n");
- return -EINVAL;
- }
- if (eb.engine->id != RCS) {
- DRM_DEBUG("RS is not available on %s\n",
-eb.engine->name);
- return -EINVAL;
- }
-
- eb.batch_flags |= I915_DISPATCH_RS;
- }
+ if (args->flags & I915_EXEC_RESOURCE_STREAMER)
+ return -EINVAL;
if (args->flags & I915_EXEC_FENCE_IN) {
in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2));
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index c03ba0fe0845..7af19097dce8 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -360,7 +360,6 @@ static const struct intel_device_info intel_valleyview_info
= {
.has_ddi = 1, \
.has_fpga_dbg = 1, \
.has_psr = 1, \
- .has_resource_streamer = 1, \
.has_dp_mst = 1, \
.has_rc6p = 0 /* RC6p removed-by HSW */, \
.has_runtime_pm = 1
@@ -433,7 +432,6 @@ static const struct intel_device_info intel_cherryview_info
= {
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
.has_64bit_reloc = 1,
.has_runtime_pm = 1,
- .has_resource_streamer = 1,
.has_rc6 = 1,
.has_logical_ring_contexts = 1,
.has_gmch_display = 1,
@@ -506,7 +504,6 @@ static const struct intel_device_info
intel_skylake_gt4_info = {
.has_runtime_pm = 1, \
.has_pooled_eu = 0, \
.has_csr = 1, \
- .has_resource_streamer = 1, \
.has_rc6 = 1, \
.has_dp_mst = 1, \
.has_logical_ring_contexts = 1, \
@@ -593,7 +590,6 @@ static const struct intel_device_info intel_cannonlake_info
= {
GEN(11), \
.ddb_size = 2048, \
.has_csr = 0, \
- .has_resource_streamer = 0, \
.has_logical_ring_elsq = 1
static const struct
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Protect against NULL client dereference in error path
== Series Details == Series: drm/i915/guc: Protect against NULL client dereference in error path URL : https://patchwork.freedesktop.org/series/46436/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9639 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46436/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9639 that come from known issues: === IGT changes === Possible fixes igt@drv_module_reload@basic-reload: fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2 igt@kms_chamelium@hdmi-hpd-fast: fi-kbl-7500u: FAIL (fdo#103841, fdo#102672) -> SKIP igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 == Participating hosts (46 -> 42) == Additional (1): fi-byt-j1900 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4479 -> Patchwork_9639 CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9639: ba03836d3c10b89ed56af04d9159981540c62f90 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ba03836d3c10 drm/i915/guc: Protect against NULL client dereference in error path == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9639/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915/gtt: Disable read-only support under GVT
> -Original Message- > From: Chris Wilson > Sent: Thursday, July 12, 2018 11:53 AM > To: intel-gfx@lists.freedesktop.org > Cc: Chris Wilson ; Zhenyu Wang > ; Bloomfield, Jon ; > Joonas Lahtinen ; Matthew Auld > > Subject: [PATCH 3/6] drm/i915/gtt: Disable read-only support under GVT > > GVT is not propagating the PTE bits, and is always setting the > read-write bit, thus breaking read-only support. > > Signed-off-by: Chris Wilson > Cc: Zhenyu Wang > Cc: Jon Bloomfield > Cc: Joonas Lahtinen > Cc: Matthew Auld > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 6c0b438afe46..8e70a45b8a90 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1662,8 +1662,12 @@ static struct i915_hw_ppgtt > *gen8_ppgtt_create(struct drm_i915_private *i915) > 1ULL << 48 : > 1ULL << 32; > > - /* From bdw, there is support for read-only pages in the PPGTT */ > - ppgtt->vm.has_read_only = true; > + /* > + * From bdw, there is support for read-only pages in the PPGTT. > + * > + * XXX GVT is not setting honouring the PTE bits. > + */ > + ppgtt->vm.has_read_only = !intel_vgpu_active(i915); > > i915_address_space_init(>vm, i915); > > -- > 2.18.0 Is there a blocker that prevents gvt respecting this bit? I can't think of an obvious reason why it would be a bad thing to support. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/guc: Protect against NULL client dereference in error path
After aborting a module load, we may try and disable guc before we have finished setting it. Long term plan is to ensure perfect onion unwind, but in the short term we want to fix the oops to re-enable drv_module_reload. [ 317.401239] BUG: unable to handle kernel NULL pointer dereference at 0030 [ 317.401279] Oops: [#1] PREEMPT SMP PTI [ 317.401294] CPU: 5 PID: 4275 Comm: drv_module_relo Tainted: G U 4.18.0-rc4-CI-CI_DRM_4476+ #1 [ 317.401317] Hardware name: System manufacturer System Product Name/Z170M-PLUS, BIOS 3610 03/29/2018 [ 317.401440] RIP: 0010:unreserve_doorbell+0x0/0x80 [i915] [ 317.401454] Code: bb e0 48 8b 35 21 4d 18 00 49 c7 c0 a8 e5 62 a0 b9 cc 00 00 00 48 c7 c2 d8 41 5f a0 48 c7 c7 c9 f6 53 a0 e8 a2 3d c2 e0 0f 0b <0f> b7 47 30 66 3d 00 01 74 20 48 8b 57 18 48 0f a3 82 40 05 00 00 [ 317.401602] RSP: 0018:c93d3da0 EFLAGS: 00010246 [ 317.401619] RAX: 8223b300 RBX: RCX: [ 317.401636] RDX: 001fffc0 RSI: 880219f115f0 RDI: [ 317.401654] RBP: 880219f11838 R08: R09: [ 317.401671] R10: R11: R12: 880219f11300 [ 317.401689] R13: 880219f17770 R14: 88022c1daef8 R15: a06ae950 [ 317.401707] FS: 7febf77a9980() GS:880236d4() knlGS: [ 317.401727] CS: 0010 DS: ES: CR0: 80050033 [ 317.401743] CR2: 0030 CR3: 000222072003 CR4: 003606e0 [ 317.401761] DR0: DR1: DR2: [ 317.401779] DR3: DR6: fffe0ff0 DR7: 0400 [ 317.401796] Call Trace: [ 317.401894] guc_client_free+0x9/0x130 [i915] [ 317.401993] intel_guc_submission_fini+0x50/0x90 [i915] [ 317.402092] intel_uc_fini+0x34/0xd0 [i915] [ 317.402179] i915_gem_fini+0x5c/0x100 [i915] [ 317.402249] i915_driver_unload+0xd2/0x110 [i915] [ 317.402321] i915_pci_remove+0x10/0x20 [i915] [ 317.402341] pci_device_remove+0x36/0xb0 [ 317.402357] device_release_driver_internal+0x185/0x250 [ 317.402374] driver_detach+0x35/0x70 [ 317.402390] bus_remove_driver+0x53/0xd0 [ 317.402404] pci_unregister_driver+0x25/0xa0 [ 317.402423] __se_sys_delete_module+0x162/0x210 [ 317.402439] ? do_syscall_64+0xd/0x190 [ 317.402454] do_syscall_64+0x55/0x190 [ 317.402470] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 317.402485] RIP: 0033:0x7febf6e5d1b7 [ 317.402496] Code: 73 01 c3 48 8b 0d d1 8c 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a1 8c 2c 00 f7 d8 64 89 01 48 [ 317.402646] RSP: 002b:7fffb5e72798 EFLAGS: 0206 ORIG_RAX: 00b0 [ 317.402667] RAX: ffda RBX: RCX: 7febf6e5d1b7 [ 317.402686] RDX: RSI: 0800 RDI: 562da1addd98 [ 317.402703] RBP: 562da1addd30 R08: 562da1addd9c R09: 7fffb5e727d8 [ 317.402721] R10: 7fffb5e71794 R11: 0206 R12: 562da0ff6470 Testcase: igt/drv_module_reload/basic-reload-inject Signed-off-by: Chris Wilson Cc: Michał Winiarski Cc: Michal Wajdeczko --- drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index cd51be8ff025..22367131d6a1 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -1128,7 +1128,8 @@ static void guc_clients_destroy(struct intel_guc *guc) guc_client_free(client); client = fetch_and_zero(>execbuf_client); - guc_client_free(client); + if (client) + guc_client_free(client); } /* -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2)
== Series Details ==
Series: series starting with [1/6] drm/i915/gtt: Add read only pages to
gen8_pte_encode (rev2)
URL : https://patchwork.freedesktop.org/series/46432/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9638 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46432/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9638:
=== IGT changes ===
Possible regressions
igt@gem_exec_suspend@basic-s3:
{fi-cfl-8109u}: PASS -> DMESG-WARN
== Known issues ==
Here are the changes found in Patchwork_9638 that come from known issues:
=== IGT changes ===
Issues hit
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
Possible fixes
igt@drv_module_reload@basic-reload:
fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2
igt@kms_chamelium@hdmi-hpd-fast:
fi-kbl-7500u: FAIL (fdo#102672, fdo#103841) -> SKIP
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
== Participating hosts (46 -> 42) ==
Additional (1): fi-byt-j1900
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4479 -> Patchwork_9638
CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9638: b8de49af2a4bb5b4276b9fd7a800647bf0f2587f @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b8de49af2a4b drm/i915/userptr: Enable read-only support on gen8+
059c35166df9 drm/i915: Reject attempted pwrites into a read-only object
edf93a9f453c drm/i915: Prevent writing into a read-only object via a GGTT mmap
f63aea6e09de drm/i915/gtt: Disable read-only support under GVT
e60d2578b1b0 drm/i915/gtt: Read-only pages for insert_entries on bdw+
7f97b3f09826 drm/i915/gtt: Add read only pages to gen8_pte_encode
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9638/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2)
== Series Details == Series: series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2) URL : https://patchwork.freedesktop.org/series/46432/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/gtt: Add read only pages to gen8_pte_encode Okay! Commit: drm/i915/gtt: Read-only pages for insert_entries on bdw+ +drivers/gpu/drm/i915/selftests/i915_gem_context.c:517:25: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/i915_gem_context.c:517:25: warning: expression using sizeof(void) Commit: drm/i915/gtt: Disable read-only support under GVT Okay! Commit: drm/i915: Prevent writing into a read-only object via a GGTT mmap Okay! Commit: drm/i915: Reject attempted pwrites into a read-only object Okay! Commit: drm/i915/userptr: Enable read-only support on gen8+ Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2)
== Series Details ==
Series: series starting with [1/6] drm/i915/gtt: Add read only pages to
gen8_pte_encode (rev2)
URL : https://patchwork.freedesktop.org/series/46432/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
7f97b3f09826 drm/i915/gtt: Add read only pages to gen8_pte_encode
e60d2578b1b0 drm/i915/gtt: Read-only pages for insert_entries on bdw+
-:192: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#192: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:336:
+ bool pt_kmap_wc:1;
-:195: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#195: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:339:
+ bool has_read_only:1;
-:271: WARNING:LINE_SPACING: Missing a blank line after declarations
#271: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:441:
+ struct drm_file *file;
+ I915_RND_STATE(prng);
-:342: ERROR:CODE_INDENT: code indent should use tabs where possible
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
total: 1 errors, 4 warnings, 1 checks, 323 lines checked
f63aea6e09de drm/i915/gtt: Disable read-only support under GVT
edf93a9f453c drm/i915: Prevent writing into a read-only object via a GGTT mmap
-:182: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#182: FILE: include/drm/drm_vma_manager.h:44:
+ bool readonly:1;
total: 0 errors, 1 warnings, 0 checks, 118 lines checked
059c35166df9 drm/i915: Reject attempted pwrites into a read-only object
b8de49af2a4b drm/i915/userptr: Enable read-only support on gen8+
___
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/6] drm/i915/gtt: Add read only pages to gen8_pte_encode
== Series Details == Series: series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode URL : https://patchwork.freedesktop.org/series/46432/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9637 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46432/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9637 that come from known issues: === IGT changes === Issues hit igt@debugfs_test@read_all_entries: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) Possible fixes igt@drv_module_reload@basic-reload: fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2 igt@kms_chamelium@hdmi-hpd-fast: fi-kbl-7500u: FAIL (fdo#103841, fdo#102672) -> SKIP fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387 == Participating hosts (46 -> 42) == Additional (1): fi-byt-j1900 Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4479 -> Patchwork_9637 CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9637: eda0213a1a4ed9dd213afef1071fe6653c16041b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == eda0213a1a4e drm/i915/userptr: Enable read-only support on gen8+ 607bbdb2cc10 drm/i915: Reject attempted pwrites into a read-only object 0dd3530a2408 drm/i915: Prevent writing into a read-only object via a GGTT mmap b9303f1f2584 drm/i915/gtt: Disable read-only support under GVT 81df64c8eb7c drm/i915/gtt: Read-only pages for insert_entries on bdw+ 14eaf2339094 drm/i915/gtt: Add read only pages to gen8_pte_encode == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9637/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/userptr: Enable read-only support on gen8+
On gen8 and onwards, we can mark GPU accesses through the ppGTT as being
read-only, that is cause any GPU write onto that page to be discarded
(not triggering a fault). This is all that we need to finally support
the read-only flag for userptr!
v2: Check default address space for read only support as a proxy for the
user context/ppgtt.
Testcase: igt/gem_userptr_blits/readonly*
Signed-off-by: Chris Wilson
Cc: Jon Bloomfield
Cc: Joonas Lahtinen
Reviewed-by: Joonas Lahtinen
Reviewed-by: Matthew Auld
Reviewed-by: Jon Bloomfield
---
drivers/gpu/drm/i915/i915_gem_object.h | 1 -
drivers/gpu/drm/i915/i915_gem_userptr.c | 18 --
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h
b/drivers/gpu/drm/i915/i915_gem_object.h
index 56e9f00d2c4c..83e5e01fa9ea 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -267,7 +267,6 @@ struct drm_i915_gem_object {
union {
struct i915_gem_userptr {
uintptr_t ptr;
- unsigned read_only :1;
struct i915_mm_struct *mm;
struct i915_mmu_object *mmu_object;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 854bd51b9478..dcd6e230d16a 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -507,7 +507,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
struct mm_struct *mm = obj->userptr.mm->mm;
unsigned int flags = 0;
- if (!obj->userptr.read_only)
+ if (!i915_gem_object_is_readonly(obj))
flags |= FOLL_WRITE;
ret = -EFAULT;
@@ -643,7 +643,7 @@ static int i915_gem_userptr_get_pages(struct
drm_i915_gem_object *obj)
if (pvec) /* defer to worker if malloc fails */
pinned = __get_user_pages_fast(obj->userptr.ptr,
num_pages,
- !obj->userptr.read_only,
+
!i915_gem_object_is_readonly(obj),
pvec);
}
@@ -789,10 +789,15 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -EFAULT;
if (args->flags & I915_USERPTR_READ_ONLY) {
- /* On almost all of the current hw, we cannot tell the GPU that
a
-* page is readonly, so this is just a placeholder in the uAPI.
+ struct i915_hw_ppgtt *ppgtt;
+
+ /*
+* On almost all of the older hw, we cannot tell the GPU that
+* a page is readonly.
*/
- return -ENODEV;
+ ppgtt = dev_priv->kernel_context->ppgtt;
+ if (!ppgtt || !ppgtt->vm.has_read_only)
+ return -ENODEV;
}
obj = i915_gem_object_alloc(dev_priv);
@@ -806,7 +811,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
obj->userptr.ptr = args->user_ptr;
- obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY);
+ if (args->flags & I915_USERPTR_READ_ONLY)
+ i915_gem_object_set_readonly(obj);
/* And keep a pointer to the current->mm for resolving the user pages
* at binding. This means that we need to hook into the mmu_notifier
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode
== Series Details == Series: series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode URL : https://patchwork.freedesktop.org/series/46432/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915/gtt: Add read only pages to gen8_pte_encode Okay! Commit: drm/i915/gtt: Read-only pages for insert_entries on bdw+ +drivers/gpu/drm/i915/selftests/i915_gem_context.c:517:25: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/i915_gem_context.c:517:25: warning: expression using sizeof(void) Commit: drm/i915/gtt: Disable read-only support under GVT Okay! Commit: drm/i915: Prevent writing into a read-only object via a GGTT mmap Okay! Commit: drm/i915: Reject attempted pwrites into a read-only object Okay! Commit: drm/i915/userptr: Enable read-only support on gen8+ Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode
== Series Details ==
Series: series starting with [1/6] drm/i915/gtt: Add read only pages to
gen8_pte_encode
URL : https://patchwork.freedesktop.org/series/46432/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
14eaf2339094 drm/i915/gtt: Add read only pages to gen8_pte_encode
81df64c8eb7c drm/i915/gtt: Read-only pages for insert_entries on bdw+
-:192: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#192: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:336:
+ bool pt_kmap_wc:1;
-:195: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#195: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:339:
+ bool has_read_only:1;
-:271: WARNING:LINE_SPACING: Missing a blank line after declarations
#271: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:441:
+ struct drm_file *file;
+ I915_RND_STATE(prng);
-:342: ERROR:CODE_INDENT: code indent should use tabs where possible
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:512:
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
total: 1 errors, 4 warnings, 1 checks, 323 lines checked
b9303f1f2584 drm/i915/gtt: Disable read-only support under GVT
0dd3530a2408 drm/i915: Prevent writing into a read-only object via a GGTT mmap
-:182: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool
bitfields as unsigned int or u<8|16|32>
#182: FILE: include/drm/drm_vma_manager.h:44:
+ bool readonly:1;
total: 0 errors, 1 warnings, 0 checks, 118 lines checked
607bbdb2cc10 drm/i915: Reject attempted pwrites into a read-only object
eda0213a1a4e drm/i915/userptr: Enable read-only support on gen8+
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/userptr: Enable read-only support on gen8+
Quoting Chris Wilson (2018-07-12 19:53:15)
> @@ -789,10 +789,12 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> return -EFAULT;
>
> if (args->flags & I915_USERPTR_READ_ONLY) {
> - /* On almost all of the current hw, we cannot tell the GPU
> that a
> -* page is readonly, so this is just a placeholder in the
> uAPI.
> + /*
> +* On almost all of the older hw, we cannot tell the GPU that
> +* a page is readonly.
> */
> - return -ENODEV;
> + if (INTEL_GEN(dev_priv) < 8 || !USES_PPGTT(dev_priv))
> + return -ENODEV;
Hmm, I need to be more careful here considering gvt.
-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] igt/gem_unfence_active_buffers: Check GEM before use
On Thu, Jul 12, 2018 at 04:40:03PM +0100, Chris Wilson wrote: > As we want to make the buffers active on the GPU before removing their > fence, an operational GPU (not wedged!) is required. > > Signed-off-by: Chris Wilson Reviewed-by: Ville Syrjälä > --- > tests/gem_unfence_active_buffers.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/gem_unfence_active_buffers.c > b/tests/gem_unfence_active_buffers.c > index 6df23cc53..b78fbafa7 100644 > --- a/tests/gem_unfence_active_buffers.c > +++ b/tests/gem_unfence_active_buffers.c > @@ -74,6 +74,7 @@ igt_simple_main > data[i] = i; > > fd = drm_open_driver(DRIVER_INTEL); > + igt_require_gem(fd); > > bufmgr = drm_intel_bufmgr_gem_init(fd, 4096); > drm_intel_bufmgr_gem_enable_reuse(bufmgr); > -- > 2.18.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/2] igt/gem_gpgpu_fill: Check for GEM before use
On Thu, Jul 12, 2018 at 04:37:08PM +0100, Chris Wilson wrote: > As we need GEM and the GPU to do a GPGPU fill, we should check that it > is operable before using -- skipping rather than failing when the device > is wedged. > > Signed-off-by: Chris Wilson Series is Reviewed-by: Ville Syrjälä > --- > tests/gem_gpgpu_fill.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/gem_gpgpu_fill.c b/tests/gem_gpgpu_fill.c > index 8ef05a3f0..dfb581652 100644 > --- a/tests/gem_gpgpu_fill.c > +++ b/tests/gem_gpgpu_fill.c > @@ -104,6 +104,7 @@ igt_simple_main > > data.drm_fd = drm_open_driver_render(DRIVER_INTEL); > data.devid = intel_get_drm_devid(data.drm_fd); > + igt_require_gem(data.drm_fd); > > data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096); > igt_assert(data.bufmgr); > -- > 2.18.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] igt/gem_render_copy_redux: Check for GEM before use
On Thu, Jul 12, 2018 at 04:38:49PM +0100, Chris Wilson wrote:
> As render copy wants to use the GPU, we should make sure it is not
> wedged first.
>
> Signed-off-by: Chris Wilson
Reviewed-by: Ville Syrjälä
> ---
> tests/gem_render_copy_redux.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/gem_render_copy_redux.c b/tests/gem_render_copy_redux.c
> index 27098ea6d..a861862d0 100644
> --- a/tests/gem_render_copy_redux.c
> +++ b/tests/gem_render_copy_redux.c
> @@ -209,6 +209,7 @@ int main(int argc, char **argv)
>
> igt_fixture {
> data_init();
> + igt_require_gem(data.fd);
> }
>
> igt_subtest("normal") {
> --
> 2.18.0
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915/userptr: Enable read-only support on gen8+
On gen8 and onwards, we can mark GPU accesses through the ppGTT as being
read-only, that is cause any GPU write onto that page to be discarded
(not triggering a fault). This is all that we need to finally support
the read-only flag for userptr!
Testcase: igt/gem_userptr_blits/readonly*
Signed-off-by: Chris Wilson
Cc: Jon Bloomfield
Cc: Joonas Lahtinen
Reviewed-by: Joonas Lahtinen
Reviewed-by: Matthew Auld
Reviewed-by: Jon Bloomfield
---
drivers/gpu/drm/i915/i915_gem_object.h | 1 -
drivers/gpu/drm/i915/i915_gem_userptr.c | 15 +--
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h
b/drivers/gpu/drm/i915/i915_gem_object.h
index 56e9f00d2c4c..83e5e01fa9ea 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -267,7 +267,6 @@ struct drm_i915_gem_object {
union {
struct i915_gem_userptr {
uintptr_t ptr;
- unsigned read_only :1;
struct i915_mm_struct *mm;
struct i915_mmu_object *mmu_object;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 854bd51b9478..045db5ef17ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -507,7 +507,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct
*_work)
struct mm_struct *mm = obj->userptr.mm->mm;
unsigned int flags = 0;
- if (!obj->userptr.read_only)
+ if (!i915_gem_object_is_readonly(obj))
flags |= FOLL_WRITE;
ret = -EFAULT;
@@ -643,7 +643,7 @@ static int i915_gem_userptr_get_pages(struct
drm_i915_gem_object *obj)
if (pvec) /* defer to worker if malloc fails */
pinned = __get_user_pages_fast(obj->userptr.ptr,
num_pages,
- !obj->userptr.read_only,
+
!i915_gem_object_is_readonly(obj),
pvec);
}
@@ -789,10 +789,12 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -EFAULT;
if (args->flags & I915_USERPTR_READ_ONLY) {
- /* On almost all of the current hw, we cannot tell the GPU that
a
-* page is readonly, so this is just a placeholder in the uAPI.
+ /*
+* On almost all of the older hw, we cannot tell the GPU that
+* a page is readonly.
*/
- return -ENODEV;
+ if (INTEL_GEN(dev_priv) < 8 || !USES_PPGTT(dev_priv))
+ return -ENODEV;
}
obj = i915_gem_object_alloc(dev_priv);
@@ -806,7 +808,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
obj->userptr.ptr = args->user_ptr;
- obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY);
+ if (args->flags & I915_USERPTR_READ_ONLY)
+ i915_gem_object_set_readonly(obj);
/* And keep a pointer to the current->mm for resolving the user pages
* at binding. This means that we need to hook into the mmu_notifier
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915/gtt: Disable read-only support under GVT
GVT is not propagating the PTE bits, and is always setting the read-write bit, thus breaking read-only support. Signed-off-by: Chris Wilson Cc: Zhenyu Wang Cc: Jon Bloomfield Cc: Joonas Lahtinen Cc: Matthew Auld --- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 6c0b438afe46..8e70a45b8a90 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1662,8 +1662,12 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915) 1ULL << 48 : 1ULL << 32; - /* From bdw, there is support for read-only pages in the PPGTT */ - ppgtt->vm.has_read_only = true; + /* +* From bdw, there is support for read-only pages in the PPGTT. +* +* XXX GVT is not setting honouring the PTE bits. +*/ + ppgtt->vm.has_read_only = !intel_vgpu_active(i915); i915_address_space_init(>vm, i915); -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915/gtt: Read-only pages for insert_entries on bdw+
From: Jon Bloomfield
Hook up the flags to allow read-only ppGTT mappings for gen8+
v2: Include a selftest to check that writes to a readonly PTE are
dropped
v3: Don't duplicate cpu_check() as we can just reuse it, and even worse
don't wholesale copy the theory-of-operation comment from igt_ctx_exec
without changing it to explain the intention behind the new test!
v4: Joonas really likes magic mystery values
Signed-off-by: Jon Bloomfield
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Matthew Auld
Reviewed-by: Joonas Lahtinen
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ---
drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
.../gpu/drm/i915/selftests/i915_gem_context.c | 112 +-
4 files changed, 153 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7292f41eb752..6c0b438afe46 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -204,7 +204,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
return err;
}
- /* Currently applicable only to VLV */
+ /* Applicable to VLV, and gen8+ */
pte_flags = 0;
if (vma->obj->gt_ro)
pte_flags |= PTE_READ_ONLY;
@@ -1044,10 +1044,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt
*ppgtt,
struct i915_page_directory_pointer *pdp,
struct sgt_dma *iter,
struct gen8_insert_pte *idx,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
struct i915_page_directory *pd;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
gen8_pte_t *vaddr;
bool ret;
@@ -1098,14 +1099,14 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt
*ppgtt,
static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
gen8_ppgtt_insert_pte_entries(ppgtt, >pdp, , ,
- cache_level);
+ cache_level, flags);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
}
@@ -1113,9 +1114,10 @@ static void gen8_ppgtt_insert_3lvl(struct
i915_address_space *vm,
static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
struct i915_page_directory_pointer
**pdps,
struct sgt_dma *iter,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
u64 start = vma->node.start;
dma_addr_t rem = iter->sg->length;
@@ -1231,19 +1233,21 @@ static void gen8_ppgtt_insert_huge_entries(struct
i915_vma *vma,
static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
- gen8_ppgtt_insert_huge_entries(vma, pdps, , cache_level);
+ gen8_ppgtt_insert_huge_entries(vma, pdps, , cache_level,
+ flags);
} else {
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
-, , cache_level))
+, , cache_level,
+flags))
GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
@@ -1658,6 +1662,9 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct
[Intel-gfx] [PATCH 4/6] drm/i915: Prevent writing into a read-only object via a GGTT mmap
If the user has created a read-only object, they should not be allowed
to circumvent the write protection by using a GGTT mmapping. Deny it.
Also most machines do not support read-only GGTT PTEs, so again we have
to reject attempted writes. Fortunately, this is known a priori, so we
can at least reject in the call to create the mmap (with a sanity check
in the fault handler).
v2: Check the vma->vm_flags during mmap() to allow readonly access.
v3: Remove VM_MAYWRITE to curtail mprotect()
Testcase: igt/gem_userptr_blits/readonly_mmap*
Signed-off-by: Chris Wilson
Cc: Jon Bloomfield
Cc: Joonas Lahtinen
Cc: Matthew Auld
Cc: David Herrmann
Reviewed-by: Matthew Auld #v1
Reviewed-by: Jon Bloomfield
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/drm_gem.c | 9 +
drivers/gpu/drm/i915/i915_gem.c | 4
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++-
drivers/gpu/drm/i915/i915_gem_object.h| 13 -
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 5 +++--
include/drm/drm_vma_manager.h | 1 +
7 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4a16d7b26c89..bf90625df3c5 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1036,6 +1036,15 @@ int drm_gem_mmap(struct file *filp, struct
vm_area_struct *vma)
return -EACCES;
}
+ if (node->readonly) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
vma);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed2be33ec58a..1910c66f48e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2012,6 +2012,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
pgoff_t page_offset;
int ret;
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
/* We don't use vmf->pgoff since that has the fake offset */
page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8e70a45b8a90..da0e9870a66f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -206,7 +206,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV, and gen8+ */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
@@ -2491,8 +2491,10 @@ static void gen8_ggtt_insert_entries(struct
i915_address_space *vm,
const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
- /* The GTT does not support read-only mappings */
- GEM_BUG_ON(flags & PTE_READ_ONLY);
+ /*
+* Note that we ignore PTE_READ_ONLY here. The caller must be careful
+* not to allow the user to override access to a read only page.
+*/
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
gtt_entries += vma->node.start >> PAGE_SHIFT;
@@ -2731,7 +2733,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
- if (obj->gt_ro)
+ if (i915_gem_object_is_readonly(obj))
pte_flags |= PTE_READ_ONLY;
intel_runtime_pm_get(i915);
@@ -2769,7 +2771,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
/* Currently applicable only to VLV */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
if (flags & I915_VMA_LOCAL_BIND) {
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h
b/drivers/gpu/drm/i915/i915_gem_object.h
index c3c6f2e588fb..56e9f00d2c4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -141,7 +141,6 @@ struct drm_i915_gem_object {
* Is the object to be mapped as read-only to the GPU
* Only honoured if hardware has relevant pte bit
*/
- unsigned long gt_ro:1;
unsigned int cache_level:3;
unsigned int cache_coherent:2;
#define I915_BO_CACHE_COHERENT_FOR_READ BIT(0)
@@ -358,6 +357,18 @@ static inline void i915_gem_object_unlock(struct
drm_i915_gem_object *obj)
reservation_object_unlock(obj->resv);
}
+static
[Intel-gfx] [PATCH 5/6] drm/i915: Reject attempted pwrites into a read-only object
If the user created a read-only object, they should not be allowed to
circumvent the write protection using the pwrite ioctl.
Signed-off-by: Chris Wilson
Cc: Jon Bloomfield
Cc: Joonas Lahtinen
Cc: Matthew Auld
Reviewed-by: Jon Bloomfield
Reviewed-by: Joonas Lahtinen
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_gem.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1910c66f48e2..42d24410a98c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1627,6 +1627,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
goto err;
}
+ /* Writes not allowed into this read-only object */
+ if (i915_gem_object_is_readonly(obj)) {
+ ret = -EINVAL;
+ goto err;
+ }
+
trace_i915_gem_object_pwrite(obj, args->offset, args->size);
ret = -ENODEV;
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915/gtt: Add read only pages to gen8_pte_encode
From: Jon Bloomfield
We can set a bit inside the ppGTT PTE to indicate a page is read-only;
writes from the GPU will be discarded. We can use this to protect pages
and in particular support read-only userptr mappings (necessary for
importing PROT_READ vma).
Signed-off-by: Jon Bloomfield
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Matthew Auld
Reviewed-by: Joonas Lahtinen
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d0acef299b9c..7292f41eb752 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -244,10 +244,13 @@ static void clear_pages(struct i915_vma *vma)
}
static gen8_pte_t gen8_pte_encode(dma_addr_t addr,
- enum i915_cache_level level)
+ enum i915_cache_level level,
+ u32 flags)
{
- gen8_pte_t pte = _PAGE_PRESENT | _PAGE_RW;
- pte |= addr;
+ gen8_pte_t pte = addr | _PAGE_PRESENT | _PAGE_RW;
+
+ if (unlikely(flags & PTE_READ_ONLY))
+ pte &= ~_PAGE_RW;
switch (level) {
case I915_CACHE_NONE:
@@ -721,7 +724,7 @@ static void gen8_initialize_pt(struct i915_address_space
*vm,
struct i915_page_table *pt)
{
fill_px(vm, pt,
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC));
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0));
}
static void gen6_initialize_pt(struct gen6_hw_ppgtt *ppgtt,
@@ -869,7 +872,7 @@ static bool gen8_ppgtt_clear_pt(struct i915_address_space
*vm,
unsigned int pte = gen8_pte_index(start);
unsigned int pte_end = pte + num_entries;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
gen8_pte_t *vaddr;
GEM_BUG_ON(num_entries > pt->used_ptes);
@@ -1044,7 +1047,7 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
enum i915_cache_level cache_level)
{
struct i915_page_directory *pd;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
gen8_pte_t *vaddr;
bool ret;
@@ -1112,7 +1115,7 @@ static void gen8_ppgtt_insert_huge_entries(struct
i915_vma *vma,
struct sgt_dma *iter,
enum i915_cache_level cache_level)
{
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
u64 start = vma->node.start;
dma_addr_t rem = iter->sg->length;
@@ -1578,7 +1581,7 @@ static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt,
struct seq_file *m)
{
struct i915_address_space *vm = >vm;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
u64 start = 0, length = ppgtt->vm.total;
if (use_4lvl(vm)) {
@@ -2461,7 +2464,7 @@ static void gen8_ggtt_insert_page(struct
i915_address_space *vm,
gen8_pte_t __iomem *pte =
(gen8_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
- gen8_set_pte(pte, gen8_pte_encode(addr, level));
+ gen8_set_pte(pte, gen8_pte_encode(addr, level, 0));
ggtt->invalidate(vm->i915);
}
@@ -2474,7 +2477,7 @@ static void gen8_ggtt_insert_entries(struct
i915_address_space *vm,
struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
struct sgt_iter sgt_iter;
gen8_pte_t __iomem *gtt_entries;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
@@ -2542,7 +2545,7 @@ static void gen8_ggtt_clear_range(struct
i915_address_space *vm,
unsigned first_entry = start >> PAGE_SHIFT;
unsigned num_entries = length >> PAGE_SHIFT;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
gen8_pte_t __iomem *gtt_base =
(gen8_pte_t __iomem *)ggtt->gsm + first_entry;
const int max_entries = ggtt_total_entries(ggtt) - first_entry;
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Interactive RPS mode (rev4)
== Series Details ==
Series: drm/i915: Interactive RPS mode (rev4)
URL : https://patchwork.freedesktop.org/series/46334/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4479 -> Patchwork_9636 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46334/revisions/4/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9636:
=== IGT changes ===
Possible regressions
igt@gem_mmap_gtt@basic-copy:
{fi-kbl-8809g}: NOTRUN -> DMESG-WARN +9
== Known issues ==
Here are the changes found in Patchwork_9636 that come from known issues:
=== IGT changes ===
Issues hit
igt@kms_flip@basic-flip-vs-dpms:
fi-skl-6700hq: PASS -> DMESG-WARN (fdo#105998)
Possible fixes
igt@drv_module_reload@basic-reload:
fi-ilk-650: DMESG-WARN (fdo#106387) -> PASS +2
igt@kms_chamelium@hdmi-hpd-fast:
fi-kbl-7500u: FAIL (fdo#103841, fdo#102672) -> SKIP
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: INCOMPLETE (fdo#107139) -> DMESG-WARN (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (46 -> 42) ==
Additional (1): fi-byt-j1900
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4479 -> Patchwork_9636
CI_DRM_4479: c9c54a1c37adefd414d51ed919aab3f94794ee35 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9636: bfc0fb4a0c59ca1c807cb0f0cd3a872f37c6372d @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
bfc0fb4a0c59 drm/i915: Interactive RPS mode
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9636/issues.html
___
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: Interactive RPS mode (rev4)
== Series Details == Series: drm/i915: Interactive RPS mode (rev4) URL : https://patchwork.freedesktop.org/series/46334/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: Interactive RPS mode -drivers/gpu/drm/i915/selftests/../i915_drv.h:3655:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3659:16: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Interactive RPS mode (rev4)
== Series Details ==
Series: drm/i915: Interactive RPS mode (rev4)
URL : https://patchwork.freedesktop.org/series/46334/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
bfc0fb4a0c59 drm/i915: Interactive RPS mode
-:24: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit e9af4ea2b9e7 ("drm/i915: Avoid
waitboosting on the active request")'
#24:
full throttle and upclock the GPU to max. But in commit e9af4ea2b9e7, we
-:74: CHECK:UNCOMMENTED_DEFINITION: struct mutex definition without comment
#74: FILE: drivers/gpu/drm/i915/i915_drv.h:788:
+ struct mutex power_lock;
-:82: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#82: FILE: drivers/gpu/drm/i915/i915_drv.h:3437:
+extern void intel_rps_set_interactive(struct drm_i915_private *dev_priv,
total: 1 errors, 0 warnings, 2 checks, 183 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RESEND] drm/i915: Interactive RPS mode
RPS provides a feedback loop where we use the load during the previous
evaluation interval to decide whether to up or down clock the GPU
frequency. Our responsiveness is split into 3 regimes, a high and low
plateau with the intent to keep the gpu clocked high to cover occasional
stalls under high load, and low despite occasional glitches under steady
low load, and inbetween. However, we run into situations like kodi where
we want to stay at low power (video decoding is done efficiently
inside the fixed function HW and doesn't need high clocks even for high
bitrate streams), but just occasionally the pipeline is more complex
than a video decode and we need a smidgen of extra GPU power to present
on time. In the high power regime, we sample at sub frame intervals with
a bias to upclocking, and conversely at low power we sample over a few
frames worth to provide what we consider to be the right levels of
responsiveness respectively. At low power, we more or less expect to be
kicked out to high power at the start of a busy sequence by waitboosting.
Prior to commit e9af4ea2b9e7 ("drm/i915: Avoid waitboosting on the active
request") whenever we missed the frame or stalled, we would immediate go
full throttle and upclock the GPU to max. But in commit e9af4ea2b9e7, we
relaxed the waitboosting to only apply if the pipeline was deep to avoid
over-committing resources for a near miss. Sadly though, a near miss is
still a miss, and perceptible as jitter in the frame delivery.
To try and prevent the near miss before having to resort to boosting
after the fact, we use the pageflip queue as an indication that we are
in an "interactive" regime and so should sample the load more frequently
to provide power before the frame misses it vblank. This will make us
more favorable to providing a small power increase (one or two bins) as
required rather than going all the way to maximum and then having to
work back down again. (We still keep the waitboosting mechanism around
just in case a dramatic change in system load requires urgent uplocking,
faster than we can provide in a few evaluation intervals.)
v2: Reduce rps_set_interactive to a boolean parameter to avoid the
confusion of what if they wanted a new power mode after pinning to a
different mode (which to choose?)
v3: Only reprogram RPS while the GT is awake, it will be set when we
wake the GT, and while off warns about being used outside of rpm.
v4: Fix deferred application of interactive mode
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107111
Fixes: e9af4ea2b9e7 ("drm/i915: Avoid waitboosting on the active request")
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Radoslaw Szwichtenberg
---
drivers/gpu/drm/i915/i915_debugfs.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 4 ++
drivers/gpu/drm/i915/intel_display.c | 20 ++
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 91 +++-
5 files changed, 89 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 099f97ef2303..ac019bb927d0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2218,6 +2218,7 @@ static int i915_rps_boost_info(struct seq_file *m, void
*data)
seq_printf(m, "CPU waiting? %d\n", count_irq_waiters(dev_priv));
seq_printf(m, "Boosts outstanding? %d\n",
atomic_read(>num_waiters));
+ seq_printf(m, "Interactive? %d\n", READ_ONCE(rps->interactive));
seq_printf(m, "Frequency requested %d\n",
intel_gpu_freq(dev_priv, rps->cur_freq));
seq_printf(m, " min hard:%d, soft:%d; max soft:%d, hard:%d\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01dd29837233..f02fbeee553f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -784,6 +784,8 @@ struct intel_rps {
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
+ unsigned int interactive;
+ struct mutex power_lock;
bool enabled;
atomic_t num_waiters;
@@ -3429,6 +3431,8 @@ extern void i915_redisable_vga_power_on(struct
drm_i915_private *dev_priv);
extern bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val);
extern void intel_init_pch_refclk(struct drm_i915_private *dev_priv);
extern int intel_set_rps(struct drm_i915_private *dev_priv, u8 val);
+extern void intel_rps_set_interactive(struct drm_i915_private *dev_priv,
+ bool state);
extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
bool enable);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7998e70a3174..5809366ff9f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13104,6 +13104,19 @@
Re: [Intel-gfx] [PATCH] drm/i915/guc: Don't read SOFT_SCRATCH(15) on MMIO error
On Thu, 12 Jul 2018 17:31:14 +0200, Chris Wilson
wrote:
Quoting Chris Wilson (2018-05-29 15:54:12)
Quoting Michal Wajdeczko (2018-05-28 18:16:18)
> SOFT_SCRATCH(15) is used by GuC for sending MMIO GuC events to host
and
> those events are now handled by
intel_guc_to_host_event_handler_mmio().
>
> We should not try to read it on MMIO action error as 1) we may be
using
> different set of registers for GuC MMIO communication, and 2) GuC may
> use CTB mechanism for sending events to host.
Ok.
> While here, upgrade error message to DRM_ERROR.
Does the error help? What do you want to convey to the user? For error
handling, we want to propagate the result back anyway for the caller has
to decide what to do next.
Good news! We see the error in BAT,
[ 542.138479] i915: unknown parameter 'enable_guc_loading' ignored
[ 542.138483] i915: unknown parameter 'enable_guc_submission' ignored
[ 542.138485] Setting dangerous option enable_guc - tainting kernel
[ 542.138488] Setting dangerous option live_selftests - tainting kernel
[ 542.173291] [drm:intel_guc_send_mmio [i915]] *ERROR* MMIO: GuC action
0x10 failed with error -5 0xf000f000
[ 542.367055] i915: probe of :00:02.0 failed with error -25
And Michał reminded me this wasn't the first time...
commit feb06c151fade9ecaa3dd410d792cce26e8b10de
Author: Michał Winiarski
Date: Mon Mar 19 10:53:47 2018 +0100
drm/i915/guc: Demote GuC error messages
We're using those functions in selftests, and the callers are expected
to do the error handling anyways. Let's demote all GuC actions and
doorbell creation to DEBUG_DRIVER.
So do we kindly ask Michał to resubmit his fix?
There are more places where DRM_ERROR is used after detection GuC error
(see intel_guc_send_ct as example, more to show up shortly)
I would rather prefer to add GUC_ERROR macro that could be tweaked
under SELFTEST config and runtime flags to demote unwanted errors:
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#define GUC_ERROR(...) \
do { \
if (unlikely(i915_selftest.mock || i915_selftest.live)) \
DRM_DEBUG_DRIVER(__VA_ARGS__); \
else \
DRM_ERROR(__VA_ARGS__);
} while (0);
#else
#define GUC_ERROR(...) DRM_ERROR(__VA_ARGS__)
#endif
/Michal
___
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: remove confusing GPIO vs PCH_GPIO
On Thu, Jul 12, 2018 at 07:03:58PM +0200, Daniel Vetter wrote:
> On Thu, Jul 12, 2018 at 6:24 PM, Lucas De Marchi
> wrote:
> > On Thu, Jul 12, 2018 at 05:04:03PM +0200, Daniel Vetter wrote:
> >> On Mon, Jul 09, 2018 at 09:22:21AM -0700, Lucas De Marchi wrote:
> >> > Instead of defining all registers twice, define just a PCH_GPIO_BASE
> >> > that has the same address as PCH_GPIO_A and use that to calculate all
> >> > the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing
> >> > the same thing.
> >> >
> >> > This also rewrites the GMBUS[05] registers since they depend on
> >> > gpio_mmio_base.
> >> >
> >> > v2: Fix GMBUS registers to be relative to gpio base; create GPIO()
> >> > macro to return a particular gpio address and move the enum out of
> >> > i915_reg.h (suggested by Jani)
> >> >
> >> > Signed-off-by: Lucas De Marchi
> >> > ---
> >> > drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
> >> > drivers/gpu/drm/i915/i915_reg.h | 53 +++--
> >> > drivers/gpu/drm/i915/intel_drv.h| 16 +
> >> > drivers/gpu/drm/i915/intel_i2c.c| 16 -
> >> > 4 files changed, 52 insertions(+), 35 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> >> > b/drivers/gpu/drm/i915/gvt/handlers.c
> >> > index e39492aaff6c..e25a74fe753b 100644
> >> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> >> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> >> > @@ -2084,7 +2084,7 @@ static int init_generic_mmio_info(struct intel_gvt
> >> > *gvt)
> >> >
> >> > MMIO_F(PCH_GMBUS0, 4 * 4, 0, 0, 0, D_ALL, gmbus_mmio_read,
> >> > gmbus_mmio_write);
> >> > - MMIO_F(PCH_GPIOA, 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
> >> > + MMIO_F(_MMIO(PCH_GPIO_BASE), 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL,
> >> > NULL);
> >> > MMIO_F(_MMIO(0xe4f00), 0x28, 0, 0, 0, D_ALL, NULL, NULL);
> >> >
> >> > MMIO_F(_MMIO(_PCH_DPB_AUX_CH_CTL), 6 * 4, 0, 0, 0, D_PRE_SKL, NULL,
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 0424e45f88db..f8f71d577613 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -3088,18 +3088,11 @@ enum i915_power_well_id {
> >> > /*
> >> > * GPIO regs
> >> > */
> >> > -#define GPIOA _MMIO(0x5010)
> >> > -#define GPIOB _MMIO(0x5014)
> >> > -#define GPIOC _MMIO(0x5018)
> >> > -#define GPIOD _MMIO(0x501c)
> >> > -#define GPIOE _MMIO(0x5020)
> >> > -#define GPIOF _MMIO(0x5024)
> >> > -#define GPIOG _MMIO(0x5028)
> >> > -#define GPIOH _MMIO(0x502c)
> >> > -#define GPIOJ _MMIO(0x5034)
> >> > -#define GPIOK _MMIO(0x5038)
> >> > -#define GPIOL _MMIO(0x503C)
> >> > -#define GPIOM _MMIO(0x5040)
> >> > +#define GPIO_OFFSET0x5010u
> >> > +#define PCH_GPIO_BASE (0xcu + GPIO_OFFSET)
> >> > +#define VLV_GPIO_BASE (VLV_DISPLAY_BASE + GPIO_OFFSET)
> >>
> >> This is a rather peculiar choice of baseline address. I'd either go with
> >> 0x5000u or 0xu (which avoids the need to change all the gmbus macros).
> >
> > I'm all for a round 0x5 number, however it doesn't match the spec.
> > I don't understand how 0x0 would make sense here. Are you suggesting to
> > embed the GPIO_OFFSET into the GPIO macro and get rid of the GPIO_BASE?
> >
> > #define GPIO_OFFSET 0x5010u
> > /* THESE 2 BELOW COULD USE ANOTHER BETTER NAME */
> > #define PCH_GPIO_BASE 0xcu
> > #define VLV_GPIO_BASE VLV_DISPLAY_BASE
> > #define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 0x5010 + 4 *
> > (gpio))
>
> Yeah that's what I had in mind.
I think I like this one the most. Keeps the 0x5010 in the GPIO() macro
so less confusion when looking it up in the spec (except on PCH platforms
perhaps).
Also I would drop the VLV_GPIO_BASE and PCH_GPIO_BASE definitions and
just do gpio_mmio_Base = VLV_DISPLAY_BASE/PCH_DISPLAY_BASE or something
like that.
I've occasionally pondered about some kind of generic south_display_base
thing that could maybe cover all the things that live on the PCH side on
those platforms. But I never bothered to look at all the register offsets
hard enough to figure out whether it'd be actually useful.
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: remove confusing GPIO vs PCH_GPIO
On Thu, Jul 12, 2018 at 6:24 PM, Lucas De Marchi
wrote:
> On Thu, Jul 12, 2018 at 05:04:03PM +0200, Daniel Vetter wrote:
>> On Mon, Jul 09, 2018 at 09:22:21AM -0700, Lucas De Marchi wrote:
>> > Instead of defining all registers twice, define just a PCH_GPIO_BASE
>> > that has the same address as PCH_GPIO_A and use that to calculate all
>> > the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing
>> > the same thing.
>> >
>> > This also rewrites the GMBUS[05] registers since they depend on
>> > gpio_mmio_base.
>> >
>> > v2: Fix GMBUS registers to be relative to gpio base; create GPIO()
>> > macro to return a particular gpio address and move the enum out of
>> > i915_reg.h (suggested by Jani)
>> >
>> > Signed-off-by: Lucas De Marchi
>> > ---
>> > drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
>> > drivers/gpu/drm/i915/i915_reg.h | 53 +++--
>> > drivers/gpu/drm/i915/intel_drv.h| 16 +
>> > drivers/gpu/drm/i915/intel_i2c.c| 16 -
>> > 4 files changed, 52 insertions(+), 35 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
>> > b/drivers/gpu/drm/i915/gvt/handlers.c
>> > index e39492aaff6c..e25a74fe753b 100644
>> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
>> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>> > @@ -2084,7 +2084,7 @@ static int init_generic_mmio_info(struct intel_gvt
>> > *gvt)
>> >
>> > MMIO_F(PCH_GMBUS0, 4 * 4, 0, 0, 0, D_ALL, gmbus_mmio_read,
>> > gmbus_mmio_write);
>> > - MMIO_F(PCH_GPIOA, 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
>> > + MMIO_F(_MMIO(PCH_GPIO_BASE), 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL,
>> > NULL);
>> > MMIO_F(_MMIO(0xe4f00), 0x28, 0, 0, 0, D_ALL, NULL, NULL);
>> >
>> > MMIO_F(_MMIO(_PCH_DPB_AUX_CH_CTL), 6 * 4, 0, 0, 0, D_PRE_SKL, NULL,
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h
>> > index 0424e45f88db..f8f71d577613 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -3088,18 +3088,11 @@ enum i915_power_well_id {
>> > /*
>> > * GPIO regs
>> > */
>> > -#define GPIOA _MMIO(0x5010)
>> > -#define GPIOB _MMIO(0x5014)
>> > -#define GPIOC _MMIO(0x5018)
>> > -#define GPIOD _MMIO(0x501c)
>> > -#define GPIOE _MMIO(0x5020)
>> > -#define GPIOF _MMIO(0x5024)
>> > -#define GPIOG _MMIO(0x5028)
>> > -#define GPIOH _MMIO(0x502c)
>> > -#define GPIOJ _MMIO(0x5034)
>> > -#define GPIOK _MMIO(0x5038)
>> > -#define GPIOL _MMIO(0x503C)
>> > -#define GPIOM _MMIO(0x5040)
>> > +#define GPIO_OFFSET0x5010u
>> > +#define PCH_GPIO_BASE (0xcu + GPIO_OFFSET)
>> > +#define VLV_GPIO_BASE (VLV_DISPLAY_BASE + GPIO_OFFSET)
>>
>> This is a rather peculiar choice of baseline address. I'd either go with
>> 0x5000u or 0xu (which avoids the need to change all the gmbus macros).
>
> I'm all for a round 0x5 number, however it doesn't match the spec.
> I don't understand how 0x0 would make sense here. Are you suggesting to
> embed the GPIO_OFFSET into the GPIO macro and get rid of the GPIO_BASE?
>
> #define GPIO_OFFSET 0x5010u
> /* THESE 2 BELOW COULD USE ANOTHER BETTER NAME */
> #define PCH_GPIO_BASE 0xcu
> #define VLV_GPIO_BASE VLV_DISPLAY_BASE
> #define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 0x5010 + 4 * (gpio))
Yeah that's what I had in mind.
> That would make sense, but I don't think it's a better option due to
> GPIO_BASE now and mmio_gpio_base matching nothing in the spec (and the
> extra add on every gpio).
>
>> Only needs a slight adjustment to your GPIO macro, but avoids the rather
>> onerous - GPIO_OFFSET + GMBUS_OFFSET you have below. That one kinda
>> indicates your offset is all confused.
>
> well.. this is only because there I'm actually interested in the vlv
> display / pch offset. The subtraction is done so I don't have to store
> a gmbus_mmio_base and can rather work it out from the gpio one.
If you want to stick with your approach, at least extract the common
->gpio_mmio_base - GPIO_OFFSET + GMBUS_OFFSET computation into a
GMBUS_BASE(dev) macro or something like that. I think that would be a
good solution too.
Either approach has my a-b (I'll be on vacations next 3 weeks to
probably no r-b from me).
-Daniel
>
> Lucas De Marchi
>
>>
>> Anyway, just a drive-by comment, I was looking for some other gmbus patch.
>> -Daniel
>>
>> > +#define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 4 * (gpio))
>> > +
>> > # define GPIO_CLOCK_DIR_MASK (1 << 0)
>> > # define GPIO_CLOCK_DIR_IN (0 << 1)
>> > # define GPIO_CLOCK_DIR_OUT(1 << 1)
>> > @@ -3115,7 +3108,11 @@ enum i915_power_well_id {
>> > # define GPIO_DATA_VAL_IN (1 << 12)
>> > #
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/kvmgt: Fix compilation error
== Series Details ==
Series: drm/i915/kvmgt: Fix compilation error
URL : https://patchwork.freedesktop.org/series/46413/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4477 -> Patchwork_9635 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46413/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9635 that come from known issues:
=== IGT changes ===
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: INCOMPLETE (fdo#107139) -> DMESG-WARN (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (47 -> 41) ==
Missing(6): fi-ilk-m540 fi-bxt-dsi fi-hsw-4200u fi-byt-squawks
fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4477 -> Patchwork_9635
CI_DRM_4477: 9d5886daf8cf0b0624c1d7b04e51c55fbca1684b @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4553: 9d25d62bb6ff62694e5226b02d79075eab304402 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9635: 930c096d74cb6dcf6d82071bea66d24cf2f2667a @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
930c096d74cb drm/i915/kvmgt: Fix compilation error
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9635/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/kvmgt: Fix compilation error
Quoting Michał Winiarski (2018-07-12 16:53:30)
> gvt_pin_guest_page extracted some of the gvt_dma_map_page functionality:
> commit 79e542f5af79 ("drm/i915/kvmgt: Support setting dma map for huge pages")
>
> And yet, part of it was reintroduced in:
> commit 39b4cbadb9a9 ("drm/i915/kvmgt: Check the pfn got from vfio_pin_pages")
As you pointed out on irc, this is a silent merge conflict between
gvt-fixes and gvt-next.
I've hopefully expunged it from drm-tip (probably not in the preferred
manner) and let's hope it disappears neatly on a future backmerge.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3 0/8] Add Plane Color Properties
Hi Uma, On Tue, Jun 12, 2018 at 04:01:31AM +, Shankar, Uma wrote: > > > >-Original Message- > >From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of > >Alexandru-Cosmin Gheorghe > >Sent: Monday, June 11, 2018 3:47 PM > >To: Shankar, Uma > >Cc: dcasta...@chromium.org; intel-gfx@lists.freedesktop.org; > >emil.l.veli...@gmail.com; dri-de...@lists.freedesktop.org; Syrjala, Ville > >; n...@arm.com; Lankhorst, Maarten > > > >Subject: Re: [RFC v3 0/8] Add Plane Color Properties > > > >Hi Uma, > > > >Any progress on userspace for this? > >I was thinking on working on using this in drm_hwcomposer. > > > > Hi Alex, > Not much work has been done till now on user space side. You can go ahead > and try to enable it in drm_hwcomposer. > > Regards, > Uma Shankar > I opened a Merge request in drm_hwcomposer, if you have time please have a look and let me know what you think. [1] https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/merge_requests/25 > >Thank you, > >Alex Gheorghe > > > >On Fri, Mar 09, 2018 at 11:47:41PM +0530, Uma Shankar wrote: > >> This patch series adds properties for plane color features. It adds > >> properties for degamma used to linearize data, CSC used for gamut > >> conversion, and gamma used to again non-linearize data as per panel > >> supported color space. These can be utilize by user space to convert > >> planes from one format to another, one color space to another etc. > >> > >> Usersapce can take smart blending decisions and utilize these hardware > >> supported plane color features to get accurate color profile. The same > >> can help in consistent color quality from source to panel taking > >> advantage of advanced color features in hardware. > >> > >> These patches just add the property interfaces and enable helper > >> functions. > >> > >> This series adds Intel Gen9 specific plane gamma feature. We can build > >> up and add other platform/hardware specific implementation on top of > >> this series > >> > >> Note: This is just to get a design feedback whether these interfaces > >> look ok. Based on community feedback on interfaces, we will implement > >> IGT tests to validate plane color features. This is un-tested currently. > >> Also, userspace implementation to use these properties is currently > >> not available. > >> > >> v2: Dropped legacy gamma table for plane as suggested by Maarten. > >> Added Gen9/BDW plane gamma feature and rebase on tot. > >> > >> v3: Added a new drm_color_lut_ext structure to accommodate 32 bit > >> precision entries, pointed to by Brian, Starkey for HDR usecases. > >> Addressed Sean,Paul comments and moved plane color properties to > >> drm_plane instead of mode_config. Added property documentation as > >suggested by Daniel, Vetter. > >> Fixed a rebase fumble which occurred in v2, pointed by Emil Velikov. > >> > >> Uma Shankar (8): > >> drm: Add Enhanced Gamma LUT precision structure > >> drm: Add Plane Degamma properties > >> drm: Add Plane CTM property > >> drm: Add Plane Gamma properties > >> drm: Define helper function for plane color enabling > >> drm/i915: Enable plane color features > >> drm/i915: Implement Plane Gamma for Bdw and Gen9 platforms > >> drm/i915: Load plane color luts from atomic flip > >> > >> Documentation/gpu/drm-kms.rst | 18 > >> drivers/gpu/drm/drm_atomic.c | 30 +++ > >> drivers/gpu/drm/drm_atomic_helper.c | 12 +++ > >> drivers/gpu/drm/drm_plane.c | 131 > >++ > >> drivers/gpu/drm/i915/i915_drv.h | 5 ++ > >> drivers/gpu/drm/i915/i915_pci.c | 5 +- > >> drivers/gpu/drm/i915/i915_reg.h | 24 ++ > >> drivers/gpu/drm/i915/intel_atomic_plane.c | 4 + > >> drivers/gpu/drm/i915/intel_color.c| 80 ++ > >> drivers/gpu/drm/i915/intel_device_info.h | 5 ++ > >> drivers/gpu/drm/i915/intel_display.c | 4 + > >> drivers/gpu/drm/i915/intel_drv.h | 10 +++ > >> drivers/gpu/drm/i915/intel_sprite.c | 4 + > >> include/drm/drm_color_mgmt.h | 5 ++ > >> include/drm/drm_plane.h | 66 +++ > >> include/uapi/drm/drm_mode.h | 15 > >> 16 files changed, 417 insertions(+), 1 deletion(-) > >> > >> -- > >> 1.9.1 > >> > >> ___ > >> dri-devel mailing list > >> dri-de...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > >-- > >Cheers, > >Alex G > >___ > >dri-devel mailing list > >dri-de...@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Cheers, Alex G ___ 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: remove confusing GPIO vs PCH_GPIO
On Thu, Jul 12, 2018 at 05:04:03PM +0200, Daniel Vetter wrote:
> On Mon, Jul 09, 2018 at 09:22:21AM -0700, Lucas De Marchi wrote:
> > Instead of defining all registers twice, define just a PCH_GPIO_BASE
> > that has the same address as PCH_GPIO_A and use that to calculate all
> > the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing
> > the same thing.
> >
> > This also rewrites the GMBUS[05] registers since they depend on
> > gpio_mmio_base.
> >
> > v2: Fix GMBUS registers to be relative to gpio base; create GPIO()
> > macro to return a particular gpio address and move the enum out of
> > i915_reg.h (suggested by Jani)
> >
> > Signed-off-by: Lucas De Marchi
> > ---
> > drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 53 +++--
> > drivers/gpu/drm/i915/intel_drv.h| 16 +
> > drivers/gpu/drm/i915/intel_i2c.c| 16 -
> > 4 files changed, 52 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index e39492aaff6c..e25a74fe753b 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -2084,7 +2084,7 @@ static int init_generic_mmio_info(struct intel_gvt
> > *gvt)
> >
> > MMIO_F(PCH_GMBUS0, 4 * 4, 0, 0, 0, D_ALL, gmbus_mmio_read,
> > gmbus_mmio_write);
> > - MMIO_F(PCH_GPIOA, 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
> > + MMIO_F(_MMIO(PCH_GPIO_BASE), 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
> > MMIO_F(_MMIO(0xe4f00), 0x28, 0, 0, 0, D_ALL, NULL, NULL);
> >
> > MMIO_F(_MMIO(_PCH_DPB_AUX_CH_CTL), 6 * 4, 0, 0, 0, D_PRE_SKL, NULL,
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 0424e45f88db..f8f71d577613 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3088,18 +3088,11 @@ enum i915_power_well_id {
> > /*
> > * GPIO regs
> > */
> > -#define GPIOA _MMIO(0x5010)
> > -#define GPIOB _MMIO(0x5014)
> > -#define GPIOC _MMIO(0x5018)
> > -#define GPIOD _MMIO(0x501c)
> > -#define GPIOE _MMIO(0x5020)
> > -#define GPIOF _MMIO(0x5024)
> > -#define GPIOG _MMIO(0x5028)
> > -#define GPIOH _MMIO(0x502c)
> > -#define GPIOJ _MMIO(0x5034)
> > -#define GPIOK _MMIO(0x5038)
> > -#define GPIOL _MMIO(0x503C)
> > -#define GPIOM _MMIO(0x5040)
> > +#define GPIO_OFFSET0x5010u
> > +#define PCH_GPIO_BASE (0xcu + GPIO_OFFSET)
> > +#define VLV_GPIO_BASE (VLV_DISPLAY_BASE + GPIO_OFFSET)
>
> This is a rather peculiar choice of baseline address. I'd either go with
> 0x5000u or 0xu (which avoids the need to change all the gmbus macros).
I'm all for a round 0x5 number, however it doesn't match the spec.
I don't understand how 0x0 would make sense here. Are you suggesting to
embed the GPIO_OFFSET into the GPIO macro and get rid of the GPIO_BASE?
#define GPIO_OFFSET 0x5010u
/* THESE 2 BELOW COULD USE ANOTHER BETTER NAME */
#define PCH_GPIO_BASE 0xcu
#define VLV_GPIO_BASE VLV_DISPLAY_BASE
#define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 0x5010 + 4 * (gpio))
That would make sense, but I don't think it's a better option due to
GPIO_BASE now and mmio_gpio_base matching nothing in the spec (and the
extra add on every gpio).
> Only needs a slight adjustment to your GPIO macro, but avoids the rather
> onerous - GPIO_OFFSET + GMBUS_OFFSET you have below. That one kinda
> indicates your offset is all confused.
well.. this is only because there I'm actually interested in the vlv
display / pch offset. The subtraction is done so I don't have to store
a gmbus_mmio_base and can rather work it out from the gpio one.
Lucas De Marchi
>
> Anyway, just a drive-by comment, I was looking for some other gmbus patch.
> -Daniel
>
> > +#define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 4 * (gpio))
> > +
> > # define GPIO_CLOCK_DIR_MASK (1 << 0)
> > # define GPIO_CLOCK_DIR_IN (0 << 1)
> > # define GPIO_CLOCK_DIR_OUT(1 << 1)
> > @@ -3115,7 +3108,11 @@ enum i915_power_well_id {
> > # define GPIO_DATA_VAL_IN (1 << 12)
> > # define GPIO_DATA_PULLUP_DISABLE (1 << 13)
> >
> > -#define GMBUS0 _MMIO(dev_priv->gpio_mmio_base +
> > 0x5100) /* clock/port select */
> > +#define GMBUS_OFFSET 0x5100u
> > +
> > +/* clock/port select */
> > +#define GMBUS0 _MMIO(dev_priv->gpio_mmio_base -
> > GPIO_OFFSET \
> > + + GMBUS_OFFSET)
> > #define GMBUS_AKSV_SELECT(1 << 11)
> > #define GMBUS_RATE_100KHZ(0 << 8)
> > #define GMBUS_RATE_50KHZ (1 <<
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/kvmgt: Fix compilation error
== Series Details ==
Series: drm/i915/kvmgt: Fix compilation error
URL : https://patchwork.freedesktop.org/series/46413/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
930c096d74cb drm/i915/kvmgt: Fix compilation error
-:10: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#10:
commit 79e542f5af79 ("drm/i915/kvmgt: Support setting dma map for huge pages")
total: 0 errors, 1 warnings, 0 checks, 12 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/kvmgt: Fix compilation error
gvt_pin_guest_page extracted some of the gvt_dma_map_page functionality:
commit 79e542f5af79 ("drm/i915/kvmgt: Support setting dma map for huge pages")
And yet, part of it was reintroduced in:
commit 39b4cbadb9a9 ("drm/i915/kvmgt: Check the pfn got from vfio_pin_pages")
Causing kvmgt part to no longer build. Let's remove it.
Reported-by: Tomasz Lis
Signed-off-by: Michał Winiarski
Cc: Changbin Du
Cc: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 718ab307a500..4d2f53ae9f0f 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -185,12 +185,6 @@ static int gvt_dma_map_page(struct intel_vgpu *vgpu,
unsigned long gfn,
if (ret)
return ret;
- if (!pfn_valid(pfn)) {
- gvt_vgpu_err("pfn 0x%lx is not mem backed\n", pfn);
- vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), , 1);
- return -EINVAL;
- }
-
/* Setup DMA mapping. */
*dma_addr = dma_map_page(dev, page, 0, size, PCI_DMA_BIDIRECTIONAL);
ret = dma_mapping_error(dev, *dma_addr);
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] igt/gem_unfence_active_buffers: Check GEM before use
As we want to make the buffers active on the GPU before removing their fence, an operational GPU (not wedged!) is required. Signed-off-by: Chris Wilson --- tests/gem_unfence_active_buffers.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/gem_unfence_active_buffers.c b/tests/gem_unfence_active_buffers.c index 6df23cc53..b78fbafa7 100644 --- a/tests/gem_unfence_active_buffers.c +++ b/tests/gem_unfence_active_buffers.c @@ -74,6 +74,7 @@ igt_simple_main data[i] = i; fd = drm_open_driver(DRIVER_INTEL); + igt_require_gem(fd); bufmgr = drm_intel_bufmgr_gem_init(fd, 4096); drm_intel_bufmgr_gem_enable_reuse(bufmgr); -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] igt/gem_render_copy_redux: Check for GEM before use
As render copy wants to use the GPU, we should make sure it is not
wedged first.
Signed-off-by: Chris Wilson
---
tests/gem_render_copy_redux.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/gem_render_copy_redux.c b/tests/gem_render_copy_redux.c
index 27098ea6d..a861862d0 100644
--- a/tests/gem_render_copy_redux.c
+++ b/tests/gem_render_copy_redux.c
@@ -209,6 +209,7 @@ int main(int argc, char **argv)
igt_fixture {
data_init();
+ igt_require_gem(data.fd);
}
igt_subtest("normal") {
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] lib/gt: Check GEM is operation before injecting a hang
To inject a hang, we execute a spinning batch. This means we require GEM
to be operation before the hang, so check.
Signed-off-by: Chris Wilson
---
lib/igt_gt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 89b318ae6..86c6e1972 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -118,6 +118,7 @@ void igt_require_hang_ring(int fd, int ring)
if (!igt_check_boolean_env_var("IGT_HANG", true))
igt_skip("hang injection disabled by user");
+ igt_require_gem(fd);
gem_require_ring(fd, ring);
gem_context_require_bannable(fd);
if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false))
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2] igt/gem_gpgpu_fill: Check for GEM before use
As we need GEM and the GPU to do a GPGPU fill, we should check that it is operable before using -- skipping rather than failing when the device is wedged. Signed-off-by: Chris Wilson --- tests/gem_gpgpu_fill.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/gem_gpgpu_fill.c b/tests/gem_gpgpu_fill.c index 8ef05a3f0..dfb581652 100644 --- a/tests/gem_gpgpu_fill.c +++ b/tests/gem_gpgpu_fill.c @@ -104,6 +104,7 @@ igt_simple_main data.drm_fd = drm_open_driver_render(DRIVER_INTEL); data.devid = intel_get_drm_devid(data.drm_fd); + igt_require_gem(data.drm_fd); data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096); igt_assert(data.bufmgr); -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add Exec param to control data port coherency. (rev6)
== Series Details ==
Series: drm/i915: Add Exec param to control data port coherency. (rev6)
URL : https://patchwork.freedesktop.org/series/40181/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4476 -> Patchwork_9634 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/40181/revisions/6/mbox/
== Known issues ==
Here are the changes found in Patchwork_9634 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_module_reload@basic-no-display:
{fi-skl-iommu}: NOTRUN -> FAIL (fdo#106066) +2
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#106066 https://bugs.freedesktop.org/show_bug.cgi?id=106066
== Participating hosts (45 -> 42) ==
Additional (2): fi-byt-j1900 fi-skl-iommu
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4476 -> Patchwork_9634
CI_DRM_4476: b818fac0878147c6df45338cb515b9b7bd878b7f @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4552: 5175aff31e00e17786ebb97aaaf25ddd38b5e72e @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9634: ad63bf48d85e2a85e9542e8a9daab2db3f19488d @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
ad63bf48d85e drm/i915: Add IOCTL Param to control data port coherency.
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9634/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if
On Thu, 2018-07-12 at 07:54 -0600, Jens Axboe wrote: > > Thanks for your invaluable and useful feedback, sharing your vast > experience in patchsets with dependencies. I've probably more experience sending patchsets with dependencies across subsystems than anyone. There is no single style that works and I've probably tried them all. It's actually a somewhat significant issue within this community that could use some arbitration. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Don't read SOFT_SCRATCH(15) on MMIO error
Quoting Chris Wilson (2018-05-29 15:54:12) > Quoting Michal Wajdeczko (2018-05-28 18:16:18) > > SOFT_SCRATCH(15) is used by GuC for sending MMIO GuC events to host and > > those events are now handled by intel_guc_to_host_event_handler_mmio(). > > > > We should not try to read it on MMIO action error as 1) we may be using > > different set of registers for GuC MMIO communication, and 2) GuC may > > use CTB mechanism for sending events to host. > > Ok. > > > While here, upgrade error message to DRM_ERROR. > > Does the error help? What do you want to convey to the user? For error > handling, we want to propagate the result back anyway for the caller has > to decide what to do next. Good news! We see the error in BAT, [ 542.138479] i915: unknown parameter 'enable_guc_loading' ignored [ 542.138483] i915: unknown parameter 'enable_guc_submission' ignored [ 542.138485] Setting dangerous option enable_guc - tainting kernel [ 542.138488] Setting dangerous option live_selftests - tainting kernel [ 542.173291] [drm:intel_guc_send_mmio [i915]] *ERROR* MMIO: GuC action 0x10 failed with error -5 0xf000f000 [ 542.367055] i915: probe of :00:02.0 failed with error -25 And Michał reminded me this wasn't the first time... commit feb06c151fade9ecaa3dd410d792cce26e8b10de Author: Michał Winiarski Date: Mon Mar 19 10:53:47 2018 +0100 drm/i915/guc: Demote GuC error messages We're using those functions in selftests, and the callers are expected to do the error handling anyways. Let's demote all GuC actions and doorbell creation to DEBUG_DRIVER. So do we kindly ask Michał to resubmit his fix? -Chris ___ 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: Add Exec param to control data port coherency. (rev6)
== Series Details == Series: drm/i915: Add Exec param to control data port coherency. (rev6) URL : https://patchwork.freedesktop.org/series/40181/ State : warning == Summary == $ dim sparse origin/drm-tip Commit: drm/i915: Add IOCTL Param to control data port coherency. -drivers/gpu/drm/i915/selftests/../i915_drv.h:3652:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add Exec param to control data port coherency. (rev6)
== Series Details == Series: drm/i915: Add Exec param to control data port coherency. (rev6) URL : https://patchwork.freedesktop.org/series/40181/ State : warning == Summary == $ dim checkpatch origin/drm-tip ad63bf48d85e drm/i915: Add IOCTL Param to control data port coherency. -:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #15: coherency at data port level. Keeping the coherency at that level is disabled -:257: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #257: FILE: drivers/gpu/drm/i915/i915_gem_execbuffer.c:2366: + err = intel_lr_context_modify_data_port_coherency(eb.request, +i915_gem_context_is_data_port_coherent_requested(eb.ctx)); -:324: WARNING:LEADING_SPACE: please, no spaces at the start of a line #324: FILE: drivers/gpu/drm/i915/intel_lrc.c:314: + return ret;$ total: 0 errors, 2 warnings, 1 checks, 196 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5] drm/i915: Add IOCTL Param to control data port coherency.
The patch adds a parameter to control the data port coherency functionality on a per-context level. When the IOCTL is called, a command to switch data port coherency state is added to the ordered list. All prior requests are executed on old coherency settings, and all exec requests after the IOCTL will use new settings. Rationale: The OpenCL driver develpers requested a functionality to control cache coherency at data port level. Keeping the coherency at that level is disabled by default due to its performance costs. OpenCL driver is planning to enable it for a small subset of submissions, when such functionality is required. Below are answers to basic question explaining background of the functionality and reasoning for the proposed implementation: 1. Why do we need a coherency enable/disable switch for memory that is shared between CPU and GEN (GPU)? Memory coherency between CPU and GEN, while being a great feature that enables CL_MEM_SVM_FINE_GRAIN_BUFFER OCL capability on Intel GEN architecture, adds overhead related to tracking (snooping) memory inside different cache units (L1$, L2$, L3$, LLC$, etc.). At the same time, minority of modern OCL applications actually use CL_MEM_SVM_FINE_GRAIN_BUFFER (and hence require memory coherency between CPU and GPU). The goal of coherency enable/disable switch is to remove overhead of memory coherency when memory coherency is not needed. 2. Why do we need a global coherency switch? In order to support I/O commands from within EUs (Execution Units), Intel GEN ISA (GEN Instruction Set Assembly) contains dedicated "send" instructions. These send instructions provide several addressing models. One of these addressing models (named "stateless") provides most flexible I/O using plain virtual addresses (as opposed to buffer_handle+offset models). This "stateless" model is similar to regular memory load/store operations available on typical CPUs. Since this model provides I/O using arbitrary virtual addresses, it enables algorithmic designs that are based on pointer-to-pointer (e.g. buffer of pointers) concepts. For instance, it allows creating tree-like data structures such as: | NODE1 | | uint64_t data | +| | NODE* | NODE*| ++---+ / \ /\ | NODE2 || NODE3 | | uint64_t data || uint64_t data | +|+| | NODE* | NODE*|| NODE* | NODE*| ++---+++---+ Please note that pointers inside such structures can point to memory locations in different OCL allocations - e.g. NODE1 and NODE2 can reside in one OCL allocation while NODE3 resides in a completely separate OCL allocation. Additionally, such pointers can be shared with CPU (i.e. using SVM - Shared Virtual Memory feature). Using pointers from different allocations doesn't affect the stateless addressing model which even allows scattered reading from different allocations at the same time (i.e. by utilizing SIMD-nature of send instructions). When it comes to coherency programming, send instructions in stateless model can be encoded (at ISA level) to either use or disable coherency. However, for generic OCL applications (such as example with tree-like data structure), OCL compiler is not able to determine origin of memory pointed to by an arbitrary pointer - i.e. is not able to track given pointer back to a specific allocation. As such, it's not able to decide whether coherency is needed or not for specific pointer (or for specific I/O instruction). As a result, compiler encodes all stateless sends as coherent (doing otherwise would lead to functional issues resulting from data corruption). Please note that it would be possible to workaround this (e.g. based on allocations map and pointer bounds checking prior to each I/O instruction) but the performance cost of such workaround would be many times greater than the cost of keeping coherency always enabled. As such, enabling/disabling memory coherency at GEN ISA level is not feasible and alternative method is needed. Such alternative solution is to have a global coherency switch that allows disabling coherency for single (though entire) GPU submission. This is beneficial because this way we: * can enable (and pay for) coherency only in submissions that actually need coherency (submissions that use CL_MEM_SVM_FINE_GRAIN_BUFFER resources) * don't care about coherency at GEN ISA granularity (no performance impact) 3. Will coherency switch be used frequently? There are scenarios that will require frequent toggling of the coherency switch. E.g. an application has two OCL compute kernels: kern_master and kern_worker. kern_master uses, concurrently with CPU, some fine
Re: [Intel-gfx] [PATCH 1/2] drm/i915: remove confusing GPIO vs PCH_GPIO
On Mon, Jul 09, 2018 at 09:22:21AM -0700, Lucas De Marchi wrote:
> Instead of defining all registers twice, define just a PCH_GPIO_BASE
> that has the same address as PCH_GPIO_A and use that to calculate all
> the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing
> the same thing.
>
> This also rewrites the GMBUS[05] registers since they depend on
> gpio_mmio_base.
>
> v2: Fix GMBUS registers to be relative to gpio base; create GPIO()
> macro to return a particular gpio address and move the enum out of
> i915_reg.h (suggested by Jani)
>
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/gvt/handlers.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 53 +++--
> drivers/gpu/drm/i915/intel_drv.h| 16 +
> drivers/gpu/drm/i915/intel_i2c.c| 16 -
> 4 files changed, 52 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index e39492aaff6c..e25a74fe753b 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2084,7 +2084,7 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
>
> MMIO_F(PCH_GMBUS0, 4 * 4, 0, 0, 0, D_ALL, gmbus_mmio_read,
> gmbus_mmio_write);
> - MMIO_F(PCH_GPIOA, 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
> + MMIO_F(_MMIO(PCH_GPIO_BASE), 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL);
> MMIO_F(_MMIO(0xe4f00), 0x28, 0, 0, 0, D_ALL, NULL, NULL);
>
> MMIO_F(_MMIO(_PCH_DPB_AUX_CH_CTL), 6 * 4, 0, 0, 0, D_PRE_SKL, NULL,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0424e45f88db..f8f71d577613 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3088,18 +3088,11 @@ enum i915_power_well_id {
> /*
> * GPIO regs
> */
> -#define GPIOA_MMIO(0x5010)
> -#define GPIOB_MMIO(0x5014)
> -#define GPIOC_MMIO(0x5018)
> -#define GPIOD_MMIO(0x501c)
> -#define GPIOE_MMIO(0x5020)
> -#define GPIOF_MMIO(0x5024)
> -#define GPIOG_MMIO(0x5028)
> -#define GPIOH_MMIO(0x502c)
> -#define GPIOJ_MMIO(0x5034)
> -#define GPIOK_MMIO(0x5038)
> -#define GPIOL_MMIO(0x503C)
> -#define GPIOM_MMIO(0x5040)
> +#define GPIO_OFFSET 0x5010u
> +#define PCH_GPIO_BASE(0xcu + GPIO_OFFSET)
> +#define VLV_GPIO_BASE(VLV_DISPLAY_BASE + GPIO_OFFSET)
This is a rather peculiar choice of baseline address. I'd either go with
0x5000u or 0xu (which avoids the need to change all the gmbus macros).
Only needs a slight adjustment to your GPIO macro, but avoids the rather
onerous - GPIO_OFFSET + GMBUS_OFFSET you have below. That one kinda
indicates your offset is all confused.
Anyway, just a drive-by comment, I was looking for some other gmbus patch.
-Daniel
> +#define GPIO(gpio) _MMIO(dev_priv->gpio_mmio_base + 4 * (gpio))
> +
> # define GPIO_CLOCK_DIR_MASK (1 << 0)
> # define GPIO_CLOCK_DIR_IN (0 << 1)
> # define GPIO_CLOCK_DIR_OUT (1 << 1)
> @@ -3115,7 +3108,11 @@ enum i915_power_well_id {
> # define GPIO_DATA_VAL_IN(1 << 12)
> # define GPIO_DATA_PULLUP_DISABLE(1 << 13)
>
> -#define GMBUS0 _MMIO(dev_priv->gpio_mmio_base +
> 0x5100) /* clock/port select */
> +#define GMBUS_OFFSET 0x5100u
> +
> +/* clock/port select */
> +#define GMBUS0 _MMIO(dev_priv->gpio_mmio_base -
> GPIO_OFFSET \
> + + GMBUS_OFFSET)
> #define GMBUS_AKSV_SELECT (1 << 11)
> #define GMBUS_RATE_100KHZ (0 << 8)
> #define GMBUS_RATE_50KHZ (1 << 8)
> @@ -3141,7 +3138,10 @@ enum i915_power_well_id {
> #define GMBUS_PIN_12_TC4_ICP 12
>
> #define GMBUS_NUM_PINS 13 /* including 0 */
> -#define GMBUS1 _MMIO(dev_priv->gpio_mmio_base +
> 0x5104) /* command/status */
> +
> +/* command/status */
> +#define GMBUS1 _MMIO(dev_priv->gpio_mmio_base -
> GPIO_OFFSET \
> + + GMBUS_OFFSET + 0x4)
> #define GMBUS_SW_CLR_INT (1 << 31)
> #define GMBUS_SW_RDY (1 << 30)
> #define GMBUS_ENT (1 << 29) /* enable timeout */
> @@ -3155,7 +3155,10 @@ enum i915_power_well_id {
> #define GMBUS_SLAVE_ADDR_SHIFT 1
> #define GMBUS_SLAVE_READ (1 << 0)
> #define GMBUS_SLAVE_WRITE (0 << 0)
> -#define GMBUS2 _MMIO(dev_priv->gpio_mmio_base +
> 0x5108) /* status */
> +
> +/* status */
> +#define GMBUS2 _MMIO(dev_priv->gpio_mmio_base -
> GPIO_OFFSET \
> + + GMBUS_OFFSET + 0x8)
> #define GMBUS_INUSE(1
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests
On Thu, Jul 12, 2018 at 03:26:41PM +0100, Chris Wilson wrote: > Quoting Jakub Bartmiński (2018-07-12 15:07:18) > > Some functions used within mock selftests may expect platform-dependent > > automatic modparams parameters to have already been resolved, resulting > > in failed assertions. > > Backing up the modparams before mock selftests and manually setting > > offending parameters inside the affected selftests should fix the issue. > > I think hiding under a rock until it's weened off the modparam abuse and > can use a mock device is my favourite strategy here. I think for reasons we want a local copy of the modparams in i915_dev. Then we could sanitize/mock them at well in per-instances mocks even. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ 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: Keep local modparams copy for mock selftests
== Series Details ==
Series: series starting with [1/2] drm/i915: Keep local modparams copy for mock
selftests
URL : https://patchwork.freedesktop.org/series/46398/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9633 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46398/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9633:
=== IGT changes ===
Possible regressions
igt@core_auth@basic-auth:
{fi-skl-iommu}: PASS -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_9633 that come from known issues:
=== IGT changes ===
Issues hit
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
== Participating hosts (45 -> 42) ==
Additional (1): fi-elk-e7500
Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4475 -> Patchwork_9633
CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9633: 42e5990d61ded7af327247b74976c52a206e12d0 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
42e5990d61de HAX enable GuC for CI
ce9387fc7344 drm/i915: Keep local modparams copy for mock selftests
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9633/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 13/40] drm/i915: Implement HDCP2.2 Enable and Disable
On Tuesday 10 July 2018 02:18 AM, Sean Paul wrote:
On Wed, Jun 27, 2018 at 02:10:02PM +0530, Ramalingam C wrote:
Implements a sequence of enabling and disabling the HDCP2.2
(auth and encryption).
This is really hard to review, since all I see are stubs. I'd much rather have
each patch do something useful, instead of just call stubs. That said, I don't
have a vested interest in HDCP2.2 on intel, so if others are fine with it, I am
too.
Sean,
Just to avoid the so lengthy patches, I have split the changes in
logical patches.
Looks like patches 11, 12, 13 and 14 are not so appealing. Merged these
patches together.
Hope now the series looks more appealing.
Please have a look at the upcoming series version too. Thanks a lot again.
-Ram
Sean
v2:
Rebased.
v3:
No Changes.
v4:
No Changes.
v5:
Rebased as part of the patch reordering.
HDCP2 encryption status is tracked.
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/intel_hdcp.c | 105 +-
1 file changed, 104 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index 34bafc2025f7..f72684488bc7 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -994,14 +994,117 @@ int intel_hdcp_check_link(struct intel_connector
*connector)
return ret;
}
+static int hdcp2_close_mei_session(struct intel_connector *connector)
+{
+ struct mei_hdcp_data *data = >hdcp.mei_data;
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct i915_hdcp_component *comp = dev_priv->hdcp_comp;
+ int ret;
+
+ if (!comp)
+ return -EINVAL;
+
+ mutex_lock(>mutex);
+ if (!comp->ops || !comp->mei_cldev || data->port == INVALID_PORT) {
+ mutex_unlock(>mutex);
+ return -EINVAL;
+ }
+ ret = comp->ops->close_hdcp_session(comp->mei_cldev, data);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
+static int hdcp2_deauthenticate_port(struct intel_connector *connector)
+{
+ return hdcp2_close_mei_session(connector);
+}
+
+static int hdcp2_authenticate_sink(struct intel_connector *connector)
+{
+ return 0;
+}
+
+static int hdcp2_enable_encryption(struct intel_connector *connector)
+{
+ return 0;
+}
+
+static int hdcp2_disable_encryption(struct intel_connector *connector)
+{
+ return 0;
+}
+
+static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector)
+{
+ int ret, i, tries = 3;
+
+ for (i = 0; i < tries; i++) {
+ ret = hdcp2_authenticate_sink(connector);
+ if (!ret)
+ break;
+
+ /* Clearing the mei hdcp session */
+ hdcp2_deauthenticate_port(connector);
+ DRM_DEBUG_KMS("HDCP2.2 Auth %d of %d Failed.(%d)\n",
+ i + 1, tries, ret);
+ }
+
+ if (i != tries) {
+ /*
+* Ensuring the required 200mSec min time interval between
+* Session Key Exchange and encryption.
+*/
+ msleep(HDCP_2_2_DELAY_BEFORE_ENCRYPTION_EN);
+ ret = hdcp2_enable_encryption(connector);
+ if (ret < 0) {
+ DRM_DEBUG_KMS("Encryption Enable Failed.(%d)\n", ret);
+ hdcp2_deauthenticate_port(connector);
+ }
+ }
+
+ return ret;
+}
+
static int _intel_hdcp2_enable(struct intel_connector *connector)
{
+ struct intel_hdcp *hdcp = >hdcp;
+ int ret;
+
+ DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being enabled. Type: %d\n",
+ connector->base.name, connector->base.base.id,
+ hdcp->content_type);
+
+ ret = hdcp2_authenticate_and_encrypt(connector);
+ if (ret) {
+ DRM_ERROR("HDCP2 Type%d Enabling Failed. (%d)\n",
+ hdcp->content_type, ret);
+ return ret;
+ }
+
+ DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is enabled. Type %d\n",
+ connector->base.name, connector->base.base.id,
+ hdcp->content_type);
+
+ hdcp->hdcp2_in_use = true;
+ hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
+ schedule_work(>hdcp_prop_work);
return 0;
}
static int _intel_hdcp2_disable(struct intel_connector *connector)
{
- return 0;
+ int ret;
+
+ DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
+ connector->base.name, connector->base.base.id);
+
+ ret = hdcp2_disable_encryption(connector);
+
+ hdcp2_deauthenticate_port(connector);
+ connector->hdcp.hdcp2_in_use = false;
+
+ return ret;
}
static int i915_hdcp_component_master_bind(struct device *dev)
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip cleaning up the doorbells on error-before-allocate
Quoting Michał Winiarski (2018-07-12 14:12:48) > On Thu, Jul 12, 2018 at 11:58:30AM +0100, Chris Wilson wrote: > > If we fail the module load, we may try and cleanup before we even > > allocate the GuC clients. KISS in order to try and re-enable > > drv_module_reload for BAT. > > > > Testcase: igt/drv_module_reload/basic-reload-inject > > Signed-off-by: Chris Wilson > > Cc: Michał Winiarski > > Cc: Michal Wajdeczko > > Reviewed-by: Michał Winiarski > > We want to fix the cleanup paths eventually though. Yes, this is just band aid so we can reenable the test to be sure we don't break the module unload as we do so. Pushed along with the other simple cleanups for drv_module_reload, thanks for the review. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 06/40] drm/i915: Define HDCP2.2 related variables
On Tuesday 10 July 2018 02:01 AM, Sean Paul wrote:
On Wed, Jun 27, 2018 at 02:09:55PM +0530, Ramalingam C wrote:
For upcoming implementation of HDCP2.2 in I915, important variable
required for HDCP2.2 are defined.
Please just introduce them when you use them. I can't provide useful review on
this patch unless I can see how the variables are used. This will also reduce
the series size, which is an added bonus for reviewers :-)
Squashed these change into the subsequent patches.
Ram.
Sean
HDCP_shim is extended to support encoder specific HDCP2.2 flows.
v2:
1.4 shim is extended to support hdcp2.2. [Sean Paul]
platform's/panel's hdcp ver capability is removed. [Sean Paul]
mei references in i915_private are moved to later patches. [Chris Wilson]
v3:
mei_cl_device ref is moved into intel_hdcp
v4:
Extra * in comment is removed [Uma]
v5:
No Change.
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/intel_drv.h | 61
1 file changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eb480574a92e..b615ea4a44c3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include "i915_drv.h"
#include
@@ -375,6 +376,32 @@ struct intel_hdcp_shim {
/* Detects panel's hdcp capability. This is optional for HDMI. */
int (*hdcp_capable)(struct intel_digital_port *intel_dig_port,
bool *hdcp_capable);
+
+ /* Write HDCP2.2 messages */
+ int (*write_2_2_msg)(struct intel_digital_port *intel_dig_port,
+void *buf, size_t size);
+
+ /* Read HDCP2.2 messages */
+ int (*read_2_2_msg)(struct intel_digital_port *intel_dig_port,
+ uint8_t msg_id, void *buf, size_t size);
+
+ /*
+* Implementation of DP HDCP2.2 Errata for the communication of stream
+* type to Receivers. In DP HDCP2.2 Stream type is one of the input to
+* the HDCP2.2 Chiper for En/De-Cryption. Not applicable for HDMI.
+*/
+ int (*config_stream_type)(struct intel_digital_port *intel_dig_port,
+ void *buf, size_t size);
+
+ /* HDCP2.2 Link Integrity Check */
+ int (*check_2_2_link)(struct intel_digital_port *intel_dig_port);
+
+ /* Detects whether Panel is HDCP2.2 capable */
+ int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port,
+ bool *capable);
+
+ /* Detects the HDCP protocol(DP/HDMI) required on the port */
+ enum hdcp_protocol (*hdcp_protocol)(void);
};
struct intel_hdcp {
@@ -384,6 +411,40 @@ struct intel_hdcp {
uint64_t hdcp_value;
struct delayed_work hdcp_check_work;
struct work_struct hdcp_prop_work;
+
+ /* HDCP2.2 related definitions */
+ bool hdcp2_supported;
+
+ /*
+* Content Stream Type defined by content owner. TYPE0(0x0) content can
+* flow in the link protected by HDCP2.2 or HDCP1.4, where as TYPE1(0x1)
+* content can flow only through a link protected by HDCP2.2.
+*/
+ u8 content_type;
+
+ bool is_paired;
+ bool is_repeater;
+
+ /*
+* Count of ReceiverID_List received. Initialized to 0 at AKE_INIT.
+* Incremented after processing the RepeaterAuth_Send_ReceiverID_List.
+* When it rolls over re-auth has to be triggered.
+*/
+ uint32_t seq_num_v;
+
+ /*
+* Count of RepeaterAuth_Stream_Manage msg propagated.
+* Initialized to 0 on AKE_INIT. Incremented after every successful
+* transmission of RepeaterAuth_Stream_Manage message. When it rolls
+* over re-Auth has to be triggered.
+*/
+ uint32_t seq_num_m;
+
+ /* mei interface related information */
+ struct mei_cl_device *cldev;
+ struct mei_hdcp_data mei_data;
+
+ struct delayed_work hdcp2_check_work;
};
struct intel_connector {
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Tidy error handling in i915_gem_init_hw
Quoting Michał Winiarski (2018-07-12 13:48:10)
> Let's reorder things so that we can do onion teardown rather than double
> goto.
>
> References: b96f6ebfd024 ("drm/i915: Correctly handle error path in
> i915_gem_init_hw")
> Signed-off-by: Michał Winiarski
> Cc: Michal Wajdeczko
> Cc: Sagar Arun Kamble
Thanks for the patch, pushed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 12/40] drm/i915: Enable HDCP1.4 incase of HDCP2.2 failure
On Tuesday 10 July 2018 02:14 AM, Sean Paul wrote:
On Wed, Jun 27, 2018 at 02:10:01PM +0530, Ramalingam C wrote:
When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is
enabled.
Just squash this into patch 11, no need for a separate patch.
Doing it in the next version of the series.
Ram.
v2:
Rebased.
v3:
No Changes.
v4:
Reviewed-by is collected.
v5:
No Change.
Signed-off-by: Ramalingam C
Reviewed-by: Uma Shankar
---
drivers/gpu/drm/i915/intel_hdcp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index b34e3b1587d6..34bafc2025f7 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -863,7 +863,9 @@ int intel_hdcp_enable(struct intel_connector *connector)
*/
if (intel_hdcp2_capable(connector))
ret = _intel_hdcp2_enable(connector);
- else if (intel_hdcp_capable(connector))
+
+ /* When HDCP2.2 fails, HDCP1.4 will be attempted */
+ if (ret && intel_hdcp_capable(connector))
ret = _intel_hdcp_enable(connector);
if (!ret) {
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 11/40] drm/i915: Enable superior HDCP ver that is capable
On Tuesday 10 July 2018 02:14 AM, Sean Paul wrote:
On Wed, Jun 27, 2018 at 02:10:00PM +0530, Ramalingam C wrote:
Considering that HDCP2.2 is more secure than HDCP1.4, When a setup
supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled.
v2:
Included few optimization suggestions [Chris Wilson]
Commit message is updated as per the rebased version.
v3:
No changes.
v4:
Extra comment added and Style issue fixed [Uma]
v5:
Rebased as part of patch reordering.
Flag is added for tracking hdcp2.2 encryption status.
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdcp.c | 90 +++
2 files changed, 83 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2eeb82b04953..7624388eecd5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -414,6 +414,7 @@ struct intel_hdcp {
/* HDCP2.2 related definitions */
bool hdcp2_supported;
+ bool hdcp2_in_use;
/*
* Content Stream Type defined by content owner. TYPE0(0x0) content can
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
b/drivers/gpu/drm/i915/intel_hdcp.c
index 32a1a3f39b65..b34e3b1587d6 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -21,6 +21,60 @@
(enum hdcp_physical_port)(port))
static int intel_hdcp2_init(struct intel_connector *connector);
+static int _intel_hdcp2_enable(struct intel_connector *connector);
+static int _intel_hdcp2_disable(struct intel_connector *connector);
+static
+int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port,
+ const struct intel_hdcp_shim *shim, u8 *bksv);
+static
+struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector);
Please avoid forward declarations of static functions. Just place things
appropriately in the file.
+
+static bool panel_supports_hdcp(struct intel_connector *connector)
+{
+ struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+ struct intel_hdcp *hdcp = >hdcp;
+ bool capable = false;
+ u8 bksv[5];
+
+ if (hdcp->hdcp_shim) {
This function can only be called from
intel_hdcp_enable() -> intel_hdcp_capable() -> panel_supports_hdcp()
Both of those preceding functions check if hdcp_shim is NULL.
+ if (hdcp->hdcp_shim->hdcp_capable) {
+ hdcp->hdcp_shim->hdcp_capable(intel_dig_port, );
+ } else {
+ if (!intel_hdcp_read_valid_bksv(intel_dig_port,
+ hdcp->hdcp_shim, bksv))
+ capable = true;
+ }
+ }
+
+ return capable;
+}
+
+static inline
+bool panel_supports_hdcp2(struct intel_connector *connector)
+{
+ struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+ struct intel_hdcp *hdcp = >hdcp;
+ bool capable = false;
+
+ /* Check the panel's hdcp2.2 compliance if platform supports it. */
+ if (hdcp->hdcp2_supported)
+ hdcp->hdcp_shim->hdcp_2_2_capable(intel_dig_port, );
+
+ return capable;
+}
+
+/* Is HDCP1.4 capable on Platform and Panel */
+static inline bool intel_hdcp_capable(struct intel_connector *connector)
+{
+ return (connector->hdcp.hdcp_shim && panel_supports_hdcp(connector));
As mentioned, the hdcp_shim check is already covered by the caller. The way
things are setup, the shim checks only need to exist at the entry points
(enable/disable/check_link).
+}
+
+/* Is HDCP2.2 capable on Platform and Panel */
+static inline bool intel_hdcp2_capable(struct intel_connector *connector)
+{
+ return (connector->hdcp.hdcp2_supported &&
+ panel_supports_hdcp2(connector));
+}
The panel_supports_* functions don't seem necessary, just do the work in the
intel_hdcp*_capable functions.
static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
const struct intel_hdcp_shim *shim)
@@ -796,20 +850,27 @@ int intel_hdcp_init(struct intel_connector *connector,
int intel_hdcp_enable(struct intel_connector *connector)
{
struct intel_hdcp *hdcp = >hdcp;
- int ret;
+ int ret = -EINVAL;
if (!hdcp->hdcp_shim)
return -ENOENT;
mutex_lock(>hdcp_mutex);
- ret = _intel_hdcp_enable(connector);
- if (ret)
- goto out;
+ /*
+* Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
+* is capable of HDCP2.2, it is preferred to use HDCP2.2.
+*/
+ if (intel_hdcp2_capable(connector))
+ ret = _intel_hdcp2_enable(connector);
+ else if (intel_hdcp_capable(connector))
+ ret = _intel_hdcp_enable(connector);
+
+
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Keep local modparams copy for mock selftests
Quoting Jakub Bartmiński (2018-07-12 15:07:18) > Some functions used within mock selftests may expect platform-dependent > automatic modparams parameters to have already been resolved, resulting > in failed assertions. > Backing up the modparams before mock selftests and manually setting > offending parameters inside the affected selftests should fix the issue. I think hiding under a rock until it's weened off the modparam abuse and can use a mock device is my favourite strategy here. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fixup GuC FW negative test
Quoting Chris Wilson (2018-07-12 12:20:13)
> From: Michał Winiarski
>
> Since:
> 0d4b78b3d2c0 ("drm/i915/guc: Assert we have the doorbell before setting it
> up")
>
> We have asserts in GuC doorbell related functions, which is a good thing.
> Unfortunately, we were using those to check whether GuC FW is refusing
> to allocate invalid doorbell - which makes the test fail.
> Well, it would make the test WARN, except we fumbled cleanup ordering
> and eat the BUG_ON instead.
> Let's keep the asserts and use the internal implementation in the test.
>
> Signed-off-by: Michał Winiarski
> Cc: Chris Wilson
> Cc: Michel Thierry
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915/intel_tv_get_modes: fix strncpy truncation warning
On Thu, Jul 12, 2018 at 03:55:26PM +0200, Dominique Martinet wrote: > Ville Syrjälä wrote on Thu, Jul 12, 2018: > > On Wed, Jul 11, 2018 at 09:46:15AM +0200, Dominique Martinet wrote: > > > This is effectively no-op as the next line writes a nul at the final > > > > What is "This". Please write self contained commit messages. > > This could either be 'this commit' as a whole or if you look only at the > commit message 'this strncpy fix' from the title (which is arguably the > same), and both interpretations sound fairly understandable in the > context of the title line without seeing the patch to me... Although > I'll admit this is difficult to judge of that as the author. The patch subject is not part of the commit message body though. This is made all the more clear when I'm editing the response in vim that doesn't even show the mail subject to me. Hence I'm always left in the dark by commit messages that aren't fully self contained. > > Thanksfully, the v2 of the patch didn't use this wording but while I > agree the message could be better I do not think it was horrible. > > > > > drivers/gpu/drm/i915/intel_tv.c: In function ‘intel_tv_get_modes’: > > > drivers/gpu/drm/i915/intel_tv.c:1358:3: error: ‘strncpy’ specified bound > > > 32 equals destination size [-Werror=stringop-truncation] > > >strncpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN); > > >^~ > > > cc1: all warnings being treated as errors > > > > That warning should be in the actual commit message. > > Yes and no, I gave it for referrence but when you update to gcc 8 you > will literally see it all over the place. > The words "strncpy truncation warning" is really precise once you've > seen them a few times and there are litteraly hundred of these warnings > in the kernel, some have already been fixed taking a glance at the git > log, some with and without the warning message. > I don't think it's worth polluting the git log with this many > warnings... Which leads to... I disagree. Without knowing what exactly is fixed how can you judge whether the patch even makes sense? And later you may get another report of the same warning and then you would want to look through the git log to see if there's a patch that already fixed it. Quite hard to do without the exact warning in the log. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] HAX enable GuC for CI
From: Michal Wajdeczko Signed-off-by: Michal Wajdeczko --- drivers/gpu/drm/i915/i915_params.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index aebe0469ddaa..3e4e128237ac 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -47,7 +47,7 @@ struct drm_printer; param(int, disable_power_well, -1) \ param(int, enable_ips, 1) \ param(int, invert_brightness, 0) \ - param(int, enable_guc, 0) \ + param(int, enable_guc, -1) \ param(int, guc_log_level, -1) \ param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ -- 2.17.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: Keep local modparams copy for mock selftests
Some functions used within mock selftests may expect platform-dependent
automatic modparams parameters to have already been resolved, resulting
in failed assertions.
Backing up the modparams before mock selftests and manually setting
offending parameters inside the affected selftests should fix the issue.
Signed-off-by: Jakub Bartmiński
Cc: Chris Wilson
Cc: Michał Winiarski
---
drivers/gpu/drm/i915/i915_pci.c | 10 ++
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 6 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 55543f1b0236..9e41851f5ca2 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -733,11 +733,21 @@ static int __init i915_init(void)
{
bool use_kms = true;
int err;
+ struct i915_params *backup_modparams;
+
+ backup_modparams = kmalloc(sizeof(*backup_modparams), GFP_KERNEL);
+ if (!backup_modparams)
+ return -ENOMEM;
+ memcpy(backup_modparams, _modparams, sizeof(*backup_modparams));
err = i915_mock_selftests();
if (err)
return err > 0 ? 0 : err;
+ /* Revert any modparams modifications made inside mock selftests */
+ memcpy(_modparams, backup_modparams, sizeof(*backup_modparams));
+ kfree(backup_modparams);
+
/*
* Enable KMS by default, unless explicitly overriden by
* either the i915.modeset prarameter or by the
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index ab2590242033..de6aad832ac9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -597,6 +597,12 @@ int i915_gem_context_mock_selftests(void)
if (!i915)
return -ENOMEM;
+ /*
+* Platform defaults have not been resolved yet, so we need to prevent
+* assertion failure on an unresolved enable_guc.
+*/
+ i915_modparams.enable_guc = 0;
+
err = i915_subtests(tests, i915);
drm_dev_put(>drm);
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Tidy error handling in i915_gem_init_hw
== Series Details == Series: drm/i915: Tidy error handling in i915_gem_init_hw URL : https://patchwork.freedesktop.org/series/46393/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9632 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46393/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9632 that come from known issues: === IGT changes === Issues hit igt@kms_frontbuffer_tracking@basic: fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#106103, fdo#102614) igt@prime_vgem@basic-fence-flip: fi-ilk-650: PASS -> FAIL (fdo#104008) fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103 == Participating hosts (45 -> 42) == Additional (1): fi-elk-e7500 Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u == Build changes == * Linux: CI_DRM_4475 -> Patchwork_9632 CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9632: 5a23ed1e4a1a2f233c8ca388a6adc96a66968c71 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 5a23ed1e4a1a drm/i915: Tidy error handling in i915_gem_init_hw == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9632/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/12] blk: use for_each_if
On 7/12/18 12:45 AM, Joe Perches wrote:
> On Wed, 2018-07-11 at 20:50 +0200, Daniel Vetter wrote:
>> On Wed, Jul 11, 2018 at 8:30 PM, Jens Axboe wrote:
>>> On 7/11/18 10:45 AM, Tejun Heo wrote:
On Wed, Jul 11, 2018 at 09:40:58AM -0700, Tejun Heo wrote:
> On Mon, Jul 09, 2018 at 10:36:40AM +0200, Daniel Vetter wrote:
>> Makes the macros resilient against if {} else {} blocks right
>> afterwards.
>>
>> Signed-off-by: Daniel Vetter
>> Cc: Tejun Heo
>> Cc: Jens Axboe
>> Cc: Shaohua Li
>> Cc: Kate Stewart
>> Cc: Greg Kroah-Hartman
>> Cc: Joseph Qi
>> Cc: Daniel Vetter
>> Cc: Arnd Bergmann
>
> Acked-by: Tejun Heo
>
> Jens, it'd probably be best to route this through block tree.
Oops, this requires an earlier patch to move the for_each_if def to a
common header and should be routed together.
>>>
>>> Yeah, this is a problem with the submission.
>>>
>>> Always (ALWAYS) CC folks on at least the cover letter and generic
>>> earlier patches. Getting just one patch sent like this is mostly
>>> useless, and causes more harm than good.
>>
>> Ime sending a patch with more than 20 or so recipients means it gets
>> stuck everywhere in moderation queues. Or outright spam filters. I
>> thought the correct way to do this is to cc: mailing lists (lkml has
>> them all), but apparently that's not how it's done. Despite that all
>> the patch series I get never have the cover letter addressed to me
>> either.
>>
>> So what's the magic way to make this possible?
>
> Jens' advice is crap.
>
> There is no generic way to make this possible.
Nobody claimed there was. And the advice is perfectly fine,
sending out patches to folks that have hidden dependencies on other
patches is a no-go.
> BCC's don't work, series that touch multiple subsystems
> get rejected when the recipient list is too large.
>
> I think you did it correctly.
Clearly that's not the case, regardless of what you think.
Thanks for your invaluable and useful feedback, sharing your vast
experience in patchsets with dependencies.
--
Jens Axboe
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Tidy error handling in i915_gem_init_hw
== Series Details ==
Series: drm/i915: Tidy error handling in i915_gem_init_hw
URL : https://patchwork.freedesktop.org/series/46393/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
5a23ed1e4a1a drm/i915: Tidy error handling in i915_gem_init_hw
-:12: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#12:
References: b96f6ebfd024 ("drm/i915: Correctly handle error path in
i915_gem_init_hw")
-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit b96f6ebfd024 ("drm/i915:
Correctly handle error path in i915_gem_init_hw")'
#12:
References: b96f6ebfd024 ("drm/i915: Correctly handle error path in
i915_gem_init_hw")
total: 1 errors, 1 warnings, 0 checks, 20 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove unused engine->cleanup
== Series Details ==
Series: drm/i915: Remove unused engine->cleanup
URL : https://patchwork.freedesktop.org/series/46392/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9631 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46392/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9631 that come from known issues:
=== IGT changes ===
Issues hit
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: PASS -> DMESG-WARN (fdo#107139, fdo#105128)
igt@kms_chamelium@dp-edid-read:
fi-kbl-7500u: PASS -> FAIL (fdo#103841)
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: DMESG-WARN (fdo#107139) -> INCOMPLETE (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (45 -> 42) ==
Additional (1): fi-elk-e7500
Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4475 -> Patchwork_9631
CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9631: 0e4085f67331144928a13c2385b3512f5b0eb562 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
0e4085f67331 drm/i915: Remove unused engine->cleanup
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9631/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Bump priority of clean up work
== Series Details == Series: drm/i915: Bump priority of clean up work URL : https://patchwork.freedesktop.org/series/46390/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9630 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/46390/revisions/1/mbox/ == Changes == No changes found == Participating hosts (45 -> 42) == Additional (1): fi-elk-e7500 Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u == Build changes == * Linux: CI_DRM_4475 -> Patchwork_9630 CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9630: af270619ba234c10def0ebe5bfdf82bffc48c105 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == af270619ba23 drm/i915: Bump priority of clean up work == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9630/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip cleaning up the doorbells on error-before-allocate
On Thu, Jul 12, 2018 at 11:58:30AM +0100, Chris Wilson wrote:
> If we fail the module load, we may try and cleanup before we even
> allocate the GuC clients. KISS in order to try and re-enable
> drv_module_reload for BAT.
>
> Testcase: igt/drv_module_reload/basic-reload-inject
> Signed-off-by: Chris Wilson
> Cc: Michał Winiarski
> Cc: Michal Wajdeczko
Reviewed-by: Michał Winiarski
We want to fix the cleanup paths eventually though.
-Michał
> ---
> drivers/gpu/drm/i915/intel_guc_submission.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c
> b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 3952656f4c9a..cd51be8ff025 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -910,8 +910,12 @@ static void guc_clients_doorbell_fini(struct intel_guc
> *guc)
> __update_doorbell_desc(guc->preempt_client,
> GUC_DOORBELL_INVALID);
> }
> - __destroy_doorbell(guc->execbuf_client);
> - __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
> +
> + if (guc->execbuf_client) {
> + __destroy_doorbell(guc->execbuf_client);
> + __update_doorbell_desc(guc->execbuf_client,
> +GUC_DOORBELL_INVALID);
> + }
> }
>
> /**
> --
> 2.18.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/selftests: Fixup GuC FW negative test
== Series Details ==
Series: drm/i915/selftests: Fixup GuC FW negative test
URL : https://patchwork.freedesktop.org/series/46388/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9629 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46388/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9629 that come from known issues:
=== IGT changes ===
Issues hit
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: PASS -> DMESG-WARN (fdo#107139, fdo#105128)
igt@prime_vgem@basic-fence-flip:
fi-byt-n2820: PASS -> FAIL (fdo#104008)
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: DMESG-WARN (fdo#107139) -> INCOMPLETE (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (45 -> 42) ==
Additional (1): fi-elk-e7500
Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4475 -> Patchwork_9629
CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9629: f9b5ed208b2f6272019f8a55b98a00e2e93e4584 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
f9b5ed208b2f drm/i915/selftests: Fixup GuC FW negative test
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9629/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 0/6] Add ChromeOS EC CEC Support
On 12/07/18 14:42, Neil Armstrong wrote: > Hi Lee, > > On 12/07/2018 14:26, Lee Jones wrote: >> On Wed, 04 Jul 2018, Neil Armstrong wrote: >> >>> Hi All, >>> >>> The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support >>> through it's Embedded Controller, to enable the Linux CEC Core to >>> communicate >>> with it and get the CEC Physical Address from the correct HDMI Connector, >>> the >>> following must be added/changed: >>> - Add the CEC sub-device registration in the ChromeOS EC MFD Driver >>> - Add the CEC related commands and events definitions into the EC MFD driver >>> - Add a way to get a CEC notifier with it's (optional) connector name >>> - Add the CEC notifier to the i915 HDMI driver >>> - Add the proper ChromeOS EC CEC Driver >>> >>> The CEC notifier with the connector name is the tricky point, since even on >>> Device-Tree platforms, there is no way to distinguish between multiple HDMI >>> connectors from the same DRM driver. The solution I implemented is pretty >>> simple and only adds an optional connector name to eventually distinguish >>> an HDMI connector notifier from another if they share the same device. >>> >>> Feel free to comment this patchset ! >>> >>> Changes since v7: >>> - rebased on v4.18-rc1 >>> - Fixed whitespace issues on patch 3 >>> - Added Lee's tags >>> >>> Changes since v6: >>> - Added stable identifier comment in intel_display.h >>> - Renamed to cec_notifier in intel_hdmi.c/intel_drv.h >>> - Added Acked-by/Reviewed-By tags >>> >>> Changes since v5: >>> - Small fixups on include/linux/mfd/cros_ec_commands.h >>> - Fixed on cros-ec-cec driver accordingly >>> - Added Reviewed-By tags >>> >>> Changes since v4: >>> - Split patch 3 to move the mkbp event size change into a separate patch >>> >>> Changes since v3 (incorrectly reported as v2): >>> - Renamed "Chrome OS" to "ChromeOS" >>> - Updated cros_ec_commands.h new structs definitions to kernel doc format >>> - Added Reviewed-By tags >>> >>> Changes since v2: >>> - Add i915 port_identifier() and use this stable name as cec_notifier conn >>> name >>> - Fixed and cleaned up the CEC commands and events handling >>> - Rebased the CEC sub-device registration on top of Enric's serie >>> - Fixed comments typo on cec driver >>> - Protected the DMI match only with PCI and DMI Kconfigs >>> >>> Changes since v1: >>> - Added cec_notifier_put to intel_hdmi >>> - Fixed all small reported issues on the EC CEC driver >>> - Moved the cec_notifier_get out of the #if .. #else .. #endif >>> >>> Changes since RFC: >>> - Moved CEC sub-device registration after CEC commands and events >>> definitions patch >>> - Removed get_notifier_get_byname >>> - Added CEC_CORE select into i915 Kconfig >>> - Removed CEC driver fallback if notifier is not configured on HW, added >>> explicit warn >>> - Fixed CEC core return type on error >>> - Moved to cros-ec-cec media platform directory >>> - Use bus_find_device() to find the pci i915 device instead of >>> get_notifier_get_byname() >>> - Fix Logical Address setup >>> - Added comment about HW support >>> - Removed memset of msg structures >>> >>> Neil Armstrong (6): >>> media: cec-notifier: Get notifier by device and connector name >>> drm/i915: hdmi: add CEC notifier to intel_hdmi >>> mfd: cros-ec: Increase maximum mkbp event size >>> mfd: cros-ec: Introduce CEC commands and events definitions. >>> mfd: cros_ec_dev: Add CEC sub-device registration >>> media: platform: Add ChromeOS EC CEC driver >>> >>> drivers/gpu/drm/i915/Kconfig | 1 + >>> drivers/gpu/drm/i915/intel_display.h | 24 ++ >>> drivers/gpu/drm/i915/intel_drv.h | 2 + >>> drivers/gpu/drm/i915/intel_hdmi.c| 13 + >>> drivers/media/cec/cec-notifier.c | 11 +- >>> drivers/media/platform/Kconfig | 11 + >>> drivers/media/platform/Makefile | 2 + >>> drivers/media/platform/cros-ec-cec/Makefile | 1 + >>> drivers/media/platform/cros-ec-cec/cros-ec-cec.c | 347 >>> +++ >>> drivers/mfd/cros_ec_dev.c| 16 ++ >>> drivers/platform/chrome/cros_ec_proto.c | 40 ++- >>> include/linux/mfd/cros_ec.h | 2 +- >>> include/linux/mfd/cros_ec_commands.h | 97 +++ >>> include/media/cec-notifier.h | 27 +- >>> 14 files changed, 578 insertions(+), 16 deletions(-) >>> create mode 100644 drivers/media/platform/cros-ec-cec/Makefile >>> create mode 100644 drivers/media/platform/cros-ec-cec/cros-ec-cec.c >> >> How would you like to handle this set? >> > > Hans proposed you take all the patches throught mfd, > then drm-intel could merge your immutable branch to avoid any conflicts. > > Rodrigo Vivi gave an ack to merge it through other trees on the v6 patchset. > > Hans, should the media tree also merge the branch ? No, there is no need for that. It's fine if it all
Re: [Intel-gfx] [PATCH] drm/i915: Tidy error handling in i915_gem_init_hw
Quoting Michał Winiarski (2018-07-12 13:48:10)
> Let's reorder things so that we can do onion teardown rather than double
> goto.
>
> References: b96f6ebfd024 ("drm/i915: Correctly handle error path in
> i915_gem_init_hw")
> Signed-off-by: Michał Winiarski
> Cc: Michal Wajdeczko
> Cc: Sagar Arun Kamble
Ok. My pasta punning pedantry pales away,
Reviewed-by: Chris Wilson
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] RFC drm/i915: Mark runtime_pm as a special class of lock
On Thu, Jul 12, 2018 at 09:41:07AM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2018-07-12 09:36:33)
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 5 +
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 11 +++
> > 3 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 3eba3d1ab5b8..2e6d3259f6d0 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -2603,6 +2603,7 @@ static int intel_runtime_suspend(struct device *kdev)
> > DRM_DEBUG_KMS("Suspending device\n");
> >
> > disable_rpm_wakeref_asserts(dev_priv);
> > + lock_map_acquire(_priv->runtime_pm.lock);
> >
> > /*
> > * We are safe here against re-faults, since the fault handler takes
> > @@ -2637,11 +2638,13 @@ static int intel_runtime_suspend(struct device
> > *kdev)
> > i915_gem_init_swizzling(dev_priv);
> > i915_gem_restore_fences(dev_priv);
> >
> > + lock_map_release(_priv->runtime_pm.lock);
> > enable_rpm_wakeref_asserts(dev_priv);
> >
> > return ret;
> > }
> >
> > + lock_map_release(_priv->runtime_pm.lock);
>
> What happens if we don't release the lock here? I think that's what we
> want... While suspended we are not allowed to do any action that would
> ordinarily require a wakeref. However that scares me for being both
> incredibly broad, and that I think lockdep is process centric so doesn't
> track locks in this manner?
Lockdep requires that acquire are in the same process context. For
dependencies crossing boundaries we want a cross-release. And yes I think
a cross-release dependency between our rpm_suspend and rpm_get is required
for full anotation. But since cross-release is suffering in limbo due to
meltdown/spectre that's a way off still :-/
Also I think if this all works out we should propose it as a patch to core
rpm code (maybe once the cross-release stuff has landed too).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove unused engine->cleanup
Quoting Michał Winiarski (2018-07-12 13:44:15) > There's nothing there. Last vfunc got removed along with gen8 legacy > ringbuffer submission. Honestly it's gt.cleanup_engine that is the unwanted vfunc. engine->cleanup is the right layer, esp as we do not plan to have a single class of engine in the system. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/selftests: Fixup GuC FW negative test
== Series Details ==
Series: drm/i915/selftests: Fixup GuC FW negative test
URL : https://patchwork.freedesktop.org/series/46388/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
f9b5ed208b2f drm/i915/selftests: Fixup GuC FW negative test
-:10: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#10:
0d4b78b3d2c0 ("drm/i915/guc: Assert we have the doorbell before setting it up")
-:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit 0d4b78b3d2c0 ("drm/i915/guc:
Assert we have the doorbell before setting it up")'
#10:
0d4b78b3d2c0 ("drm/i915/guc: Assert we have the doorbell before setting it up")
total: 1 errors, 1 warnings, 0 checks, 26 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Tidy error handling in i915_gem_init_hw
Let's reorder things so that we can do onion teardown rather than double
goto.
References: b96f6ebfd024 ("drm/i915: Correctly handle error path in
i915_gem_init_hw")
Signed-off-by: Michał Winiarski
Cc: Michal Wajdeczko
Cc: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_gem.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 07a92ca61dbf..ed2be33ec58a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5313,13 +5313,17 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
ret = __i915_gem_restart_engines(dev_priv);
if (ret)
goto cleanup_uc;
-out:
+
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
- return ret;
+
+ return 0;
cleanup_uc:
intel_uc_fini_hw(dev_priv);
- goto out;
+out:
+ intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+
+ return ret;
}
static int __intel_engines_record_defaults(struct drm_i915_private *i915)
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Skip cleaning up the doorbells on error-before-allocate
== Series Details ==
Series: drm/i915/guc: Skip cleaning up the doorbells on error-before-allocate
URL : https://patchwork.freedesktop.org/series/46382/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9628 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/46382/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9628 that come from known issues:
=== IGT changes ===
Warnings
igt@gem_exec_suspend@basic-s4-devices:
{fi-kbl-8809g}: DMESG-WARN (fdo#107139) -> INCOMPLETE (fdo#107139)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
== Participating hosts (45 -> 41) ==
Additional (1): fi-elk-e7500
Missing(5): fi-ctg-p8600 fi-byt-squawks fi-ilk-m540 fi-bxt-dsi
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4475 -> Patchwork_9628
CI_DRM_4475: 1b6f049d73237a170919604538e747b0282b0109 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4551: 93cf6931b33e2c0f5b89c89b65817fe245ecc391 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9628: 77bd92a62bafa36de294911cf749fab419738e42 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
77bd92a62baf drm/i915/guc: Skip cleaning up the doorbells on
error-before-allocate
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9628/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Remove unused engine->cleanup
There's nothing there. Last vfunc got removed along with gen8 legacy
ringbuffer submission.
Signed-off-by: Michał Winiarski
Cc: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c| 3 ---
drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ---
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
3 files changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 35d37af0cb9a..0a7fefd718c4 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2341,9 +2341,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*engine)
WARN_ON((I915_READ_MODE(engine) & MODE_IDLE) == 0);
}
- if (engine->cleanup)
- engine->cleanup(engine);
-
intel_engine_cleanup_common(engine);
lrc_destroy_wa_ctx(engine);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f4bd185c9369..8137eaa146b9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1446,9 +1446,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine)
intel_ring_unpin(engine->buffer);
intel_ring_free(engine->buffer);
- if (engine->cleanup)
- engine->cleanup(engine);
-
intel_engine_cleanup_common(engine);
dev_priv->engine[engine->id] = NULL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d1eee08e5f6b..c2a9ae1fd01e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -503,7 +503,6 @@ struct intel_engine_cs {
* monotonic, even if not coherent.
*/
void(*irq_seqno_barrier)(struct intel_engine_cs *engine);
- void(*cleanup)(struct intel_engine_cs *engine);
/* GEN8 signal/wait table - never trust comments!
*signal to signal tosignal to signal to signal
to
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
