From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751319AbdLATRn (ORCPT ); Fri, 1 Dec 2017 14:17:43 -0500 Received: from mail-it0-f65.google.com ([209.85.214.65]:33289 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750968AbdLATRm (ORCPT ); Fri, 1 Dec 2017 14:17:42 -0500 X-Google-Smtp-Source: AGs4zMYYEHictTLUO0TbpmDZuiJErFrPxmaBPCTxRs072yO4hI1l72CNUUUkGSOxUVRSM3JKajM2sA== MIME-Version: 1.0 In-Reply-To: <20171201190614.GM10981@intel.com> References: <20171201172032.47357-1-seanpaul@chromium.org> <20171201172032.47357-7-seanpaul@chromium.org> <20171201190614.GM10981@intel.com> From: Sean Paul Date: Fri, 1 Dec 2017 14:17:19 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 6/8] drm/i915: Add function to output Aksv over GMBUS To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: dri-devel , Intel Graphics Development , David Airlie , Joonas Lahtinen , Linux Kernel Mailing List , Rodrigo Vivi , Daniel Vetter Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vB1JHlwU001530 On Fri, Dec 1, 2017 at 2:06 PM, Ville Syrjälä 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 >> --- >> 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 >> #include >> #include >> +#include >> #include "intel_drv.h" >> #include >> #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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC