Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On 12/8/2024 5:57 PM, Xiangxu Yin wrote: On 12/7/2024 4:13 AM, Abhinav Kumar wrote: On 12/3/2024 5:58 AM, Dmitry Baryshkov wrote: On Tue, Dec 03, 2024 at 03:41:53PM +0800, Xiangxu Yin wrote: On 12/2/2024 5:32 PM, Dmitry Baryshkov wrote: On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: Introduce a maximum width constraint for modes during validation. This ensures that the modes are filtered based on hardware capabilities, specifically addressing the line buffer limitations of individual pipes. This doesn't describe, why this is necessary. What does "buffer limitations of individual pipes" mean? If the platforms have hw capabilities like being unable to support 8k or 10k, it should go to platform data It's SSPP line buffer limitation for this platform and only support to 2160 mode width. Then, shall I add max_width config to struct msm_dp_desc in next patch? for other platform will set defualt value to ‘DP_MAX_WIDTH 7680' SSPP line buffer limitations are to be handled in the DPU driver. The DP driver shouldn't care about those. Ok, Will drop this part in next patch. If you drop it, what will be left from the patch itself? Yes agree with Dmitry, max_width is really not a DP related terminology. This patch should be dropped. So there were two issues, overall in this series causing this patch: 1) In https://patchwork.freedesktop.org/patch/625822/, instead of using VIG_SDM845_MASK, we should be using VIG_SDM845_MASK_SDMA. Without that even 2k will not work, will leave a comment there. 2) 4k will still fail. I dont think we can even support 4k on QCS615 but the modes should be filtered out because there is no 3dmux. I have submitted https://patchwork.freedesktop.org/patch/627694/ to address this. Xiangxu, please let me know if that works for you. Thanks Abhinav Thanks for your patchsets, After apply patch 625822 & 627694，mode filter works correctly on QCS615 platform with both 4k and 2k monitor. work>>> Thanks. If you can give your Tested-by on https://patchwork.freedesktop.org/patch/627967/, that would be great. Signed-off-by: Xiangxu Yin --- drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_panel.c | 13 + drivers/gpu/drm/msm/dp/dp_panel.h | 1 + 4 files changed, 18 insertions(+)
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On 12/7/2024 4:13 AM, Abhinav Kumar wrote: > > > On 12/3/2024 5:58 AM, Dmitry Baryshkov wrote: >> On Tue, Dec 03, 2024 at 03:41:53PM +0800, Xiangxu Yin wrote: >>> >>> >>> On 12/2/2024 5:32 PM, Dmitry Baryshkov wrote: On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: > > > > On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: >> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin >> wrote: >>> >>> Introduce a maximum width constraint for modes during validation. This >>> ensures that the modes are filtered based on hardware capabilities, >>> specifically addressing the line buffer limitations of individual pipes. >> >> This doesn't describe, why this is necessary. What does "buffer >> limitations of individual pipes" mean? >> If the platforms have hw capabilities like being unable to support 8k >> or 10k, it should go to platform data >> > It's SSPP line buffer limitation for this platform and only support to > 2160 mode width. > Then, shall I add max_width config to struct msm_dp_desc in next patch? > for other platform will set defualt value to ‘DP_MAX_WIDTH 7680' SSPP line buffer limitations are to be handled in the DPU driver. The DP driver shouldn't care about those. >>> Ok, Will drop this part in next patch. >> >> If you drop it, what will be left from the patch itself? >> > > Yes agree with Dmitry, max_width is really not a DP related terminology. > > This patch should be dropped. > > So there were two issues, overall in this series causing this patch: > > 1) In https://patchwork.freedesktop.org/patch/625822/, instead of using > VIG_SDM845_MASK, we should be using VIG_SDM845_MASK_SDMA. Without that even > 2k will not work, will leave a comment there. > > 2) 4k will still fail. I dont think we can even support 4k on QCS615 but the > modes should be filtered out because there is no 3dmux. > > I have submitted https://patchwork.freedesktop.org/patch/627694/ to address > this. > > Xiangxu, please let me know if that works for you. > > Thanks > > Abhinav Thanks for your patchsets, After apply patch 625822 & 627694，mode filter works correctly on QCS615 platform with both 4k and 2k monitor. work>>> >>> Signed-off-by: Xiangxu Yin >>> --- >>> drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ >>> drivers/gpu/drm/msm/dp/dp_display.h | 1 + >>> drivers/gpu/drm/msm/dp/dp_panel.c | 13 + >>> drivers/gpu/drm/msm/dp/dp_panel.h | 1 + >>> 4 files changed, 18 insertions(+) >>> >>
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On 12/3/2024 5:58 AM, Dmitry Baryshkov wrote: On Tue, Dec 03, 2024 at 03:41:53PM +0800, Xiangxu Yin wrote: On 12/2/2024 5:32 PM, Dmitry Baryshkov wrote: On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: Introduce a maximum width constraint for modes during validation. This ensures that the modes are filtered based on hardware capabilities, specifically addressing the line buffer limitations of individual pipes. This doesn't describe, why this is necessary. What does "buffer limitations of individual pipes" mean? If the platforms have hw capabilities like being unable to support 8k or 10k, it should go to platform data It's SSPP line buffer limitation for this platform and only support to 2160 mode width. Then, shall I add max_width config to struct msm_dp_desc in next patch? for other platform will set defualt value to ‘DP_MAX_WIDTH 7680' SSPP line buffer limitations are to be handled in the DPU driver. The DP driver shouldn't care about those. Ok, Will drop this part in next patch. If you drop it, what will be left from the patch itself? Yes agree with Dmitry, max_width is really not a DP related terminology. This patch should be dropped. So there were two issues, overall in this series causing this patch: 1) In https://patchwork.freedesktop.org/patch/625822/, instead of using VIG_SDM845_MASK, we should be using VIG_SDM845_MASK_SDMA. Without that even 2k will not work, will leave a comment there. 2) 4k will still fail. I dont think we can even support 4k on QCS615 but the modes should be filtered out because there is no 3dmux. I have submitted https://patchwork.freedesktop.org/patch/627694/ to address this. Xiangxu, please let me know if that works for you. Thanks Abhinav Signed-off-by: Xiangxu Yin --- drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_panel.c | 13 + drivers/gpu/drm/msm/dp/dp_panel.h | 1 + 4 files changed, 18 insertions(+)
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On Tue, Dec 03, 2024 at 03:41:53PM +0800, Xiangxu Yin wrote: > > > On 12/2/2024 5:32 PM, Dmitry Baryshkov wrote: > > On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: > >> > >> > >> > >> On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: > >>> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > >>> wrote: > > Introduce a maximum width constraint for modes during validation. This > ensures that the modes are filtered based on hardware capabilities, > specifically addressing the line buffer limitations of individual pipes. > >>> > >>> This doesn't describe, why this is necessary. What does "buffer > >>> limitations of individual pipes" mean? > >>> If the platforms have hw capabilities like being unable to support 8k > >>> or 10k, it should go to platform data > >>> > >> It's SSPP line buffer limitation for this platform and only support to > >> 2160 mode width. > >> Then, shall I add max_width config to struct msm_dp_desc in next patch? > >> for other platform will set defualt value to ‘DP_MAX_WIDTH 7680' > > > > SSPP line buffer limitations are to be handled in the DPU driver. The > > DP driver shouldn't care about those. > > > Ok, Will drop this part in next patch. If you drop it, what will be left from the patch itself? > > Signed-off-by: Xiangxu Yin > --- > drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dp/dp_panel.c | 13 + > drivers/gpu/drm/msm/dp/dp_panel.h | 1 + > 4 files changed, 18 insertions(+) > > > > > -- With best wishes Dmitry
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On 12/2/2024 5:32 PM, Dmitry Baryshkov wrote: > On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: >> >> >> >> On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: >>> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: Introduce a maximum width constraint for modes during validation. This ensures that the modes are filtered based on hardware capabilities, specifically addressing the line buffer limitations of individual pipes. >>> >>> This doesn't describe, why this is necessary. What does "buffer >>> limitations of individual pipes" mean? >>> If the platforms have hw capabilities like being unable to support 8k >>> or 10k, it should go to platform data >>> >> It's SSPP line buffer limitation for this platform and only support to 2160 >> mode width. >> Then, shall I add max_width config to struct msm_dp_desc in next patch? for >> other platform will set defualt value to ‘DP_MAX_WIDTH 7680' > > SSPP line buffer limitations are to be handled in the DPU driver. The > DP driver shouldn't care about those. > Ok, Will drop this part in next patch. Signed-off-by: Xiangxu Yin --- drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_panel.c | 13 + drivers/gpu/drm/msm/dp/dp_panel.h | 1 + 4 files changed, 18 insertions(+) > >
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On Mon, 2 Dec 2024 at 11:05, Xiangxu Yin wrote: > > > > On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote: > > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: > >> > >> Introduce a maximum width constraint for modes during validation. This > >> ensures that the modes are filtered based on hardware capabilities, > >> specifically addressing the line buffer limitations of individual pipes. > > > > This doesn't describe, why this is necessary. What does "buffer > > limitations of individual pipes" mean? > > If the platforms have hw capabilities like being unable to support 8k > > or 10k, it should go to platform data > > > It's SSPP line buffer limitation for this platform and only support to 2160 > mode width. > Then, shall I add max_width config to struct msm_dp_desc in next patch? for > other platform will set defualt value to ‘DP_MAX_WIDTH 7680' SSPP line buffer limitations are to be handled in the DPU driver. The DP driver shouldn't care about those. > >> > >> Signed-off-by: Xiangxu Yin > >> --- > >> drivers/gpu/drm/msm/dp/dp_display.c | 3 +++ > >> drivers/gpu/drm/msm/dp/dp_display.h | 1 + > >> drivers/gpu/drm/msm/dp/dp_panel.c | 13 + > >> drivers/gpu/drm/msm/dp/dp_panel.h | 1 + > >> 4 files changed, 18 insertions(+) -- With best wishes Dmitry
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On 11/29/2024 9:52 PM, Dmitry Baryshkov wrote:
> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote:
>>
>> Introduce a maximum width constraint for modes during validation. This
>> ensures that the modes are filtered based on hardware capabilities,
>> specifically addressing the line buffer limitations of individual pipes.
>
> This doesn't describe, why this is necessary. What does "buffer
> limitations of individual pipes" mean?
> If the platforms have hw capabilities like being unable to support 8k
> or 10k, it should go to platform data
>
It's SSPP line buffer limitation for this platform and only support to 2160
mode width.
Then, shall I add max_width config to struct msm_dp_desc in next patch? for
other platform will set defualt value to ‘DP_MAX_WIDTH 7680'
>>
>> Signed-off-by: Xiangxu Yin
>> ---
>> drivers/gpu/drm/msm/dp/dp_display.c | 3 +++
>> drivers/gpu/drm/msm/dp/dp_display.h | 1 +
>> drivers/gpu/drm/msm/dp/dp_panel.c | 13 +
>> drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
>> 4 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index
>> 4c83402fc7e0d41cb7621fa2efda043269d0a608..eb6fb76c68e505fafbec563440e9784f51e1894b
>> 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -944,6 +944,9 @@ enum drm_mode_status msm_dp_bridge_mode_valid(struct
>> drm_bridge *bridge,
>> msm_dp_display = container_of(dp, struct msm_dp_display_private,
>> msm_dp_display);
>> link_info = &msm_dp_display->panel->link_info;
>>
>> + if (mode->hdisplay > msm_dp_display->panel->max_dp_width)
>> + return MODE_BAD;
>> +
>> if (drm_mode_is_420_only(&dp->connector->display_info, mode) &&
>> msm_dp_display->panel->vsc_sdp_supported)
>> mode_pclk_khz /= 2;
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
>> b/drivers/gpu/drm/msm/dp/dp_display.h
>> index
>> ecbc2d92f546a346ee53adcf1b060933e4f54317..7a11f7eeb691976f06afc7aff67650397d7deb90
>> 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>> @@ -11,6 +11,7 @@
>> #include "disp/msm_disp_snapshot.h"
>>
>> #define DP_MAX_PIXEL_CLK_KHZ 675000
>> +#define DP_MAX_WIDTH 7680
>>
>> struct msm_dp {
>> struct drm_device *drm_dev;
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
>> b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index
>> 8654180aa259234bbd41f4f88c13c485f9791b1d..10501e301c5e073d8d34093b86a15d72e646a01f
>> 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -4,6 +4,7 @@
>> */
>>
>> #include "dp_panel.h"
>> +#include "dp_display.h"
>> #include "dp_utils.h"
>>
>> #include
>> @@ -455,6 +456,16 @@ static u32 msm_dp_panel_link_frequencies(struct
>> device_node *of_node)
>> return frequency;
>> }
>>
>> +static u32 msm_dp_panel_max_width(struct device_node *of_node)
>> +{
>> + u32 max_width = 0;
>> +
>> + if (of_property_read_u32(of_node, "max-width", &max_width))
>> + max_width = DP_MAX_WIDTH;
>> +
>> + return max_width;
>
> msm_dp_panel->max_dp_width = DP_MAX_WIDTH;
> of_property_read_u32(of_node, "max-width", &msm_dp_panel->max_dp_width);
>
>> +}
>> +
>> static int msm_dp_panel_parse_dt(struct msm_dp_panel *msm_dp_panel)
>> {
>> struct msm_dp_panel_private *panel;
>> @@ -490,6 +501,8 @@ static int msm_dp_panel_parse_dt(struct msm_dp_panel
>> *msm_dp_panel)
>> if (!msm_dp_panel->max_dp_link_rate)
>> msm_dp_panel->max_dp_link_rate = DP_LINK_RATE_HBR2;
>>
>> + msm_dp_panel->max_dp_width = msm_dp_panel_max_width(of_node);
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
>> b/drivers/gpu/drm/msm/dp/dp_panel.h
>> index
>> 7603b92c32902bd3d4485539bd6308537ff75a2c..61513644161209c243bbb623ee4ded951b2a0597
>> 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
>> @@ -51,6 +51,7 @@ struct msm_dp_panel {
>> u32 lane_map[DP_MAX_NUM_DP_LANES];
>> u32 max_dp_lanes;
>> u32 max_dp_link_rate;
>> + u32 max_dp_width;
>>
>> u32 max_bw_code;
>> };
>>
>> --
>> 2.25.1
>>
>
>
Re: [PATCH 6/8] drm/msm/dp: Add maximum width limitation for modes
On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote:
>
> Introduce a maximum width constraint for modes during validation. This
> ensures that the modes are filtered based on hardware capabilities,
> specifically addressing the line buffer limitations of individual pipes.
This doesn't describe, why this is necessary. What does "buffer
limitations of individual pipes" mean?
If the platforms have hw capabilities like being unable to support 8k
or 10k, it should go to platform data
>
> Signed-off-by: Xiangxu Yin
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 3 +++
> drivers/gpu/drm/msm/dp/dp_display.h | 1 +
> drivers/gpu/drm/msm/dp/dp_panel.c | 13 +
> drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
> 4 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index
> 4c83402fc7e0d41cb7621fa2efda043269d0a608..eb6fb76c68e505fafbec563440e9784f51e1894b
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -944,6 +944,9 @@ enum drm_mode_status msm_dp_bridge_mode_valid(struct
> drm_bridge *bridge,
> msm_dp_display = container_of(dp, struct msm_dp_display_private,
> msm_dp_display);
> link_info = &msm_dp_display->panel->link_info;
>
> + if (mode->hdisplay > msm_dp_display->panel->max_dp_width)
> + return MODE_BAD;
> +
> if (drm_mode_is_420_only(&dp->connector->display_info, mode) &&
> msm_dp_display->panel->vsc_sdp_supported)
> mode_pclk_khz /= 2;
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index
> ecbc2d92f546a346ee53adcf1b060933e4f54317..7a11f7eeb691976f06afc7aff67650397d7deb90
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -11,6 +11,7 @@
> #include "disp/msm_disp_snapshot.h"
>
> #define DP_MAX_PIXEL_CLK_KHZ 675000
> +#define DP_MAX_WIDTH 7680
>
> struct msm_dp {
> struct drm_device *drm_dev;
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index
> 8654180aa259234bbd41f4f88c13c485f9791b1d..10501e301c5e073d8d34093b86a15d72e646a01f
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -4,6 +4,7 @@
> */
>
> #include "dp_panel.h"
> +#include "dp_display.h"
> #include "dp_utils.h"
>
> #include
> @@ -455,6 +456,16 @@ static u32 msm_dp_panel_link_frequencies(struct
> device_node *of_node)
> return frequency;
> }
>
> +static u32 msm_dp_panel_max_width(struct device_node *of_node)
> +{
> + u32 max_width = 0;
> +
> + if (of_property_read_u32(of_node, "max-width", &max_width))
> + max_width = DP_MAX_WIDTH;
> +
> + return max_width;
msm_dp_panel->max_dp_width = DP_MAX_WIDTH;
of_property_read_u32(of_node, "max-width", &msm_dp_panel->max_dp_width);
> +}
> +
> static int msm_dp_panel_parse_dt(struct msm_dp_panel *msm_dp_panel)
> {
> struct msm_dp_panel_private *panel;
> @@ -490,6 +501,8 @@ static int msm_dp_panel_parse_dt(struct msm_dp_panel
> *msm_dp_panel)
> if (!msm_dp_panel->max_dp_link_rate)
> msm_dp_panel->max_dp_link_rate = DP_LINK_RATE_HBR2;
>
> + msm_dp_panel->max_dp_width = msm_dp_panel_max_width(of_node);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
> b/drivers/gpu/drm/msm/dp/dp_panel.h
> index
> 7603b92c32902bd3d4485539bd6308537ff75a2c..61513644161209c243bbb623ee4ded951b2a0597
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -51,6 +51,7 @@ struct msm_dp_panel {
> u32 lane_map[DP_MAX_NUM_DP_LANES];
> u32 max_dp_lanes;
> u32 max_dp_link_rate;
> + u32 max_dp_width;
>
> u32 max_bw_code;
> };
>
> --
> 2.25.1
>
--
With best wishes
Dmitry
