Re: [Freedreno] [PATCH v2 2/2] drm/msm/dp: add supported max link rate specified from dtsi
Quoting Kuogee Hsieh (2021-02-18 12:55:04) > Allow supported link rate to be limited to the value specified at > dtsi. If it is not specified, then link rate is derived from dpcd > directly. Below are examples, > link-rate = <162000> for max link rate limited at 1.62G > link-rate = <27> for max link rate limited at 2.7G > link-rate = <54> for max link rate limited at 5.4G > link-rate = <81> for max link rate limited at 8.1G > > Changes in V2: > -- allow supported max link rate specified from dtsi Please don't roll this into the patch that removes the limit. The previous version of this patch was fine. The part that lowers the limit back down should be another patch. We rejected link-rate in DT before and we should reject it upstream again. As far as I can tell, the maximum link rate should be determined based on the panel or the type-c port on the board. The dp controller can always achieve HBR3, so limiting it at the dp controller is incorrect. The driver should query the endpoints to figure out if they want to limit the link rate. Is that done automatically sometimes by intercepting the DPCD? ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 2/2] drm/msm/dp: add supported max link rate specified from dtsi
On Thu 18 Feb 14:55 CST 2021, Kuogee Hsieh wrote:
> Allow supported link rate to be limited to the value specified at
> dtsi. If it is not specified, then link rate is derived from dpcd
> directly. Below are examples,
> link-rate = <162000> for max link rate limited at 1.62G
> link-rate = <27> for max link rate limited at 2.7G
> link-rate = <54> for max link rate limited at 5.4G
> link-rate = <81> for max link rate limited at 8.1G
>
> Changes in V2:
> -- allow supported max link rate specified from dtsi
>
> Signed-off-by: Kuogee Hsieh
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 1 +
> drivers/gpu/drm/msm/dp/dp_panel.c | 7 ---
> drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
> drivers/gpu/drm/msm/dp/dp_parser.c | 13 +
> drivers/gpu/drm/msm/dp/dp_parser.h | 1 +
> 5 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 5a39da6..f633ba6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -322,6 +322,7 @@ static int dp_display_process_hpd_high(struct
> dp_display_private *dp)
> struct edid *edid;
>
> dp->panel->max_dp_lanes = dp->parser->max_dp_lanes;
> + dp->panel->max_link_rate = dp->parser->max_link_rate;
>
> rc = dp_panel_read_sink_caps(dp->panel, dp->dp_display.connector);
> if (rc)
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 9cc8166..be7f102 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -76,9 +76,10 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
> if (link_info->num_lanes > dp_panel->max_dp_lanes)
> link_info->num_lanes = dp_panel->max_dp_lanes;
>
> - /* Limit support upto HBR2 until HBR3 support is added */
> - if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
> - link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
> + /* Limit support of ink rate if specified */
> + if (dp_panel->max_link_rate &&
Make the parser always initialize max_link_rate to something reasonable
and just compare against that here.
> + (link_info->rate > dp_panel->max_link_rate))
No need for the (), nor for line breaking this.
> + link_info->rate = dp_panel->max_link_rate;
>
> DRM_DEBUG_DP("version: %d.%d\n", major, minor);
> DRM_DEBUG_DP("link_rate=%d\n", link_info->rate);
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
> b/drivers/gpu/drm/msm/dp/dp_panel.h
> index 9023e5b..1876f5e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -51,6 +51,7 @@ struct dp_panel {
> u32 vic;
> u32 max_pclk_khz;
> u32 max_dp_lanes;
> + u32 max_link_rate;
>
> u32 max_bw_code;
> };
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index 0519dd3..d8b6898 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -87,6 +87,8 @@ static int dp_parser_misc(struct dp_parser *parser)
> struct device_node *of_node = parser->pdev->dev.of_node;
> int len = 0;
> const char *data_lane_property = "data-lanes";
> + const char *link_rate_property = "link-rate";
There's no reason for stashing these in local variables.
> + u32 rate = 0;
>
> len = of_property_count_elems_of_size(of_node,
>data_lane_property, sizeof(u32));
> @@ -97,6 +99,17 @@ static int dp_parser_misc(struct dp_parser *parser)
> }
>
> parser->max_dp_lanes = len;
> +
> + len = of_property_count_elems_of_size(of_node,
> + link_rate_property, sizeof(u32));
I'm afraid I don't see the reason for this, simply rely on the return
value of of_property_read_u32(), i.e.
ret = of_property_read_u32(node, "link-rate", &rate);
if (!ret)
parser->max_link_rate = rate;
Or if you want to give it some default value:
rate = 1234;
of_property_read_u32(node, "link-rate", &rate);
Which either will overwrite the rate with the value of the property, or
leave it untouched.
Regards,
Bjorn
> +
> + if (len == 1) {
> + of_property_read_u32_index(of_node,
> + link_rate_property, 0, &rate);
> +
> + parser->max_link_rate = rate;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 34b4962..7046fce 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -116,6 +116,7 @@ struct dp_parser {
> struct dp_display_data disp_data;
> const struct dp_regulator_cfg *regulator_cfg;
> u32 max_dp_lanes;
> + u32 max_link_rate;
>
> int (*parse)(struct dp_parser *parser)
[Freedreno] [PATCH v2 2/2] drm/msm/dp: add supported max link rate specified from dtsi
Allow supported link rate to be limited to the value specified at
dtsi. If it is not specified, then link rate is derived from dpcd
directly. Below are examples,
link-rate = <162000> for max link rate limited at 1.62G
link-rate = <27> for max link rate limited at 2.7G
link-rate = <54> for max link rate limited at 5.4G
link-rate = <81> for max link rate limited at 8.1G
Changes in V2:
-- allow supported max link rate specified from dtsi
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 1 +
drivers/gpu/drm/msm/dp/dp_panel.c | 7 ---
drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
drivers/gpu/drm/msm/dp/dp_parser.c | 13 +
drivers/gpu/drm/msm/dp/dp_parser.h | 1 +
5 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 5a39da6..f633ba6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -322,6 +322,7 @@ static int dp_display_process_hpd_high(struct
dp_display_private *dp)
struct edid *edid;
dp->panel->max_dp_lanes = dp->parser->max_dp_lanes;
+ dp->panel->max_link_rate = dp->parser->max_link_rate;
rc = dp_panel_read_sink_caps(dp->panel, dp->dp_display.connector);
if (rc)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 9cc8166..be7f102 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -76,9 +76,10 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
if (link_info->num_lanes > dp_panel->max_dp_lanes)
link_info->num_lanes = dp_panel->max_dp_lanes;
- /* Limit support upto HBR2 until HBR3 support is added */
- if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
- link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
+ /* Limit support of ink rate if specified */
+ if (dp_panel->max_link_rate &&
+ (link_info->rate > dp_panel->max_link_rate))
+ link_info->rate = dp_panel->max_link_rate;
DRM_DEBUG_DP("version: %d.%d\n", major, minor);
DRM_DEBUG_DP("link_rate=%d\n", link_info->rate);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9023e5b..1876f5e 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -51,6 +51,7 @@ struct dp_panel {
u32 vic;
u32 max_pclk_khz;
u32 max_dp_lanes;
+ u32 max_link_rate;
u32 max_bw_code;
};
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 0519dd3..d8b6898 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -87,6 +87,8 @@ static int dp_parser_misc(struct dp_parser *parser)
struct device_node *of_node = parser->pdev->dev.of_node;
int len = 0;
const char *data_lane_property = "data-lanes";
+ const char *link_rate_property = "link-rate";
+ u32 rate = 0;
len = of_property_count_elems_of_size(of_node,
data_lane_property, sizeof(u32));
@@ -97,6 +99,17 @@ static int dp_parser_misc(struct dp_parser *parser)
}
parser->max_dp_lanes = len;
+
+ len = of_property_count_elems_of_size(of_node,
+link_rate_property, sizeof(u32));
+
+ if (len == 1) {
+ of_property_read_u32_index(of_node,
+ link_rate_property, 0, &rate);
+
+ parser->max_link_rate = rate;
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 34b4962..7046fce 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -116,6 +116,7 @@ struct dp_parser {
struct dp_display_data disp_data;
const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
+ u32 max_link_rate;
int (*parse)(struct dp_parser *parser);
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Hi Nicolas
On 2021-02-10 19:33, Nicolas Boichat wrote:
Many of the DSI flags have names opposite to their actual effects,
e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
be disabled. Fix this by including _NO_ in the flag names, e.g.
MIPI_DSI_MODE_NO_EOT_PACKET.
Signed-off-by: Nicolas Boichat
For the msm/dsi/ part, please feel free to add :
Reviewed-by: Abhinav Kumar
---
I considered adding _DISABLE_ instead, but that'd make the
flag names a big too long.
Generated with:
flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
(then minor format changes)
drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +-
drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++--
drivers/gpu/drm/bridge/tc358768.c| 2 +-
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 8
drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +-
drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +-
drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++--
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
drivers/gpu/drm/panel/panel-simple.c | 2 +-
drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
include/drm/drm_mipi_dsi.h | 8
25 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index aa19d5a40e31..59d718bde8c4 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv)
dsi->lanes = adv->num_dsi_lanes;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
- MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
+ MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
ret = mipi_dsi_attach(dsi);
if (ret < 0) {
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 65cc05982f82..beecfe6bf359 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1334,7 +1334,7 @@ static int anx7625_attach_dsi(struct anx7625_data
*ctx)
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO|
MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
- MIPI_DSI_MODE_EOT_PACKET|
+ MIPI_DSI_MODE_NO_EOT_PACKET |
MIPI_DSI_MODE_VIDEO_HSE;
if (mipi_dsi_attach(dsi) < 0) {
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
b/drivers/gpu/drm/bridge/cdns-dsi.c
index 76373e31df92..34aa24269a57 100644
--- a/drivers/gpu/drm/bridge/cdns-dsi.c
+++ b/drivers/gpu/drm/bridge/cdns-dsi.c
@@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct
drm_bridge *bridge)
tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) -
DIV_ROUND_UP(dsi_cfg.hsa, nlanes);
- if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
+ if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes);
tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8,
@@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct
drm_bridge *bridge)
tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL);
tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE);
- if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
+ if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
tmp |= HOST_EO
Re: [Freedreno] [PATCH 2/2] drm/msm/dp: Drop limit link rate at HBR2
Quoting Kuogee Hsieh (2021-02-17 15:09:57) > Drop limit link rate at HBR2 to support link rate > upto HBR3. > > Signed-off-by: Kuogee Hsieh > --- Should also say Tested-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 2/2] drm/msm/dp: Drop limit link rate at HBR2
Quoting Kuogee Hsieh (2021-02-17 15:09:57) > Drop limit link rate at HBR2 to support link rate > upto HBR3. > > Signed-off-by: Kuogee Hsieh > --- Reviewed-by: Stephen Boyd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2] drm/gem: Move drm_gem_fb_prepare_fb() to GEM atomic helpers
Hi, On Thu, Feb 11, 2021 at 09:16:36AM +0100, Thomas Zimmermann wrote: > diff --git a/include/drm/drm_gem_framebuffer_helper.h > b/include/drm/drm_gem_framebuffer_helper.h > index 6b013154911d..495d174d9989 100644 > --- a/include/drm/drm_gem_framebuffer_helper.h > +++ b/include/drm/drm_gem_framebuffer_helper.h > @@ -9,9 +9,11 @@ struct drm_framebuffer; > struct drm_framebuffer_funcs; > struct drm_gem_object; > struct drm_mode_fb_cmd2; > +#if 0 > struct drm_plane; > struct drm_plane_state; > struct drm_simple_display_pipe; > +#endif That's probably not what you meant? With that fixed, Acked-by: Maxime Ripard Thanks! Maxime signature.asc Description: PGP signature ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/msm/a6xx: fix for kernels without CONFIG_NVMEM
On Thu, Feb 18, 2021 at 4:28 AM Akhil P Oommen wrote:
>
> On 2/18/2021 2:05 AM, Jonathan Marek wrote:
> > On 2/17/21 3:18 PM, Rob Clark wrote:
> >> On Wed, Feb 17, 2021 at 11:08 AM Jordan Crouse
> >> wrote:
> >>>
> >>> On Wed, Feb 17, 2021 at 07:14:16PM +0530, Akhil P Oommen wrote:
> On 2/17/2021 8:36 AM, Rob Clark wrote:
> > On Tue, Feb 16, 2021 at 12:10 PM Jonathan Marek
> > wrote:
> >>
> >> Ignore nvmem_cell_get() EOPNOTSUPP error in the same way as a
> >> ENOENT error,
> >> to fix the case where the kernel was compiled without CONFIG_NVMEM.
> >>
> >> Fixes: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
> >> Signed-off-by: Jonathan Marek
> >> ---
> >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> index ba8e9d3cf0fe..7fe5d97606aa 100644
> >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >> @@ -1356,10 +1356,10 @@ static int a6xx_set_supported_hw(struct
> >> device *dev, struct a6xx_gpu *a6xx_gpu,
> >>
> >> cell = nvmem_cell_get(dev, "speed_bin");
> >> /*
> >> -* -ENOENT means that the platform doesn't support
> >> speedbin which is
> >> -* fine
> >> +* -ENOENT means no speed bin in device tree,
> >> +* -EOPNOTSUPP means kernel was built without CONFIG_NVMEM
> >
> > very minor nit, it would be nice to at least preserve the gist of the
> > "which is fine" (ie. some variation of "this is an optional thing and
> > things won't catch fire without it" ;-))
> >
> > (which is, I believe, is true, hopefully Akhil could confirm.. if not
> > we should have a harder dependency on CONFIG_NVMEM..)
> IIRC, if the gpu opp table in the DT uses the 'opp-supported-hw'
> property,
> we will see some error during boot up if we don't call
> dev_pm_opp_set_supported_hw(). So calling "nvmem_cell_get(dev,
> "speed_bin")"
> is a way to test this.
>
> If there is no other harm, we can put a hard dependency on
> CONFIG_NVMEM.
> >>>
> >>> I'm not sure if we want to go this far given the squishiness about
> >>> module
> >>> dependencies. As far as I know we are the only driver that uses this
> >>> seriously
> >>> on QCOM SoCs and this is only needed for certain targets. I don't
> >>> know if we
> >>> want to force every target to build NVMEM and QFPROM on our behalf.
> >>> But maybe
> >>> I'm just saying that because Kconfig dependencies tend to break my
> >>> brain (and
> >>> then Arnd has to send a patch to fix it).
> >>>
> >>
> >> Hmm, good point.. looks like CONFIG_NVMEM itself doesn't have any
> >> other dependencies, so I suppose it wouldn't be the end of the world
> >> to select that.. but I guess we don't want to require QFPROM
> >>
> >> I guess at the end of the day, what is the failure mode if you have a
> >> speed-bin device, but your kernel config misses QFPROM (and possibly
> >> NVMEM)? If the result is just not having the highest clk rate(s)
>
> Atleast on sc7180's gpu, using an unsupported FMAX breaks gmu. It won't
> be very obvious what went wrong when this happens!
Ugg, ok..
I suppose we could select NVMEM, but not QFPROM, and then the case
where QFPROM is not enabled on platforms that have the speed-bin field
in DT will fail gracefully and all other platforms would continue on
happily?
BR,
-R
>
> >> available, that isn't the end of the world. But if it makes things
> >> not-work, that is sub-optimal. Generally, especially on ARM, kconfig
> >> seems to be way harder than it should be to build a kernel that works,
> >> if we could somehow not add to that problem (for both people with a6xx
> >> and older gens) that would be nice ;-)
> >>
> >
> > There is a "imply" kconfig option which solves exactly this problem.
> > (you would "imply NVMEM" instead of "select NVMEM". then it would be
> > possible to disable NVMEM but it would get enabled by default)
> >
> >> BR,
> >> -R
> >>
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
Hi Nicolas,
On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote:
> Many of the DSI flags have names opposite to their actual effects,
> e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
> be disabled. Fix this by including _NO_ in the flag names, e.g.
> MIPI_DSI_MODE_NO_EOT_PACKET.
It is OK for anx7625.c part, Please add my r-b.
Reviewed-by: Xin Ji
>
> Signed-off-by: Nicolas Boichat
> ---
> I considered adding _DISABLE_ instead, but that'd make the
> flag names a big too long.
>
> Generated with:
> flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
> xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
> flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
> xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
> flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
> xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
> flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
> xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
> (then minor format changes)
>
> drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
> drivers/gpu/drm/bridge/analogix/anx7625.c| 2 +-
> drivers/gpu/drm/bridge/cdns-dsi.c| 4 ++--
> drivers/gpu/drm/bridge/tc358768.c| 2 +-
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8
> drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> drivers/gpu/drm/msm/dsi/dsi_host.c | 8
> drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
> drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
> drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
> drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
> drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
> drivers/gpu/drm/panel/panel-novatek-nt35510.c| 2 +-
> drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
> drivers/gpu/drm/panel/panel-samsung-s6d16d0.c| 2 +-
> drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
> drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c| 2 +-
> drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c| 4 ++--
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
> drivers/gpu/drm/panel/panel-simple.c | 2 +-
> drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
> drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
> include/drm/drm_mipi_dsi.h | 8
> 25 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index aa19d5a40e31..59d718bde8c4 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -165,7 +165,7 @@ int adv7533_attach_dsi(struct adv7511 *adv)
> dsi->lanes = adv->num_dsi_lanes;
> dsi->format = MIPI_DSI_FMT_RGB888;
> dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
> + MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
>
> ret = mipi_dsi_attach(dsi);
> if (ret < 0) {
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 65cc05982f82..beecfe6bf359 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1334,7 +1334,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx)
> dsi->format = MIPI_DSI_FMT_RGB888;
> dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> - MIPI_DSI_MODE_EOT_PACKET|
> + MIPI_DSI_MODE_NO_EOT_PACKET |
> MIPI_DSI_MODE_VIDEO_HSE;
>
> if (mipi_dsi_attach(dsi) < 0) {
> diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c
> b/drivers/gpu/drm/bridge/cdns-dsi.c
> index 76373e31df92..34aa24269a57 100644
> --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> @@ -829,7 +829,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
> *bridge)
> tmp = DIV_ROUND_UP(dsi_cfg.htotal, nlanes) -
> DIV_ROUND_UP(dsi_cfg.hsa, nlanes);
>
> - if (!(output->dev->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
> + if (!(output->dev->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
> tmp -= DIV_ROUND_UP(DSI_EOT_PKT_SIZE, nlanes);
>
> tx_byte_period = DIV_ROUND_DOWN_ULL((u64)NSEC_PER_SEC * 8,
> @@ -902,7 +902,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge
> *bridge)
> tmp = readl(dsi->regs + MCTL_MAIN_DATA_CTL);
> tmp &= ~(IF_VID_SELECT_MASK | HOST_EOT_GEN | IF_VID_MODE);
>
[Freedreno] [v4] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
Set the flag vblank_disable_immediate = true to turn off vblank irqs
immediately as soon as drm_vblank_put is requested so that there are
no irqs triggered during idle state. This will reduce cpu wakeups
and help in power saving.
To enable vblank_disable_immediate flag the underlying KMS driver
needs to support high precision vblank timestamping and also a
reliable way of providing vblank counter which is incrementing
at the leading edge of vblank.
This patch also brings in changes to support vblank_disable_immediate
requirement in dpu driver.
Changes in v1:
- Specify reason to add vblank timestamp support. (Rob).
- Add changes to provide vblank counter from dpu driver.
Changes in v2:
- Fix warn stack reported by Rob Clark with v2 patch.
Changes in v3:
- Move back to HW frame counter (Rob).
Signed-off-by: Kalyan Thota
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 80 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 30
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 11 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 1 +
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 26 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 5 ++
8 files changed, 155 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4662e8..9a80981 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -65,6 +65,83 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
kfree(dpu_crtc);
}
+static struct drm_encoder *get_encoder_from_crtc(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_encoder *encoder;
+
+ drm_for_each_encoder(encoder, dev)
+ if (encoder->crtc == crtc)
+ return encoder;
+
+ return NULL;
+}
+
+static u32 dpu_crtc_get_vblank_counter(struct drm_crtc *crtc)
+{
+ struct drm_encoder *encoder;
+
+ encoder = get_encoder_from_crtc(crtc);
+ if (!encoder) {
+ DRM_ERROR("no encoder found for crtc %d\n", crtc->index);
+ return false;
+ }
+
+ return dpu_encoder_get_frame_count(encoder);
+}
+
+static bool dpu_crtc_get_scanout_position(struct drm_crtc *crtc,
+ bool in_vblank_irq,
+ int *vpos, int *hpos,
+ ktime_t *stime, ktime_t *etime,
+ const struct drm_display_mode *mode)
+{
+ unsigned int pipe = crtc->index;
+ struct drm_encoder *encoder;
+ int line, vsw, vbp, vactive_start, vactive_end, vfp_end;
+
+ encoder = get_encoder_from_crtc(crtc);
+ if (!encoder) {
+ DRM_ERROR("no encoder found for crtc %d\n", pipe);
+ return false;
+ }
+
+ vsw = mode->crtc_vsync_end - mode->crtc_vsync_start;
+ vbp = mode->crtc_vtotal - mode->crtc_vsync_end;
+
+ /*
+* the line counter is 1 at the start of the VSYNC pulse and VTOTAL at
+* the end of VFP. Translate the porch values relative to the line
+* counter positions.
+*/
+
+ vactive_start = vsw + vbp + 1;
+ vactive_end = vactive_start + mode->crtc_vdisplay;
+
+ /* last scan line before VSYNC */
+ vfp_end = mode->crtc_vtotal;
+
+ if (stime)
+ *stime = ktime_get();
+
+ line = dpu_encoder_get_linecount(encoder);
+
+ if (line < vactive_start)
+ line -= vactive_start;
+ else if (line > vactive_end)
+ line = line - vfp_end - vactive_start;
+ else
+ line -= vactive_start;
+
+ *vpos = line;
+ *hpos = 0;
+
+ if (etime)
+ *etime = ktime_get();
+
+ return true;
+}
+
static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
struct dpu_plane_state *pstate, struct dpu_format *format)
{
@@ -1243,6 +1320,8 @@ static const struct drm_crtc_funcs dpu_crtc_funcs = {
.early_unregister = dpu_crtc_early_unregister,
.enable_vblank = msm_crtc_enable_vblank,
.disable_vblank = msm_crtc_disable_vblank,
+ .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
+ .get_vblank_counter = dpu_crtc_get_vblank_counter,
};
static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
@@ -1251,6 +1330,7 @@ static const struct drm_crtc_helper_funcs
dpu_crtc_helper_funcs = {
.atomic_check = dpu_crtc_atomic_check,
.atomic_begin = dpu_crtc_atomic_begin,
.atomic_flush = dpu_crtc_atomic_flush,
+ .get_scanout_position = dpu_crtc_get_scanout_position,
};
/* initialize crtc */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/dr
Re: [Freedreno] [PATCH] drm/msm/a6xx: fix for kernels without CONFIG_NVMEM
On 2/18/2021 2:05 AM, Jonathan Marek wrote:
On 2/17/21 3:18 PM, Rob Clark wrote:
On Wed, Feb 17, 2021 at 11:08 AM Jordan Crouse
wrote:
On Wed, Feb 17, 2021 at 07:14:16PM +0530, Akhil P Oommen wrote:
On 2/17/2021 8:36 AM, Rob Clark wrote:
On Tue, Feb 16, 2021 at 12:10 PM Jonathan Marek
wrote:
Ignore nvmem_cell_get() EOPNOTSUPP error in the same way as a
ENOENT error,
to fix the case where the kernel was compiled without CONFIG_NVMEM.
Fixes: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Jonathan Marek
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ba8e9d3cf0fe..7fe5d97606aa 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1356,10 +1356,10 @@ static int a6xx_set_supported_hw(struct
device *dev, struct a6xx_gpu *a6xx_gpu,
cell = nvmem_cell_get(dev, "speed_bin");
/*
- * -ENOENT means that the platform doesn't support
speedbin which is
- * fine
+ * -ENOENT means no speed bin in device tree,
+ * -EOPNOTSUPP means kernel was built without CONFIG_NVMEM
very minor nit, it would be nice to at least preserve the gist of the
"which is fine" (ie. some variation of "this is an optional thing and
things won't catch fire without it" ;-))
(which is, I believe, is true, hopefully Akhil could confirm.. if not
we should have a harder dependency on CONFIG_NVMEM..)
IIRC, if the gpu opp table in the DT uses the 'opp-supported-hw'
property,
we will see some error during boot up if we don't call
dev_pm_opp_set_supported_hw(). So calling "nvmem_cell_get(dev,
"speed_bin")"
is a way to test this.
If there is no other harm, we can put a hard dependency on
CONFIG_NVMEM.
I'm not sure if we want to go this far given the squishiness about
module
dependencies. As far as I know we are the only driver that uses this
seriously
on QCOM SoCs and this is only needed for certain targets. I don't
know if we
want to force every target to build NVMEM and QFPROM on our behalf.
But maybe
I'm just saying that because Kconfig dependencies tend to break my
brain (and
then Arnd has to send a patch to fix it).
Hmm, good point.. looks like CONFIG_NVMEM itself doesn't have any
other dependencies, so I suppose it wouldn't be the end of the world
to select that.. but I guess we don't want to require QFPROM
I guess at the end of the day, what is the failure mode if you have a
speed-bin device, but your kernel config misses QFPROM (and possibly
NVMEM)? If the result is just not having the highest clk rate(s)
Atleast on sc7180's gpu, using an unsupported FMAX breaks gmu. It won't
be very obvious what went wrong when this happens!
-Akhil.
available, that isn't the end of the world. But if it makes things
not-work, that is sub-optimal. Generally, especially on ARM, kconfig
seems to be way harder than it should be to build a kernel that works,
if we could somehow not add to that problem (for both people with a6xx
and older gens) that would be nice ;-)
There is a "imply" kconfig option which solves exactly this problem.
(you would "imply NVMEM" instead of "select NVMEM". then it would be
possible to disable NVMEM but it would get enabled by default)
BR,
-R
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
