Re: [Intel-gfx] [PATCH 10/11] drm/i915/debugfs: add a separate debugfs file for VBT

[Jani Nikula]

On Mon, 14 Dec 2015, Chris Wilson  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 
>> ---
>>  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 = _priv->opregion;
>> +int ret;
>> +
>> +ret = mutex_lock_interruptible(>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.



-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/11] drm/i915/debugfs: add a separate debugfs file for VBT

[Jani Nikula]

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 
---
 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 = _priv->opregion;
+   int ret;
+
+   ret = mutex_lock_interruptible(>struct_mutex);
+   if (ret)
+   goto out;
+
+   if (opregion->vbt)
+   seq_write(m, opregion->vbt, opregion->vbt_size);
+
+   mutex_unlock(>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;
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/11] drm/i915/debugfs: add a separate debugfs file for VBT

[Chris Wilson]

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 
> ---
>  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 = _priv->opregion;
> + int ret;
> +
> + ret = mutex_lock_interruptible(>struct_mutex);

The contents of opregion->vbt are not serialised by struct_mutex, right?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/11] drm/i915/debugfs: add a separate debugfs file for VBT

[Chris Wilson]

On Mon, Dec 14, 2015 at 01:06:51PM +0200, Jani Nikula wrote:
> On Mon, 14 Dec 2015, Chris Wilson  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 
> >> ---
> >>  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 = _priv->opregion;
> >> +  int ret;
> >> +
> >> +  ret = mutex_lock_interruptible(>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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx