Re: [PATCH v3 06/15] drm/bridge: tc358767: Simplify AUX data read
On Thu, Jun 6, 2019 at 3:59 AM Andrzej Hajda wrote: > > On 05.06.2019 09:04, Andrey Smirnov wrote: > > Simplify AUX data read by removing index arithmetic and shifting with > > a helper functions that does three things: > > > > 1. Fetch data from up to 4 32-bit registers from the chip > > 2. Optionally fix data endianness (not needed on LE hosts) > > 3. Copy read data into user provided array. > > > > Signed-off-by: Andrey Smirnov > > Cc: Archit Taneja > > Cc: Andrzej Hajda > > Cc: Laurent Pinchart > > Cc: Tomi Valkeinen > > Cc: Andrey Gusakov > > Cc: Philipp Zabel > > Cc: Cory Tusar > > Cc: Chris Healy > > Cc: Lucas Stach > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-ker...@vger.kernel.org > > --- > > drivers/gpu/drm/bridge/tc358767.c | 40 +-- > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > > b/drivers/gpu/drm/bridge/tc358767.c > > index e197ce0fb166..da47d81e7109 100644 > > --- a/drivers/gpu/drm/bridge/tc358767.c > > +++ b/drivers/gpu/drm/bridge/tc358767.c > > @@ -321,6 +321,29 @@ static int tc_aux_get_status(struct tc_data *tc, u8 > > *reply) > > return 0; > > } > > > > +static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size) > > +{ > > + u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)]; > > + int ret, i, count = DIV_ROUND_UP(size, sizeof(u32)); > > + > > + ret = regmap_bulk_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count); > > + if (ret) > > + return ret; > > + > > + for (i = 0; i < count; i++) { > > + /* > > + * Our regmap is configured as LE for register data, > > + * so we need undo any byte swapping that might have > > + * happened to preserve original byte order. > > + */ > > + le32_to_cpus(&auxrdata[i]); > > + } > > + > > + memcpy(data, auxrdata, size); > > + > > + return size; > > +} > > + > > > Hmm, cannot we just use regmap_raw_read? I'll give it a try in v4. Thanks, Andrey Smirnov ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 06/15] drm/bridge: tc358767: Simplify AUX data read
On 05.06.2019 09:04, Andrey Smirnov wrote: > Simplify AUX data read by removing index arithmetic and shifting with > a helper functions that does three things: > > 1. Fetch data from up to 4 32-bit registers from the chip > 2. Optionally fix data endianness (not needed on LE hosts) > 3. Copy read data into user provided array. > > Signed-off-by: Andrey Smirnov > Cc: Archit Taneja > Cc: Andrzej Hajda > Cc: Laurent Pinchart > Cc: Tomi Valkeinen > Cc: Andrey Gusakov > Cc: Philipp Zabel > Cc: Cory Tusar > Cc: Chris Healy > Cc: Lucas Stach > Cc: dri-devel@lists.freedesktop.org > Cc: linux-ker...@vger.kernel.org > --- > drivers/gpu/drm/bridge/tc358767.c | 40 +-- > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index e197ce0fb166..da47d81e7109 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -321,6 +321,29 @@ static int tc_aux_get_status(struct tc_data *tc, u8 > *reply) > return 0; > } > > +static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size) > +{ > + u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)]; > + int ret, i, count = DIV_ROUND_UP(size, sizeof(u32)); > + > + ret = regmap_bulk_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count); > + if (ret) > + return ret; > + > + for (i = 0; i < count; i++) { > + /* > + * Our regmap is configured as LE for register data, > + * so we need undo any byte swapping that might have > + * happened to preserve original byte order. > + */ > + le32_to_cpus(&auxrdata[i]); > + } > + > + memcpy(data, auxrdata, size); > + > + return size; > +} > + Hmm, cannot we just use regmap_raw_read? Beside this: Reviewed-by: Andrzej Hajda -- Regards Andrzej > static ssize_t tc_aux_transfer(struct drm_dp_aux *aux, > struct drm_dp_aux_msg *msg) > { > @@ -379,19 +402,10 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux, > if (ret) > return ret; > > - if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) { > - /* Read data */ > - while (i < size) { > - if ((i % 4) == 0) { > - ret = regmap_read(tc->regmap, > - DP0_AUXRDATA(i >> 2), &tmp); > - if (ret) > - return ret; > - } > - buf[i] = tmp & 0xff; > - tmp = tmp >> 8; > - i++; > - } > + switch (request) { > + case DP_AUX_NATIVE_READ: > + case DP_AUX_I2C_READ: > + return tc_aux_read_data(tc, msg->buffer, size); > } > > return size; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 06/15] drm/bridge: tc358767: Simplify AUX data read
Simplify AUX data read by removing index arithmetic and shifting with a helper functions that does three things: 1. Fetch data from up to 4 32-bit registers from the chip 2. Optionally fix data endianness (not needed on LE hosts) 3. Copy read data into user provided array. Signed-off-by: Andrey Smirnov Cc: Archit Taneja Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Tomi Valkeinen Cc: Andrey Gusakov Cc: Philipp Zabel Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org --- drivers/gpu/drm/bridge/tc358767.c | 40 +-- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index e197ce0fb166..da47d81e7109 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -321,6 +321,29 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply) return 0; } +static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size) +{ + u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)]; + int ret, i, count = DIV_ROUND_UP(size, sizeof(u32)); + + ret = regmap_bulk_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count); + if (ret) + return ret; + + for (i = 0; i < count; i++) { + /* +* Our regmap is configured as LE for register data, +* so we need undo any byte swapping that might have +* happened to preserve original byte order. +*/ + le32_to_cpus(&auxrdata[i]); + } + + memcpy(data, auxrdata, size); + + return size; +} + static ssize_t tc_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { @@ -379,19 +402,10 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux, if (ret) return ret; - if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) { - /* Read data */ - while (i < size) { - if ((i % 4) == 0) { - ret = regmap_read(tc->regmap, - DP0_AUXRDATA(i >> 2), &tmp); - if (ret) - return ret; - } - buf[i] = tmp & 0xff; - tmp = tmp >> 8; - i++; - } + switch (request) { + case DP_AUX_NATIVE_READ: + case DP_AUX_I2C_READ: + return tc_aux_read_data(tc, msg->buffer, size); } return size; -- 2.21.0