| Message ID | 20250307-drm-rework-dpcd-access-v3-1-9044a3a868ee@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/display: dp: add new DPCD access functions | expand |
A few tiny nitpicks below, but with those addressed: Reviewed-by: Lyude Paul <lyude@redhat.com> On Fri, 2025-03-07 at 06:34 +0200, Dmitry Baryshkov wrote: > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Existing DPCD access functions return an error code or the number of > bytes being read / write in case of partial access. However a lot of > drivers either (incorrectly) ignore partial access or mishandle error > codes. In other cases this results in a boilerplate code which compares > returned value with the size. > > Implement new set of DPCD access helpers, which ignore partial access, > always return 0 or an error code. > > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com> > Acked-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/display/drm_dp_helper.c | 4 ++ > include/drm/display/drm_dp_helper.h | 92 ++++++++++++++++++++++++++++++++- > 2 files changed, 94 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c > index dbce1c3f49691fc687fee2404b723c73d533f23d..e43a8f4a252dae22eeaae1f4ca94da064303033d 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -704,6 +704,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_set_powered); > * function returns -EPROTO. Errors from the underlying AUX channel transfer > * function, with the exception of -EBUSY (which causes the transaction to > * be retried), are propagated to the caller. > + * > + * In most of the cases you want to use drm_dp_dpcd_read_data() instead. > */ > ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, > void *buffer, size_t size) > @@ -752,6 +754,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_read); > * function returns -EPROTO. Errors from the underlying AUX channel transfer > * function, with the exception of -EBUSY (which causes the transaction to > * be retried), are propagated to the caller. > + * > + * In most of the cases you want to use drm_dp_dpcd_write_data() instead. > */ > ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, > void *buffer, size_t size) > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h > index 5ae4241959f24e2c1fb581d7c7d770485d603099..c5be44d72c9a04474f6c795e03bf02bf08f5eaef 100644 > --- a/include/drm/display/drm_dp_helper.h > +++ b/include/drm/display/drm_dp_helper.h > @@ -527,6 +527,64 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, > ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, > void *buffer, size_t size); > > +/** > + * drm_dp_dpcd_read_data() - read a series of bytes from the DPCD > + * @aux: DisplayPort AUX channel (SST or MST) > + * @offset: address of the (first) register to read > + * @buffer: buffer to store the register values > + * @size: number of bytes in @buffer > + * > + * Returns zero (0) on success, or a negative error > + * code on failure. -EIO is returned if the request was NAKed by the sink or > + * if the retry count was exceeded. If not all bytes were transferred, this > + * function returns -EPROTO. Errors from the underlying AUX channel transfer > + * function, with the exception of -EBUSY (which causes the transaction to > + * be retried), are propagated to the caller. > + */ > +static inline int drm_dp_dpcd_read_data(struct drm_dp_aux *aux, > + unsigned int offset, > + void *buffer, size_t size) > +{ > + int ret; > + > + ret = drm_dp_dpcd_read(aux, offset, buffer, size); > + if (ret < 0) > + return ret; > + if (ret < size) > + return -EPROTO; > + > + return 0; > +} > + > +/** > + * drm_dp_dpcd_write_data() - write a series of bytes to the DPCD > + * @aux: DisplayPort AUX channel (SST or MST) > + * @offset: address of the (first) register to write > + * @buffer: buffer containing the values to write > + * @size: number of bytes in @buffer > + * > + * Returns zero (0) on success, or a negative error > + * code on failure. -EIO is returned if the request was NAKed by the sink or > + * if the retry count was exceeded. If not all bytes were transferred, this > + * function returns -EPROTO. Errors from the underlying AUX channel transfer > + * function, with the exception of -EBUSY (which causes the transaction to > + * be retried), are propagated to the caller. > + */ > +static inline int drm_dp_dpcd_write_data(struct drm_dp_aux *aux, > + unsigned int offset, > + void *buffer, size_t size) > +{ > + int ret; > + > + ret = drm_dp_dpcd_write(aux, offset, buffer, size); > + if (ret < 0) > + return ret; > + if (ret < size) > + return -EPROTO; > + > + return 0; > +} > + > /** > * drm_dp_dpcd_readb() - read a single byte from the DPCD > * @aux: DisplayPort AUX channel > @@ -534,7 +592,8 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, > * @valuep: location where the value of the register will be stored > * > * Returns the number of bytes transferred (1) on success, or a negative > - * error code on failure. > + * error code on failure. In most of the cases you should be using > + * drm_dp_dpcd_read_byte() instead Missing a period here > */ > static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux, > unsigned int offset, u8 *valuep) > @@ -549,7 +608,8 @@ static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux, > * @value: value to write to the register > * > * Returns the number of bytes transferred (1) on success, or a negative > - * error code on failure. > + * error code on failure. In most of the cases you should be using > + * drm_dp_dpcd_write_byte() instead And here Otherwise looks great! :) > */ > static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, > unsigned int offset, u8 value) > @@ -557,6 +617,34 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, > return drm_dp_dpcd_write(aux, offset, &value, 1); > } > > +/** > + * drm_dp_dpcd_read_byte() - read a single byte from the DPCD > + * @aux: DisplayPort AUX channel > + * @offset: address of the register to read > + * @valuep: location where the value of the register will be stored > + * > + * Returns zero (0) on success, or a negative error code on failure. > + */ > +static inline int drm_dp_dpcd_read_byte(struct drm_dp_aux *aux, > + unsigned int offset, u8 *valuep) > +{ > + return drm_dp_dpcd_read_data(aux, offset, valuep, 1); > +} > + > +/** > + * drm_dp_dpcd_write_byte() - write a single byte to the DPCD > + * @aux: DisplayPort AUX channel > + * @offset: address of the register to write > + * @value: value to write to the register > + * > + * Returns zero (0) on success, or a negative error code on failure. > + */ > +static inline int drm_dp_dpcd_write_byte(struct drm_dp_aux *aux, > + unsigned int offset, u8 value) > +{ > + return drm_dp_dpcd_write_data(aux, offset, &value, 1); > +} > + > int drm_dp_read_dpcd_caps(struct drm_dp_aux *aux, > u8 dpcd[DP_RECEIVER_CAP_SIZE]); > >
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index dbce1c3f49691fc687fee2404b723c73d533f23d..e43a8f4a252dae22eeaae1f4ca94da064303033d 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -704,6 +704,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_set_powered); * function returns -EPROTO. Errors from the underlying AUX channel transfer * function, with the exception of -EBUSY (which causes the transaction to * be retried), are propagated to the caller. + * + * In most of the cases you want to use drm_dp_dpcd_read_data() instead. */ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, void *buffer, size_t size) @@ -752,6 +754,8 @@ EXPORT_SYMBOL(drm_dp_dpcd_read); * function returns -EPROTO. Errors from the underlying AUX channel transfer * function, with the exception of -EBUSY (which causes the transaction to * be retried), are propagated to the caller. + * + * In most of the cases you want to use drm_dp_dpcd_write_data() instead. */ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, void *buffer, size_t size) diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index 5ae4241959f24e2c1fb581d7c7d770485d603099..c5be44d72c9a04474f6c795e03bf02bf08f5eaef 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -527,6 +527,64 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, void *buffer, size_t size); +/** + * drm_dp_dpcd_read_data() - read a series of bytes from the DPCD + * @aux: DisplayPort AUX channel (SST or MST) + * @offset: address of the (first) register to read + * @buffer: buffer to store the register values + * @size: number of bytes in @buffer + * + * Returns zero (0) on success, or a negative error + * code on failure. -EIO is returned if the request was NAKed by the sink or + * if the retry count was exceeded. If not all bytes were transferred, this + * function returns -EPROTO. Errors from the underlying AUX channel transfer + * function, with the exception of -EBUSY (which causes the transaction to + * be retried), are propagated to the caller. + */ +static inline int drm_dp_dpcd_read_data(struct drm_dp_aux *aux, + unsigned int offset, + void *buffer, size_t size) +{ + int ret; + + ret = drm_dp_dpcd_read(aux, offset, buffer, size); + if (ret < 0) + return ret; + if (ret < size) + return -EPROTO; + + return 0; +} + +/** + * drm_dp_dpcd_write_data() - write a series of bytes to the DPCD + * @aux: DisplayPort AUX channel (SST or MST) + * @offset: address of the (first) register to write + * @buffer: buffer containing the values to write + * @size: number of bytes in @buffer + * + * Returns zero (0) on success, or a negative error + * code on failure. -EIO is returned if the request was NAKed by the sink or + * if the retry count was exceeded. If not all bytes were transferred, this + * function returns -EPROTO. Errors from the underlying AUX channel transfer + * function, with the exception of -EBUSY (which causes the transaction to + * be retried), are propagated to the caller. + */ +static inline int drm_dp_dpcd_write_data(struct drm_dp_aux *aux, + unsigned int offset, + void *buffer, size_t size) +{ + int ret; + + ret = drm_dp_dpcd_write(aux, offset, buffer, size); + if (ret < 0) + return ret; + if (ret < size) + return -EPROTO; + + return 0; +} + /** * drm_dp_dpcd_readb() - read a single byte from the DPCD * @aux: DisplayPort AUX channel @@ -534,7 +592,8 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, * @valuep: location where the value of the register will be stored * * Returns the number of bytes transferred (1) on success, or a negative - * error code on failure. + * error code on failure. In most of the cases you should be using + * drm_dp_dpcd_read_byte() instead */ static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux, unsigned int offset, u8 *valuep) @@ -549,7 +608,8 @@ static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux, * @value: value to write to the register * * Returns the number of bytes transferred (1) on success, or a negative - * error code on failure. + * error code on failure. In most of the cases you should be using + * drm_dp_dpcd_write_byte() instead */ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, unsigned int offset, u8 value) @@ -557,6 +617,34 @@ static inline ssize_t drm_dp_dpcd_writeb(struct drm_dp_aux *aux, return drm_dp_dpcd_write(aux, offset, &value, 1); } +/** + * drm_dp_dpcd_read_byte() - read a single byte from the DPCD + * @aux: DisplayPort AUX channel + * @offset: address of the register to read + * @valuep: location where the value of the register will be stored + * + * Returns zero (0) on success, or a negative error code on failure. + */ +static inline int drm_dp_dpcd_read_byte(struct drm_dp_aux *aux, + unsigned int offset, u8 *valuep) +{ + return drm_dp_dpcd_read_data(aux, offset, valuep, 1); +} + +/** + * drm_dp_dpcd_write_byte() - write a single byte to the DPCD + * @aux: DisplayPort AUX channel + * @offset: address of the register to write + * @value: value to write to the register + * + * Returns zero (0) on success, or a negative error code on failure. + */ +static inline int drm_dp_dpcd_write_byte(struct drm_dp_aux *aux, + unsigned int offset, u8 value) +{ + return drm_dp_dpcd_write_data(aux, offset, &value, 1); +} + int drm_dp_read_dpcd_caps(struct drm_dp_aux *aux, u8 dpcd[DP_RECEIVER_CAP_SIZE]);