| Message ID | dbb687a04b5cb8b2c64da1a2f6b6a232340c68b9.1450089383.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 14, 2015 at 12:50:54PM +0200, Jani Nikula wrote: > In the future the VBT might not be in mailbox #4 of the ACPI OpRegion, > thus unavailable in i915_opregion, so add a separate file for the VBT. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_opregion.c | 4 +++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index a9e1f18c36d1..aef1393e707f 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1857,6 +1857,27 @@ out: > return 0; > } > > +static int i915_vbt(struct seq_file *m, void *unused) > +{ > + struct drm_info_node *node = m->private; > + struct drm_device *dev = node->minor->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_opregion *opregion = &dev_priv->opregion; > + int ret; > + > + ret = mutex_lock_interruptible(&dev->struct_mutex); The contents of opregion->vbt are not serialised by struct_mutex, right? -Chris
On Mon, 14 Dec 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Mon, Dec 14, 2015 at 12:50:54PM +0200, Jani Nikula wrote: >> In the future the VBT might not be in mailbox #4 of the ACPI OpRegion, >> thus unavailable in i915_opregion, so add a separate file for the VBT. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++++++++ >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_opregion.c | 4 +++- >> 3 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index a9e1f18c36d1..aef1393e707f 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -1857,6 +1857,27 @@ out: >> return 0; >> } >> >> +static int i915_vbt(struct seq_file *m, void *unused) >> +{ >> + struct drm_info_node *node = m->private; >> + struct drm_device *dev = node->minor->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_opregion *opregion = &dev_priv->opregion; >> + int ret; >> + >> + ret = mutex_lock_interruptible(&dev->struct_mutex); > > The contents of opregion->vbt are not serialised by struct_mutex, right? Cargo culting ftw. I figured it was maybe for suspend/resume paths, but that doesn't really make sense does it? BR, Jani.
On Mon, Dec 14, 2015 at 01:06:51PM +0200, Jani Nikula wrote: > On Mon, 14 Dec 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Mon, Dec 14, 2015 at 12:50:54PM +0200, Jani Nikula wrote: > >> In the future the VBT might not be in mailbox #4 of the ACPI OpRegion, > >> thus unavailable in i915_opregion, so add a separate file for the VBT. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++++++++ > >> drivers/gpu/drm/i915/i915_drv.h | 1 + > >> drivers/gpu/drm/i915/intel_opregion.c | 4 +++- > >> 3 files changed, 26 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >> index a9e1f18c36d1..aef1393e707f 100644 > >> --- a/drivers/gpu/drm/i915/i915_debugfs.c > >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c > >> @@ -1857,6 +1857,27 @@ out: > >> return 0; > >> } > >> > >> +static int i915_vbt(struct seq_file *m, void *unused) > >> +{ > >> + struct drm_info_node *node = m->private; > >> + struct drm_device *dev = node->minor->dev; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct intel_opregion *opregion = &dev_priv->opregion; > >> + int ret; > >> + > >> + ret = mutex_lock_interruptible(&dev->struct_mutex); > > > > The contents of opregion->vbt are not serialised by struct_mutex, right? > > Cargo culting ftw. I figured it was maybe for suspend/resume paths, but > that doesn't really make sense does it? No, the only thing that we control here are the dev_priv->opregion pointers - and they are init only. So I think we are totally safe to drop the struct_mutex. -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a9e1f18c36d1..aef1393e707f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1857,6 +1857,27 @@ out: return 0; } +static int i915_vbt(struct seq_file *m, void *unused) +{ + struct drm_info_node *node = m->private; + struct drm_device *dev = node->minor->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_opregion *opregion = &dev_priv->opregion; + int ret; + + ret = mutex_lock_interruptible(&dev->struct_mutex); + if (ret) + goto out; + + if (opregion->vbt) + seq_write(m, opregion->vbt, opregion->vbt_size); + + mutex_unlock(&dev->struct_mutex); + +out: + return 0; +} + static int i915_gem_framebuffer_info(struct seq_file *m, void *data) { struct drm_info_node *node = m->private; @@ -5325,6 +5346,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_ips_status", i915_ips_status, 0}, {"i915_sr_status", i915_sr_status, 0}, {"i915_opregion", i915_opregion, 0}, + {"i915_vbt", i915_vbt, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, {"i915_dump_lrc", i915_dump_lrc, 0}, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 10e47167c594..ca8c2a64bc6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -458,6 +458,7 @@ struct intel_opregion { u32 swsci_sbcb_sub_functions; struct opregion_asle *asle; const void *vbt; + u32 vbt_size; u32 *lid_state; struct work_struct asle_work; }; diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index f9403b1c8fdd..e89ee2383fe1 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -990,8 +990,10 @@ int intel_opregion_setup(struct drm_device *dev) const void *vbt = base + OPREGION_VBT_OFFSET; u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET; - if (intel_bios_is_valid_vbt(vbt, vbt_size)) + if (intel_bios_is_valid_vbt(vbt, vbt_size)) { opregion->vbt = vbt; + opregion->vbt_size = vbt_size; + } } return 0;
In the future the VBT might not be in mailbox #4 of the ACPI OpRegion, thus unavailable in i915_opregion, so add a separate file for the VBT. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 22 ++++++++++++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_opregion.c | 4 +++- 3 files changed, 26 insertions(+), 1 deletion(-)