Re: [PATCH v2 6/8] drm/i915: Add function to output Aksv over GMBUS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 1, 2017 at 2:06 PM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Dec 01, 2017 at 12:20:28PM -0500, Sean Paul wrote:
>> Once the Aksv is available in the PCH, we need to get it on the wire to
>> the receiver via DDC. The hardware doesn't allow us to read the value
>> directly, so we need to tell GMBUS to source the Aksv internally and
>> send it to the right offset on the receiver.
>>
>> The way we do this is to initiate an indexed write where the index is
>> the Aksv register offset. We write dummy values to GMBUS3 as if we were
>> sending the key, and the hardware slips in the "real" values when it
>> goes out.
>>
>> Changes in v2:
>> - None
>>
>> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>>  drivers/gpu/drm/i915/intel_i2c.c | 54 ++++++++++++++++++++++++++++++++++------
>>  3 files changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 36bb4927484a..10f740c9e571 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -4043,6 +4043,7 @@ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv);
>>  extern void intel_teardown_gmbus(struct drm_i915_private *dev_priv);
>>  extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>>                                    unsigned int pin);
>> +extern int intel_gmbus_output_aksv(struct i2c_adapter *adapter);
>>
>>  extern struct i2c_adapter *
>>  intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 6dca305ccbf7..8b71a20882ca 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3040,6 +3040,7 @@ enum i915_power_well_id {
>>  # define GPIO_DATA_PULLUP_DISABLE    (1 << 13)
>>
>>  #define GMBUS0                       _MMIO(dev_priv->gpio_mmio_base + 0x5100) /* clock/port select */
>> +#define   GMBUS_AKSV_SELECT  (1<<11)
>>  #define   GMBUS_RATE_100KHZ  (0<<8)
>>  #define   GMBUS_RATE_50KHZ   (1<<8)
>>  #define   GMBUS_RATE_400KHZ  (2<<8) /* reserved on Pineview */
>> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
>> index eb5827110d8f..c01156bf0f27 100644
>> --- a/drivers/gpu/drm/i915/intel_i2c.c
>> +++ b/drivers/gpu/drm/i915/intel_i2c.c
>> @@ -30,6 +30,7 @@
>>  #include <linux/i2c-algo-bit.h>
>>  #include <linux/export.h>
>>  #include <drm/drmP.h>
>> +#include <drm/drm_hdcp.h>
>>  #include "intel_drv.h"
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>> @@ -373,7 +374,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>>
>>  static int
>>  gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
>> -                    unsigned short addr, u8 *buf, unsigned int len)
>> +                    unsigned short addr, u8 *buf, unsigned int len,
>> +                    u32 gmbus1_index)
>>  {
>>       unsigned int chunk_size = len;
>>       u32 val, loop;
>> @@ -386,7 +388,7 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
>>
>>       I915_WRITE_FW(GMBUS3, val);
>>       I915_WRITE_FW(GMBUS1,
>> -                   GMBUS_CYCLE_WAIT |
>> +                   gmbus1_index | GMBUS_CYCLE_WAIT |
>>                     (chunk_size << GMBUS_BYTE_COUNT_SHIFT) |
>>                     (addr << GMBUS_SLAVE_ADDR_SHIFT) |
>>                     GMBUS_SLAVE_WRITE | GMBUS_SW_RDY);
>> @@ -409,7 +411,8 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv,
>>  }
>>
>>  static int
>> -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
>> +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg,
>> +              u32 gmbus1_index)
>>  {
>>       u8 *buf = msg->buf;
>>       unsigned int tx_size = msg->len;
>> @@ -419,7 +422,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg)
>>       do {
>>               len = min(tx_size, GMBUS_BYTE_COUNT_MAX);
>>
>> -             ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len);
>> +             ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len,
>> +                                          gmbus1_index);
>>               if (ret)
>>                       return ret;
>>
>> @@ -470,7 +474,8 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs)
>>  }
>>
>>  static int
>> -do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>> +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num,
>> +           u32 gmbus0_source, u32 gmbus1_index)
>>  {
>>       struct intel_gmbus *bus = container_of(adapter,
>>                                              struct intel_gmbus,
>> @@ -480,7 +485,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>>       int ret = 0;
>>
>>  retry:
>> -     I915_WRITE_FW(GMBUS0, bus->reg0);
>> +     I915_WRITE_FW(GMBUS0, gmbus0_source | bus->reg0);
>>
>>       for (; i < num; i += inc) {
>>               inc = 1;
>> @@ -490,7 +495,8 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>>               } else if (msgs[i].flags & I2C_M_RD) {
>>                       ret = gmbus_xfer_read(dev_priv, &msgs[i], 0);
>>               } else {
>> -                     ret = gmbus_xfer_write(dev_priv, &msgs[i]);
>> +                     ret = gmbus_xfer_write(dev_priv, &msgs[i],
>> +                                            gmbus1_index);
>>               }
>>
>>               if (!ret)
>> @@ -598,7 +604,7 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>>               if (ret < 0)
>>                       bus->force_bit &= ~GMBUS_FORCE_BIT_RETRY;
>>       } else {
>> -             ret = do_gmbus_xfer(adapter, msgs, num);
>> +             ret = do_gmbus_xfer(adapter, msgs, num, 0, 0);
>>               if (ret == -EAGAIN)
>>                       bus->force_bit |= GMBUS_FORCE_BIT_RETRY;
>>       }
>> @@ -608,6 +614,38 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>>       return ret;
>>  }
>>
>> +int intel_gmbus_output_aksv(struct i2c_adapter *adapter)
>> +{
>> +     struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus,
>> +                                            adapter);
>> +     struct drm_i915_private *dev_priv = bus->dev_priv;
>> +     int ret;
>> +     u8 buf[DRM_HDCP_KSV_LEN] = { 0 };
>> +     struct i2c_msg msg = {
>> +             .addr = DRM_HDCP_DDC_ADDR,
>> +             .flags = 0,
>> +             .len = sizeof(buf),
>> +             .buf = buf,
>> +     };
>> +
>> +     intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>> +     mutex_lock(&dev_priv->gmbus_mutex);
>> +
>> +     /*
>> +      * In order to output Aksv to the receiver, use an indexed write to
>> +      * pass the i2c command, and tell GMBUS to use the HW-provided value
>> +      * instead of sourcing GMBUS3 for the data.
>> +      */
>> +     ret = do_gmbus_xfer(adapter, &msg, 1, GMBUS_AKSV_SELECT,
>> +                         GMBUS_CYCLE_INDEX |
>> +                         (DRM_HDCP_DDC_AKSV << GMBUS_SLAVE_INDEX_SHIFT));
>
> It might be nicer to just use two msgs here and modify
> gmbus_xfer_index_read() to support indexed writes as well.

I think two msgs implies a STOP bit in between. At least, that's how I
interpret "normal" writes.

> Would avoid having to pass in the GMBUS1 value all the way.

We could infer an indexed write if num_msgs == 1 && flags == 0 && len
> 1. However that would require us to assume that all single msg
writes have a command byte at the beginning of the transaction, which
might not be true for all i2c slaves.

Sean


>
> Not sure what to do with GMBUS_AKSV_SELECT. I guess either pass
> it in explicitly like you're doing here, or maybe add a small
> helper ala gmbus_is_index_read() so that do_gmbus_xfer() could
> sniff out which source we should use based on the passed in msgs?
>
>> +
>> +     mutex_unlock(&dev_priv->gmbus_mutex);
>> +     intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>> +
>> +     return ret;
>> +}
>> +
>>  static u32 gmbus_func(struct i2c_adapter *adapter)
>>  {
>>       return i2c_bit_algo.functionality(adapter) &
>> --
>> 2.15.0.531.g2ccb3012c9-goog
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux