Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Fri, 10 Dec 2021, Kees Cook wrote: > On Fri, Dec 10, 2021 at 12:06:20PM +0200, Jani Nikula wrote: >> Post Cursor2 was completely optional for the transmitter even before it >> was deprecated. >> >> And now we'd be adding 5 bytes extra to all link status reads. To fix >> the only user of drm_dp_get_adjust_request_post_cursor() that apparently >> has never worked as intended. I'm just not convinced. >> >> I was trying to look through the implications of DP_LINK_STATUS_SIZE >> increase, and at least drm_dp_dpcd_read_phy_link_status() comes across >> as something probably needing attention. > > Okay, it sounds like you'd prefer the "make it tegra-specific" patch I > proposed. I will work that up as a proper patch and send it. Yes. Of course, I'd like to hear Thierry's view as well. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Fri, Dec 10, 2021 at 12:06:20PM +0200, Jani Nikula wrote:
> On Thu, 09 Dec 2021, Kees Cook wrote:
> > On Thu, Dec 09, 2021 at 05:20:45PM -0500, Harry Wentland wrote:
> >>
> >>
> >> On 2021-12-09 01:23, Kees Cook wrote:
> >> > On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
> >> >> On Fri, 03 Dec 2021, Kees Cook wrote:
> >> >>> The link_status array was not large enough to read the Adjust Request
> >> >>> Post Cursor2 register. Adjust the size to include it. Found with a
> >> >>> -Warray-bounds build:
> >> >>>
> >> >>> drivers/gpu/drm/drm_dp_helper.c: In function
> >> >>> 'drm_dp_get_adjust_request_post_cursor':
> >> >>> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is
> >> >>> outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> >> >>> [-Werror=array-bounds]
> >> >>>59 | return link_status[r - DP_LANE0_1_STATUS];
> >> >>> |~~~^~~
> >> >>> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
> >> >>> 'link_status'
> >> >>> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> >> >>> link_status[DP_LINK_STATUS_SIZE],
> >> >>> |
> >> >>> ~^~~~
> >> >>>
> >> >>> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor
> >> >>> adjustments")
> >> >>> Signed-off-by: Kees Cook
> >> >>
> >> >> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
> >> >> published in 2014, and Tegra is the only user of
> >> >> drm_dp_get_adjust_request_post_cursor().
> >> >
> >> > I see POST_CURSOR2 is used here too:
> >> >
> >> > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> >> >
> >>
> >> Looks like we read and parse that in the admgpu driver without
> >> using drm_dp_get_adjust_request_post_cursor.
> >
> > Right, and probably that could be switched to use it, but I'm not sure
> > what the impact of the larger link_status read is.
> >
> >>
> >> I don't have a strong feeling but I liked your original
> >> patch a bit better. I'm not sure what it means when part
> >> of a spec is deprecated. Once a spec is written display
> >> vendors might implement it. We should make sure that
> >> displays like that are always handled in a sane manner.
> >
> > Jani, Dave, any guidance here? I'm fine with whatever, but the current
> > code is for sure broken. ;)
>
> Post Cursor2 was completely optional for the transmitter even before it
> was deprecated.
>
> And now we'd be adding 5 bytes extra to all link status reads. To fix
> the only user of drm_dp_get_adjust_request_post_cursor() that apparently
> has never worked as intended. I'm just not convinced.
>
> I was trying to look through the implications of DP_LINK_STATUS_SIZE
> increase, and at least drm_dp_dpcd_read_phy_link_status() comes across
> as something probably needing attention.
Okay, it sounds like you'd prefer the "make it tegra-specific" patch I
proposed. I will work that up as a proper patch and send it.
Thanks!
--
Kees Cook
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Thu, 09 Dec 2021, Kees Cook wrote:
> On Thu, Dec 09, 2021 at 05:20:45PM -0500, Harry Wentland wrote:
>>
>>
>> On 2021-12-09 01:23, Kees Cook wrote:
>> > On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
>> >> On Fri, 03 Dec 2021, Kees Cook wrote:
>> >>> The link_status array was not large enough to read the Adjust Request
>> >>> Post Cursor2 register. Adjust the size to include it. Found with a
>> >>> -Warray-bounds build:
>> >>>
>> >>> drivers/gpu/drm/drm_dp_helper.c: In function
>> >>> 'drm_dp_get_adjust_request_post_cursor':
>> >>> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is
>> >>> outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
>> >>> [-Werror=array-bounds]
>> >>>59 | return link_status[r - DP_LANE0_1_STATUS];
>> >>> |~~~^~~
>> >>> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
>> >>> 'link_status'
>> >>> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
>> >>> link_status[DP_LINK_STATUS_SIZE],
>> >>> |
>> >>> ~^~~~
>> >>>
>> >>> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
>> >>> Signed-off-by: Kees Cook
>> >>
>> >> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
>> >> published in 2014, and Tegra is the only user of
>> >> drm_dp_get_adjust_request_post_cursor().
>> >
>> > I see POST_CURSOR2 is used here too:
>> >
>> > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> >
>>
>> Looks like we read and parse that in the admgpu driver without
>> using drm_dp_get_adjust_request_post_cursor.
>
> Right, and probably that could be switched to use it, but I'm not sure
> what the impact of the larger link_status read is.
>
>>
>> I don't have a strong feeling but I liked your original
>> patch a bit better. I'm not sure what it means when part
>> of a spec is deprecated. Once a spec is written display
>> vendors might implement it. We should make sure that
>> displays like that are always handled in a sane manner.
>
> Jani, Dave, any guidance here? I'm fine with whatever, but the current
> code is for sure broken. ;)
Post Cursor2 was completely optional for the transmitter even before it
was deprecated.
And now we'd be adding 5 bytes extra to all link status reads. To fix
the only user of drm_dp_get_adjust_request_post_cursor() that apparently
has never worked as intended. I'm just not convinced.
I was trying to look through the implications of DP_LINK_STATUS_SIZE
increase, and at least drm_dp_dpcd_read_phy_link_status() comes across
as something probably needing attention.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Thu, Dec 09, 2021 at 05:20:45PM -0500, Harry Wentland wrote:
>
>
> On 2021-12-09 01:23, Kees Cook wrote:
> > On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
> >> On Fri, 03 Dec 2021, Kees Cook wrote:
> >>> The link_status array was not large enough to read the Adjust Request
> >>> Post Cursor2 register. Adjust the size to include it. Found with a
> >>> -Warray-bounds build:
> >>>
> >>> drivers/gpu/drm/drm_dp_helper.c: In function
> >>> 'drm_dp_get_adjust_request_post_cursor':
> >>> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is
> >>> outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> >>> [-Werror=array-bounds]
> >>>59 | return link_status[r - DP_LANE0_1_STATUS];
> >>> |~~~^~~
> >>> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
> >>> 'link_status'
> >>> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> >>> link_status[DP_LINK_STATUS_SIZE],
> >>> |
> >>> ~^~~~
> >>>
> >>> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
> >>> Signed-off-by: Kees Cook
> >>
> >> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
> >> published in 2014, and Tegra is the only user of
> >> drm_dp_get_adjust_request_post_cursor().
> >
> > I see POST_CURSOR2 is used here too:
> >
> > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> >
>
> Looks like we read and parse that in the admgpu driver without
> using drm_dp_get_adjust_request_post_cursor.
Right, and probably that could be switched to use it, but I'm not sure
what the impact of the larger link_status read is.
>
> I don't have a strong feeling but I liked your original
> patch a bit better. I'm not sure what it means when part
> of a spec is deprecated. Once a spec is written display
> vendors might implement it. We should make sure that
> displays like that are always handled in a sane manner.
Jani, Dave, any guidance here? I'm fine with whatever, but the current
code is for sure broken. ;)
-Kees
--
Kees Cook
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On 2021-12-09 01:23, Kees Cook wrote:
> On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
>> On Fri, 03 Dec 2021, Kees Cook wrote:
>>> The link_status array was not large enough to read the Adjust Request
>>> Post Cursor2 register. Adjust the size to include it. Found with a
>>> -Warray-bounds build:
>>>
>>> drivers/gpu/drm/drm_dp_helper.c: In function
>>> 'drm_dp_get_adjust_request_post_cursor':
>>> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside
>>> array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
>>> [-Werror=array-bounds]
>>>59 | return link_status[r - DP_LANE0_1_STATUS];
>>> |~~~^~~
>>> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
>>> 'link_status'
>>> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
>>> link_status[DP_LINK_STATUS_SIZE],
>>> |
>>> ~^~~~
>>>
>>> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
>>> Signed-off-by: Kees Cook
>>
>> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
>> published in 2014, and Tegra is the only user of
>> drm_dp_get_adjust_request_post_cursor().
>
> I see POST_CURSOR2 is used here too:
>
> drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>
Looks like we read and parse that in the admgpu driver without
using drm_dp_get_adjust_request_post_cursor.
I don't have a strong feeling but I liked your original
patch a bit better. I'm not sure what it means when part
of a spec is deprecated. Once a spec is written display
vendors might implement it. We should make sure that
displays like that are always handled in a sane manner.
Harry
> Here's a version of that for tegra (untested):
>
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 23f9073bc473..c9528aa62c9c 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8
> link_status[DP_LINK_STATUS_SIZE],
> }
> EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset);
>
> -u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> - unsigned int lane)
> -{
> - unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
> - u8 value = dp_link_status(link_status, offset);
> -
> - return (value >> (lane << 1)) & 0x3;
> -}
> -EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
> -
> static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux, u8
> rd_interval)
> {
> if (rd_interval > 4)
> diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
> index 70dfb7d1dec5..f5535eb04c6b 100644
> --- a/drivers/gpu/drm/tegra/dp.c
> +++ b/drivers/gpu/drm/tegra/dp.c
> @@ -549,6 +549,15 @@ static void drm_dp_link_get_adjustments(struct
> drm_dp_link *link,
> {
> struct drm_dp_link_train_set *adjust = >train.adjust;
> unsigned int i;
> + u8 post_cursor;
> + int err;
> +
> + err = drm_dp_dpcd_read(link->aux, DP_ADJUST_REQUEST_POST_CURSOR2,
> +_cursor, sizeof(post_cursor));
> + if (err < 0) {
> + DRM_ERROR("failed to read post_cursor2: %d\n", err);
> + post_cursor = 0;
> + }
>
> for (i = 0; i < link->lanes; i++) {
> adjust->voltage_swing[i] =
> @@ -560,7 +569,7 @@ static void drm_dp_link_get_adjustments(struct
> drm_dp_link *link,
> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>
> adjust->post_cursor[i] =
> - drm_dp_get_adjust_request_post_cursor(status, i);
> + (post_cursor >> (i << 1)) & 0x3;
> }
> }
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 30359e434c3f..28378db676c8 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8
> link_status[DP_LINK_STATUS_SI
> int lane);
> u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE],
> int lane);
> -u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> - unsigned int lane);
>
> #define DP_BRANCH_OUI_HEADER_SIZE0xc
> #define DP_RECEIVER_CAP_SIZE 0xf
>
>
> Is that the right way to go?
>
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
> On Fri, 03 Dec 2021, Kees Cook wrote:
> > The link_status array was not large enough to read the Adjust Request
> > Post Cursor2 register. Adjust the size to include it. Found with a
> > -Warray-bounds build:
> >
> > drivers/gpu/drm/drm_dp_helper.c: In function
> > 'drm_dp_get_adjust_request_post_cursor':
> > drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside
> > array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> > [-Werror=array-bounds]
> >59 | return link_status[r - DP_LANE0_1_STATUS];
> > |~~~^~~
> > drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
> > 'link_status'
> > 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> > link_status[DP_LINK_STATUS_SIZE],
> > |
> > ~^~~~
> >
> > Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
> > Signed-off-by: Kees Cook
>
> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
> published in 2014, and Tegra is the only user of
> drm_dp_get_adjust_request_post_cursor().
I see POST_CURSOR2 is used here too:
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
Here's a version of that for tegra (untested):
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 23f9073bc473..c9528aa62c9c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8
link_status[DP_LINK_STATUS_SIZE],
}
EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset);
-u8 drm_dp_get_adjust_request_post_cursor(const u8
link_status[DP_LINK_STATUS_SIZE],
-unsigned int lane)
-{
- unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
- u8 value = dp_link_status(link_status, offset);
-
- return (value >> (lane << 1)) & 0x3;
-}
-EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
-
static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux, u8
rd_interval)
{
if (rd_interval > 4)
diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
index 70dfb7d1dec5..f5535eb04c6b 100644
--- a/drivers/gpu/drm/tegra/dp.c
+++ b/drivers/gpu/drm/tegra/dp.c
@@ -549,6 +549,15 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link
*link,
{
struct drm_dp_link_train_set *adjust = >train.adjust;
unsigned int i;
+ u8 post_cursor;
+ int err;
+
+ err = drm_dp_dpcd_read(link->aux, DP_ADJUST_REQUEST_POST_CURSOR2,
+ _cursor, sizeof(post_cursor));
+ if (err < 0) {
+ DRM_ERROR("failed to read post_cursor2: %d\n", err);
+ post_cursor = 0;
+ }
for (i = 0; i < link->lanes; i++) {
adjust->voltage_swing[i] =
@@ -560,7 +569,7 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link
*link,
DP_TRAIN_PRE_EMPHASIS_SHIFT;
adjust->post_cursor[i] =
- drm_dp_get_adjust_request_post_cursor(status, i);
+ (post_cursor >> (i << 1)) & 0x3;
}
}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 30359e434c3f..28378db676c8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8
link_status[DP_LINK_STATUS_SI
int lane);
u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE],
int lane);
-u8 drm_dp_get_adjust_request_post_cursor(const u8
link_status[DP_LINK_STATUS_SIZE],
-unsigned int lane);
#define DP_BRANCH_OUI_HEADER_SIZE 0xc
#define DP_RECEIVER_CAP_SIZE 0xf
Is that the right way to go?
--
Kees Cook
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Wed, Dec 08, 2021 at 01:19:28PM +0200, Jani Nikula wrote:
> On Fri, 03 Dec 2021, Kees Cook wrote:
> > The link_status array was not large enough to read the Adjust Request
> > Post Cursor2 register. Adjust the size to include it. Found with a
> > -Warray-bounds build:
> >
> > drivers/gpu/drm/drm_dp_helper.c: In function
> > 'drm_dp_get_adjust_request_post_cursor':
> > drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside
> > array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> > [-Werror=array-bounds]
> >59 | return link_status[r - DP_LANE0_1_STATUS];
> > |~~~^~~
> > drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
> > 'link_status'
> > 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> > link_status[DP_LINK_STATUS_SIZE],
> > |
> > ~^~~~
> >
> > Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
> > Signed-off-by: Kees Cook
>
> Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
> published in 2014, and Tegra is the only user of
> drm_dp_get_adjust_request_post_cursor().
>
> Instead of bumping the link status read size from 6 to 11 for all
> drivers I'd much rather see some other (maybe Tegra specific) solution
> to this.
Hmmm... Well given this is currently non-functional on Tegra (and is an
OOB memory read), how about just removing it entirely?
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 23f9073bc473..c9528aa62c9c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8
link_status[DP_LINK_STATUS_SIZE],
}
EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset);
-u8 drm_dp_get_adjust_request_post_cursor(const u8
link_status[DP_LINK_STATUS_SIZE],
-unsigned int lane)
-{
- unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
- u8 value = dp_link_status(link_status, offset);
-
- return (value >> (lane << 1)) & 0x3;
-}
-EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
-
static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux, u8
rd_interval)
{
if (rd_interval > 4)
diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
index 70dfb7d1dec5..bb5bfa93950f 100644
--- a/drivers/gpu/drm/tegra/dp.c
+++ b/drivers/gpu/drm/tegra/dp.c
@@ -559,8 +559,7 @@ static void drm_dp_link_get_adjustments(struct drm_dp_link
*link,
drm_dp_get_adjust_request_pre_emphasis(status, i) >>
DP_TRAIN_PRE_EMPHASIS_SHIFT;
- adjust->post_cursor[i] =
- drm_dp_get_adjust_request_post_cursor(status, i);
+ adjust->post_cursor[i] = 0;
}
}
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 30359e434c3f..28378db676c8 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8
link_status[DP_LINK_STATUS_SI
int lane);
u8 drm_dp_get_adjust_tx_ffe_preset(const u8 link_status[DP_LINK_STATUS_SIZE],
int lane);
-u8 drm_dp_get_adjust_request_post_cursor(const u8
link_status[DP_LINK_STATUS_SIZE],
-unsigned int lane);
#define DP_BRANCH_OUI_HEADER_SIZE 0xc
#define DP_RECEIVER_CAP_SIZE 0xf
Or maybe do a long link status read in Tegra only?
--
Kees Cook
Re: [PATCH] drm/dp: Actually read Adjust Request Post Cursor2 register
On Fri, 03 Dec 2021, Kees Cook wrote:
> The link_status array was not large enough to read the Adjust Request
> Post Cursor2 register. Adjust the size to include it. Found with a
> -Warray-bounds build:
>
> drivers/gpu/drm/drm_dp_helper.c: In function
> 'drm_dp_get_adjust_request_post_cursor':
> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside
> array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
> [-Werror=array-bounds]
>59 | return link_status[r - DP_LANE0_1_STATUS];
> |~~~^~~
> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
> 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
> link_status[DP_LINK_STATUS_SIZE],
> |
> ~^~~~
>
> Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
> Signed-off-by: Kees Cook
Using DP_ADJUST_REQUEST_POST_CURSOR2 has been deprecated since DP 1.3
published in 2014, and Tegra is the only user of
drm_dp_get_adjust_request_post_cursor().
Instead of bumping the link status read size from 6 to 11 for all
drivers I'd much rather see some other (maybe Tegra specific) solution
to this.
BR,
Jani.
> ---
> include/drm/drm_dp_helper.h | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 472dac376284..277643d2fe2c 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1517,7 +1517,15 @@ enum drm_dp_phy {
> #define DP_MST_LOGICAL_PORT_0 8
>
> #define DP_LINK_CONSTANT_N_VALUE 0x8000
> -#define DP_LINK_STATUS_SIZE 6
> +/*
> + * DPCD registers in link_status:
> + * Link Status: 0x202 through 0x204
> + * Sink Status: 0x205
> + * Adjust Request: 0x206 through 0x207
> + * Training Score: 0x208 through 0x20b
> + * AR Post Cursor2: 0x20c
> + */
> +#define DP_LINK_STATUS_SIZE 11
> bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
> int lane_count);
> bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
--
Jani Nikula, Intel Open Source Graphics Center
