* [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
@ 2015-02-05 11:40 Shobhit Kumar
2015-02-05 17:14 ` Ville Syrjälä
2015-02-05 22:39 ` shuang.he
0 siblings, 2 replies; 7+ messages in thread
From: Shobhit Kumar @ 2015-02-05 11:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Shobhit Kumar, Daniel Vetter
As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
port ID should be enough to identify devices unless they are MFD. The
SB_DevFn was intended to remove ambiguity in case of these MFD devices.
For non MFD devices the recommendation for the target device IP was to
ignore these fields, but not all of them followed the recommendation.
Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
It turned out that this did not follow the recommendation and expected 0
in this field.
In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
all devices except target PCI devices.
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 01d841e..731b10a 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
mutex_lock(&dev_priv->dpio_lock);
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
SB_CRRDDA_NP, addr, &val);
mutex_unlock(&dev_priv->dpio_lock);
@@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
mutex_lock(&dev_priv->dpio_lock);
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
SB_CRWRDA_NP, addr, &val);
mutex_unlock(&dev_priv->dpio_lock);
}
@@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
SB_CRRDDA_NP, reg, &val);
return val;
@@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
SB_CRWRDA_NP, reg, &val);
}
@@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
mutex_lock(&dev_priv->dpio_lock);
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
SB_CRRDDA_NP, addr, &val);
mutex_unlock(&dev_priv->dpio_lock);
@@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
SB_CRRDDA_NP, reg, &val);
return val;
}
void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
SB_CRWRDA_NP, reg, &val);
}
u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
SB_CRRDDA_NP, reg, &val);
return val;
}
void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
SB_CRWRDA_NP, reg, &val);
}
u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
SB_CRRDDA_NP, reg, &val);
return val;
}
void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
SB_CRWRDA_NP, reg, &val);
}
u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
{
u32 val = 0;
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
SB_CRRDDA_NP, reg, &val);
return val;
}
void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
{
- vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
+ vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
SB_CRWRDA_NP, reg, &val);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-05 11:40 [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers Shobhit Kumar
@ 2015-02-05 17:14 ` Ville Syrjälä
2015-02-06 8:50 ` Jani Nikula
2015-02-09 11:16 ` Shobhit Kumar
2015-02-05 22:39 ` shuang.he
1 sibling, 2 replies; 7+ messages in thread
From: Ville Syrjälä @ 2015-02-05 17:14 UTC (permalink / raw)
To: Shobhit Kumar; +Cc: Jani Nikula, Daniel Vetter, intel-gfx
On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote:
> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
> port ID should be enough to identify devices unless they are MFD. The
> SB_DevFn was intended to remove ambiguity in case of these MFD devices.
>
> For non MFD devices the recommendation for the target device IP was to
> ignore these fields, but not all of them followed the recommendation.
> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
> It turned out that this did not follow the recommendation and expected 0
> in this field.
>
> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
> all devices except target PCI devices.
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW
GPIONC seems to have become more relaxed and accepts devfn 2.0 as well.
I also tried to poke at all the other sideband units we care about and
they appear to work just as well with 2.0 and 0.0. But being consistent
seems like a good idea, so 0.0 it is for everyone.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 01d841e..731b10a 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>
> mutex_lock(&dev_priv->dpio_lock);
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
> SB_CRRDDA_NP, addr, &val);
> mutex_unlock(&dev_priv->dpio_lock);
>
> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>
> mutex_lock(&dev_priv->dpio_lock);
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
> SB_CRWRDA_NP, addr, &val);
> mutex_unlock(&dev_priv->dpio_lock);
> }
> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
> {
> u32 val = 0;
>
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
> SB_CRRDDA_NP, reg, &val);
>
> return val;
> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>
> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> {
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
> SB_CRWRDA_NP, reg, &val);
> }
>
> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>
> mutex_lock(&dev_priv->dpio_lock);
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
> SB_CRRDDA_NP, addr, &val);
> mutex_unlock(&dev_priv->dpio_lock);
>
> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
> {
> u32 val = 0;
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
> SB_CRRDDA_NP, reg, &val);
> return val;
> }
>
> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> {
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
> SB_CRWRDA_NP, reg, &val);
> }
>
> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
> {
> u32 val = 0;
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
> SB_CRRDDA_NP, reg, &val);
> return val;
> }
>
> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> {
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
> SB_CRWRDA_NP, reg, &val);
> }
>
> u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
> {
> u32 val = 0;
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
> SB_CRRDDA_NP, reg, &val);
> return val;
> }
>
> void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> {
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
> SB_CRWRDA_NP, reg, &val);
> }
>
> u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
> {
> u32 val = 0;
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
> SB_CRRDDA_NP, reg, &val);
> return val;
> }
>
> void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
> {
> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
> SB_CRWRDA_NP, reg, &val);
> }
>
> --
> 1.9.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-05 11:40 [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers Shobhit Kumar
2015-02-05 17:14 ` Ville Syrjälä
@ 2015-02-05 22:39 ` shuang.he
1 sibling, 0 replies; 7+ messages in thread
From: shuang.he @ 2015-02-05 22:39 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, shobhit.kumar
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5719
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV +1 282/283 283/283
ILK 316/319 316/319
SNB +22-2 322/346 342/346
IVB 382/384 382/384
BYT 296/296 296/296
HSW -1 425/428 424/428
BDW 318/333 318/333
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
PNV igt_gen3_render_linear_blits FAIL(3, M7)CRASH(1, M23)PASS(4, M25M23) PASS(1, M25)
SNB igt_kms_cursor_crc_cursor-size-change NSPT(2, M22M35)PASS(2, M35)DMESG_FAIL(1, M35) PASS(1, M35)
SNB igt_kms_flip_dpms-vs-vblank-race-interruptible DMESG_WARN(2, M35)PASS(3, M22M35) DMESG_WARN(1, M35)
SNB igt_kms_flip_event_leak NSPT(2, M22M35)PASS(3, M35) PASS(1, M35)
SNB igt_kms_flip_modeset-vs-vblank-race-interruptible DMESG_WARN(1, M35)PASS(2, M22M35) DMESG_WARN(1, M35)
SNB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_kms_rotation_crc_primary-rotation NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_kms_rotation_crc_sprite-rotation NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_cursor NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_cursor-dpms NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_dpms-non-lpsp NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_drm-resources-equal NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_fences NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_fences-dpms NSPT(1, M22)DMESG_WARN(1, M35)PASS(2, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-execbuf NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-mmap-cpu NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-mmap-gtt NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_gem-pread NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_i2c NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_modeset-non-lpsp NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_pci-d3-state NSPT(1, M22)PASS(3, M35) PASS(1, M35)
SNB igt_pm_rpm_rte NSPT(1, M22)PASS(3, M35) PASS(1, M35)
*HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance PASS(3, M20) DMESG_WARN(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-05 17:14 ` Ville Syrjälä
@ 2015-02-06 8:50 ` Jani Nikula
2015-02-06 10:45 ` Shobhit Kumar
2015-02-09 11:16 ` Shobhit Kumar
1 sibling, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2015-02-06 8:50 UTC (permalink / raw)
To: Ville Syrjälä, Shobhit Kumar; +Cc: Daniel Vetter, intel-gfx
On Thu, 05 Feb 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote:
>> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
>> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
>> port ID should be enough to identify devices unless they are MFD. The
>> SB_DevFn was intended to remove ambiguity in case of these MFD devices.
>>
>> For non MFD devices the recommendation for the target device IP was to
>> ignore these fields, but not all of them followed the recommendation.
>> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
>> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
>> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
>> It turned out that this did not follow the recommendation and expected 0
>> in this field.
>>
>> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
>> all devices except target PCI devices.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW
> GPIONC seems to have become more relaxed and accepts devfn 2.0 as well.
>
> I also tried to poke at all the other sideband units we care about and
> they appear to work just as well with 2.0 and 0.0. But being consistent
> seems like a good idea, so 0.0 it is for everyone.
Shobhit, do you have any idea if there are devices out there that would
really need this, i.e. should we backport this to stable kernels or is
it good enough for next?
BR,
Jani.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>> ---
>> drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
>> index 01d841e..731b10a 100644
>> --- a/drivers/gpu/drm/i915/intel_sideband.c
>> +++ b/drivers/gpu/drm/i915/intel_sideband.c
>> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>> SB_CRRDDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>>
>> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>> SB_CRWRDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>> }
>> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>>
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>> SB_CRRDDA_NP, reg, &val);
>>
>> return val;
>> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>
>> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
>> SB_CRRDDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>>
>> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> --
>> 1.9.1
>
> --
> Ville Syrjälä
> Intel OTC
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-06 8:50 ` Jani Nikula
@ 2015-02-06 10:45 ` Shobhit Kumar
0 siblings, 0 replies; 7+ messages in thread
From: Shobhit Kumar @ 2015-02-06 10:45 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä, Shobhit Kumar
Cc: Daniel Vetter, intel-gfx
On 02/06/2015 02:20 PM, Jani Nikula wrote:
> On Thu, 05 Feb 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote:
>>> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
>>> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
>>> port ID should be enough to identify devices unless they are MFD. The
>>> SB_DevFn was intended to remove ambiguity in case of these MFD devices.
>>>
>>> For non MFD devices the recommendation for the target device IP was to
>>> ignore these fields, but not all of them followed the recommendation.
>>> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
>>> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
>>> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
>>> It turned out that this did not follow the recommendation and expected 0
>>> in this field.
>>>
>>> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
>>> all devices except target PCI devices.
>>>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>
>> Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW
>> GPIONC seems to have become more relaxed and accepts devfn 2.0 as well.
>>
>> I also tried to poke at all the other sideband units we care about and
>> they appear to work just as well with 2.0 and 0.0. But being consistent
>> seems like a good idea, so 0.0 it is for everyone.
>
> Shobhit, do you have any idea if there are devices out there that would
> really need this, i.e. should we backport this to stable kernels or is
> it good enough for next?
For sure the Asus T100 does not need any sequence and so does not need
this patch also. But I think there are other devices which might need.
Till now I figured out only GPIONC and GPS as two targets misbehaving.
If GPIONC is needed in sequences of those other devices in market, they
will get impacted.
Regards
Shobhit
>
> BR,
> Jani.
>
>
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>>> ---
>>> drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
>>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
>>> index 01d841e..731b10a 100644
>>> --- a/drivers/gpu/drm/i915/intel_sideband.c
>>> +++ b/drivers/gpu/drm/i915/intel_sideband.c
>>> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>>> SB_CRRDDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>>
>>> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>>> SB_CRWRDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>> }
>>> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>>
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>>> SB_CRRDDA_NP, reg, &val);
>>>
>>> return val;
>>> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>>
>>> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
>>> SB_CRRDDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>>
>>> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> --
>>> 1.9.1
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-05 17:14 ` Ville Syrjälä
2015-02-06 8:50 ` Jani Nikula
@ 2015-02-09 11:16 ` Shobhit Kumar
2015-02-09 12:27 ` Jani Nikula
1 sibling, 1 reply; 7+ messages in thread
From: Shobhit Kumar @ 2015-02-09 11:16 UTC (permalink / raw)
To: Ville Syrjälä, Shobhit Kumar
Cc: Jani Nikula, Daniel Vetter, intel-gfx
On 02/05/2015 10:44 PM, Ville Syrjälä wrote:
> On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote:
>> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
>> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
>> port ID should be enough to identify devices unless they are MFD. The
>> SB_DevFn was intended to remove ambiguity in case of these MFD devices.
>>
>> For non MFD devices the recommendation for the target device IP was to
>> ignore these fields, but not all of them followed the recommendation.
>> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
>> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
>> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
>> It turned out that this did not follow the recommendation and expected 0
>> in this field.
>>
>> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
>> all devices except target PCI devices.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW
> GPIONC seems to have become more relaxed and accepts devfn 2.0 as well.
>
> I also tried to poke at all the other sideband units we care about and
> they appear to work just as well with 2.0 and 0.0. But being consistent
> seems like a good idea, so 0.0 it is for everyone.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks Ville. Daniel this needs more testing/reviews to merge ?
Regards
Shobhit
>
>> ---
>> drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
>> index 01d841e..731b10a 100644
>> --- a/drivers/gpu/drm/i915/intel_sideband.c
>> +++ b/drivers/gpu/drm/i915/intel_sideband.c
>> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>> SB_CRRDDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>>
>> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>> SB_CRWRDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>> }
>> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>>
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>> SB_CRRDDA_NP, reg, &val);
>>
>> return val;
>> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>
>> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>
>> mutex_lock(&dev_priv->dpio_lock);
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
>> SB_CRRDDA_NP, addr, &val);
>> mutex_unlock(&dev_priv->dpio_lock);
>>
>> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
>> {
>> u32 val = 0;
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>> SB_CRRDDA_NP, reg, &val);
>> return val;
>> }
>>
>> void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>> {
>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>> SB_CRWRDA_NP, reg, &val);
>> }
>>
>> --
>> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers
2015-02-09 11:16 ` Shobhit Kumar
@ 2015-02-09 12:27 ` Jani Nikula
0 siblings, 0 replies; 7+ messages in thread
From: Jani Nikula @ 2015-02-09 12:27 UTC (permalink / raw)
To: Shobhit Kumar, Ville Syrjälä, Shobhit Kumar
Cc: Daniel Vetter, intel-gfx
On Mon, 09 Feb 2015, Shobhit Kumar <shobhit.kumar@linux.intel.com> wrote:
> On 02/05/2015 10:44 PM, Ville Syrjälä wrote:
>> On Thu, Feb 05, 2015 at 05:10:56PM +0530, Shobhit Kumar wrote:
>>> As per the specififcation, the SB_DevFn is the PCI_DEVFN of the target
>>> device and not the source. So PCI_DEVFN(2,0) is not correct. Further the
>>> port ID should be enough to identify devices unless they are MFD. The
>>> SB_DevFn was intended to remove ambiguity in case of these MFD devices.
>>>
>>> For non MFD devices the recommendation for the target device IP was to
>>> ignore these fields, but not all of them followed the recommendation.
>>> Some like CCK ignore these fields and hence PCI_DEVFN(2, 0) works and so
>>> does PCI_DEVFN(0, 0) as it works for DPIO. The issue came to light because
>>> of GPIONC which was not getting programmed correctly with PCI_DEVFN(2, 0).
>>> It turned out that this did not follow the recommendation and expected 0
>>> in this field.
>>>
>>> In general the recommendation is to use SB_DevFn as PCI_DEVFN(0, 0) for
>>> all devices except target PCI devices.
>>>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>
>> Yep, confirmed on my BYT that GPIONC doesn't like the devfn 2.0. On BSW
>> GPIONC seems to have become more relaxed and accepts devfn 2.0 as well.
>>
>> I also tried to poke at all the other sideband units we care about and
>> they appear to work just as well with 2.0 and 0.0. But being consistent
>> seems like a good idea, so 0.0 it is for everyone.
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Thanks Ville. Daniel this needs more testing/reviews to merge ?
Pushed to drm-intel-next-fixes with cc: stable, thanks for the patch and
review.
BR,
Jani.
>
> Regards
> Shobhit
>
>>
>>> ---
>>> drivers/gpu/drm/i915/intel_sideband.c | 26 +++++++++++++-------------
>>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
>>> index 01d841e..731b10a 100644
>>> --- a/drivers/gpu/drm/i915/intel_sideband.c
>>> +++ b/drivers/gpu/drm/i915/intel_sideband.c
>>> @@ -82,7 +82,7 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>>> SB_CRRDDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>>
>>> @@ -94,7 +94,7 @@ void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>>> SB_CRWRDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>> }
>>> @@ -103,7 +103,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>>
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>>> SB_CRRDDA_NP, reg, &val);
>>>
>>> return val;
>>> @@ -111,7 +111,7 @@ u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
>>>
>>> void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_BUNIT,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_BUNIT,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> @@ -122,7 +122,7 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>>> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>>
>>> mutex_lock(&dev_priv->dpio_lock);
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
>>> SB_CRRDDA_NP, addr, &val);
>>> mutex_unlock(&dev_priv->dpio_lock);
>>>
>>> @@ -132,56 +132,56 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>>> u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPIO_NC,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPIO_NC,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCK,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCK,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_CCU,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_CCU,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> u32 vlv_gps_core_read(struct drm_i915_private *dev_priv, u32 reg)
>>> {
>>> u32 val = 0;
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>>> SB_CRRDDA_NP, reg, &val);
>>> return val;
>>> }
>>>
>>> void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>>> {
>>> - vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_GPS_CORE,
>>> + vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_GPS_CORE,
>>> SB_CRWRDA_NP, reg, &val);
>>> }
>>>
>>> --
>>> 1.9.1
>>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-02-09 12:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-05 11:40 [PATCH] drm/i915: Correct the IOSF Dev_FN field for IOSF transfers Shobhit Kumar
2015-02-05 17:14 ` Ville Syrjälä
2015-02-06 8:50 ` Jani Nikula
2015-02-06 10:45 ` Shobhit Kumar
2015-02-09 11:16 ` Shobhit Kumar
2015-02-09 12:27 ` Jani Nikula
2015-02-05 22:39 ` shuang.he
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox