2024-02-20 19:54:19

by Abhinav Kumar

Subject: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()

Currently the size parameter of drm_dp_vsc_sdp_pack() is always
the size of struct dp_sdp. Hence lets drop this parameter and
use sizeof() directly.

Suggested-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
---
drivers/gpu/drm/display/drm_dp_helper.c | 8 ++------
drivers/gpu/drm/i915/display/intel_dp.c | 3 +--
include/drm/display/drm_dp_helper.h | 3 +--
3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 6c91f400ecb1..10ee82e34de7 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2918,19 +2918,15 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
* @vsc: vsc sdp initialized according to its purpose as defined in
* table 2-118 - table 2-120 in DP 1.4a specification
* @sdp: valid handle to the generic dp_sdp which will be packed
- * @size: valid size of the passed sdp handle
*
* Returns length of sdp on success and error code on failure
*/
ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
- struct dp_sdp *sdp, size_t size)
+ struct dp_sdp *sdp)
{
size_t length = sizeof(struct dp_sdp);

- if (size < length)
- return -ENOSPC;
-
- memset(sdp, 0, size);
+ memset(sdp, 0, sizeof(struct dp_sdp));

/*
* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a9458df475e2..e13121dc3a03 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4181,8 +4181,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,

switch (type) {
case DP_SDP_VSC:
- len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp,
- sizeof(sdp));
+ len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp);
break;
case HDMI_PACKET_TYPE_GAMUT_METADATA:
len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 8474504d4c88..1f41994796d3 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -812,7 +812,6 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
int bpp_x16, unsigned long flags);
int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);

-ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
- struct dp_sdp *sdp, size_t size);
+ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp);

#endif /* _DRM_DP_HELPER_H_ */
--
2.34.1

2024-02-20 20:40:56

by Dmitry Baryshkov

Subject: Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()

On Tue, 20 Feb 2024 at 21:54, Abhinav Kumar <[email protected]> wrote:
>
> Currently the size parameter of drm_dp_vsc_sdp_pack() is always
> the size of struct dp_sdp. Hence lets drop this parameter and
> use sizeof() directly.
>
> Suggested-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 8 ++------
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +--
> include/drm/display/drm_dp_helper.h | 3 +--
> 3 files changed, 4 insertions(+), 10 deletions(-)

Thank you!

Acked-by: Dmitry Baryshkov <[email protected]>



--
With best wishes
Dmitry

2024-02-22 19:09:55

by Rodrigo Vivi

Subject: Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()

On Tue, Feb 20, 2024 at 11:53:47AM -0800, Abhinav Kumar wrote:
> Currently the size parameter of drm_dp_vsc_sdp_pack() is always
> the size of struct dp_sdp. Hence lets drop this parameter and
> use sizeof() directly.
>
> Suggested-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 8 ++------
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +--

Acked-by: Rodrigo Vivi <[email protected]>

> include/drm/display/drm_dp_helper.h | 3 +--
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 6c91f400ecb1..10ee82e34de7 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2918,19 +2918,15 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
> * @vsc: vsc sdp initialized according to its purpose as defined in
> * table 2-118 - table 2-120 in DP 1.4a specification
> * @sdp: valid handle to the generic dp_sdp which will be packed
> - * @size: valid size of the passed sdp handle
> *
> * Returns length of sdp on success and error code on failure
> */
> ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size)
> + struct dp_sdp *sdp)
> {
> size_t length = sizeof(struct dp_sdp);
>
> - if (size < length)
> - return -ENOSPC;
> -
> - memset(sdp, 0, size);
> + memset(sdp, 0, sizeof(struct dp_sdp));
>
> /*
> * Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a9458df475e2..e13121dc3a03 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4181,8 +4181,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>
> switch (type) {
> case DP_SDP_VSC:
> - len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp,
> - sizeof(sdp));
> + len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp);
> break;
> case HDMI_PACKET_TYPE_GAMUT_METADATA:
> len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index 8474504d4c88..1f41994796d3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -812,7 +812,6 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
> int bpp_x16, unsigned long flags);
> int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);
>
> -ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size);
> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp);
>
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.34.1
>

2024-02-22 20:16:20

by Rodrigo Vivi

Subject: Re: [PATCH v3 2/2] drm/dp: drop the size parameter from drm_dp_vsc_sdp_pack()

On Tue, Feb 20, 2024 at 11:53:47AM -0800, Abhinav Kumar wrote:
> Currently the size parameter of drm_dp_vsc_sdp_pack() is always
> the size of struct dp_sdp. Hence lets drop this parameter and
> use sizeof() directly.
>
> Suggested-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Abhinav Kumar <[email protected]>

it looks indeed an unecessary check.
you can convert my ack to a

Reviewed-by: Rodrigo Vivi <[email protected]>

and the ack to take this through drm-misc if needed

> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 8 ++------
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +--
> include/drm/display/drm_dp_helper.h | 3 +--
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 6c91f400ecb1..10ee82e34de7 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2918,19 +2918,15 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
> * @vsc: vsc sdp initialized according to its purpose as defined in
> * table 2-118 - table 2-120 in DP 1.4a specification
> * @sdp: valid handle to the generic dp_sdp which will be packed
> - * @size: valid size of the passed sdp handle
> *
> * Returns length of sdp on success and error code on failure
> */
> ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size)
> + struct dp_sdp *sdp)
> {
> size_t length = sizeof(struct dp_sdp);
>
> - if (size < length)
> - return -ENOSPC;
> -
> - memset(sdp, 0, size);
> + memset(sdp, 0, sizeof(struct dp_sdp));
>
> /*
> * Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a9458df475e2..e13121dc3a03 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4181,8 +4181,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>
> switch (type) {
> case DP_SDP_VSC:
> - len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp,
> - sizeof(sdp));
> + len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp);
> break;
> case HDMI_PACKET_TYPE_GAMUT_METADATA:
> len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index 8474504d4c88..1f41994796d3 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -812,7 +812,6 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
> int bpp_x16, unsigned long flags);
> int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);
>
> -ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
> - struct dp_sdp *sdp, size_t size);
> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp);
>
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.34.1
>