Re: [RFC PATCH 1/4] drm/msm: register a fault handler for display mmu faults
Quoting Abhinav Kumar (2024-05-17 16:37:56)
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct
> drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags,
> void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(>dump_mutex);
>From past experience I've seen the smmu fault handler called in hardirq
context, so it can't sleep. Is there some way to grab the register
contents without sleeping? Otherwise this will have to fork off
somewhere else that can take locks, runtime PM resume, etc.
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(>dump_mutex);
> +
Re: [PATCH] drm/msm: remove python 3.9 dependency for compiling msm
Hi,
On Tue, May 7, 2024 at 4:05 PM Abhinav Kumar wrote:
>
> Since commit 5acf49119630 ("drm/msm: import gen_header.py script from Mesa"),
> compilation is broken on machines having python versions older than 3.9
> due to dependency on argparse.BooleanOptionalAction.
>
> Switch to use simple bool for the validate flag to remove the dependency.
>
> Fixes: 5acf49119630 ("drm/msm: import gen_header.py script from Mesa")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/registers/gen_header.py | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
FWIW, it looks like the commit this is fixing is now present in
Linus's tree. Is there any plan to land this fix? It would be nifty if
it could somehow make it in time for -rc1 so I don't need to track
down this patch every time I need to build a subsystem tree for the
next several weeks...
-Doug
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
Hi, On Sun, May 19, 2024 at 2:01 AM Dmitry Baryshkov wrote: > > On Tue, May 14, 2024 at 03:55:14PM +0300, Jani Nikula wrote: > > Prefer the struct drm_edid based functions for reading the EDID and > > updating the connector. > > > > Simplify the flow by updating the EDID property when the EDID is read > > instead of at .get_modes. > > > > Signed-off-by: Jani Nikula > > > > --- > > The patch looks good to me, I'd like to hear an opinion from Doug (added > to CC). > > Reviewed-by: Dmitry Baryshkov > > What is the merge strategy for the series? Do you plan to pick up all > the patches to drm-misc or should we pick up individual patches into > driver trees? I'm not sure I have too much to add here aside from what you guys have already talked about. I'm OK with: Reviewed-by: Douglas Anderson
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
On Mon, 20 May 2024 at 15:25, Jani Nikula wrote:
>
> On Sun, 19 May 2024, Dmitry Baryshkov wrote:
> > On Tue, May 14, 2024 at 03:55:14PM +0300, Jani Nikula wrote:
> >> Prefer the struct drm_edid based functions for reading the EDID and
> >> updating the connector.
> >>
> >> Simplify the flow by updating the EDID property when the EDID is read
> >> instead of at .get_modes.
> >>
> >> Signed-off-by: Jani Nikula
> >>
> >> ---
> >
> > The patch looks good to me, I'd like to hear an opinion from Doug (added
> > to CC).
> >
> > Reviewed-by: Dmitry Baryshkov
>
> Thanks!
>
> > What is the merge strategy for the series? Do you plan to pick up all
> > the patches to drm-misc or should we pick up individual patches into
> > driver trees?
>
> I think all of the patches here are connected in theme, but
> independent. Either way is fine by me.
>
> >
> >
> >>
> >> Cc: Rob Clark
> >> Cc: Abhinav Kumar
> >> Cc: Dmitry Baryshkov
> >> Cc: Sean Paul
> >> Cc: Marijn Suijten
> >> Cc: linux-arm-...@vger.kernel.org
> >> Cc: freedreno@lists.freedesktop.org
> >> ---
> >> drivers/gpu/drm/msm/dp/dp_display.c | 11 +++
> >> drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
> >> drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
> >> 3 files changed, 20 insertions(+), 40 deletions(-)
> >
> > [skipped]
> >
> >> @@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel
> >> *dp_panel)
> >> panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
> >>
> >> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
> >> +/* FIXME: get rid of drm_edid_raw() */
> >
> > The code here can get use of something like drm_edid_smth_checksum().
> > 'Something', because I could not come up with the word that would make
> > it clear that it is the declared checksum instead of the actual /
> > computed one.
>
> This is an annoying one, to be honest, and linked to the historical fact
> that we filter some EDID blocks that have an incorrect checksum.
It is a part of the DP test suite if I remember correctly.
>
> (Some blocks, yes. We don't filter all blocks, because there are some
> nasty docks out there that modify the CTA block but fail to update the
> checksum, and filtering the CTA blocks would render the display
> useless. So we accept CTA blocks with incorrect checksums. But reject
> others. Yay.)
>
> IMO the real fix would be to stop mucking with the EDID, and just expose
> it to userspace, warts and all. We could still ignore the EDID blocks
> with incorrect checksum while using it ourselves if we want to. And with
> that, we could just have a function that checks the last EDID block's
> checksum, and stop using this ->real_edid_checksum thing.
>
> Anyway, yes, we could add the function already.
>
> BR,
> Jani.
>
> >
> >> +const struct edid *edid = drm_edid_raw(dp_panel->drm_edid);
> >> u8 checksum;
> >>
> >> -if (dp_panel->edid)
> >> -checksum = dp_panel_get_edid_checksum(dp_panel->edid);
> >> +if (edid)
> >> +checksum = dp_panel_get_edid_checksum(edid);
> >> else
> >> checksum = dp_panel->connector->real_edid_checksum;
> >>
>
> --
> Jani Nikula, Intel
--
With best wishes
Dmitry
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
On Sun, 19 May 2024, Dmitry Baryshkov wrote:
> On Tue, May 14, 2024 at 03:55:14PM +0300, Jani Nikula wrote:
>> Prefer the struct drm_edid based functions for reading the EDID and
>> updating the connector.
>>
>> Simplify the flow by updating the EDID property when the EDID is read
>> instead of at .get_modes.
>>
>> Signed-off-by: Jani Nikula
>>
>> ---
>
> The patch looks good to me, I'd like to hear an opinion from Doug (added
> to CC).
>
> Reviewed-by: Dmitry Baryshkov
Thanks!
> What is the merge strategy for the series? Do you plan to pick up all
> the patches to drm-misc or should we pick up individual patches into
> driver trees?
I think all of the patches here are connected in theme, but
independent. Either way is fine by me.
>
>
>>
>> Cc: Rob Clark
>> Cc: Abhinav Kumar
>> Cc: Dmitry Baryshkov
>> Cc: Sean Paul
>> Cc: Marijn Suijten
>> Cc: linux-arm-...@vger.kernel.org
>> Cc: freedreno@lists.freedesktop.org
>> ---
>> drivers/gpu/drm/msm/dp/dp_display.c | 11 +++
>> drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
>> drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
>> 3 files changed, 20 insertions(+), 40 deletions(-)
>
> [skipped]
>
>> @@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel
>> *dp_panel)
>> panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>>
>> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
>> +/* FIXME: get rid of drm_edid_raw() */
>
> The code here can get use of something like drm_edid_smth_checksum().
> 'Something', because I could not come up with the word that would make
> it clear that it is the declared checksum instead of the actual /
> computed one.
This is an annoying one, to be honest, and linked to the historical fact
that we filter some EDID blocks that have an incorrect checksum.
(Some blocks, yes. We don't filter all blocks, because there are some
nasty docks out there that modify the CTA block but fail to update the
checksum, and filtering the CTA blocks would render the display
useless. So we accept CTA blocks with incorrect checksums. But reject
others. Yay.)
IMO the real fix would be to stop mucking with the EDID, and just expose
it to userspace, warts and all. We could still ignore the EDID blocks
with incorrect checksum while using it ourselves if we want to. And with
that, we could just have a function that checks the last EDID block's
checksum, and stop using this ->real_edid_checksum thing.
Anyway, yes, we could add the function already.
BR,
Jani.
>
>> +const struct edid *edid = drm_edid_raw(dp_panel->drm_edid);
>> u8 checksum;
>>
>> -if (dp_panel->edid)
>> -checksum = dp_panel_get_edid_checksum(dp_panel->edid);
>> +if (edid)
>> +checksum = dp_panel_get_edid_checksum(edid);
>> else
>> checksum = dp_panel->connector->real_edid_checksum;
>>
--
Jani Nikula, Intel
[PATCH 7/7] drm/msm/dpu: support setting the TE source
Make the DPU driver use the TE source specified in the DT. If none is
specified, the driver defaults to the first GPIO (mdp_vsync0).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 44 -
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e9991f3756d4..932d0bb41d7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -505,6 +505,44 @@ static void dpu_kms_wait_flush(struct msm_kms *kms,
unsigned crtc_mask)
dpu_kms_wait_for_commit_done(kms, crtc);
}
+static const char *dpu_vsync_sources[] = {
+ [DPU_VSYNC_SOURCE_GPIO_0] = "mdp_gpio0",
+ [DPU_VSYNC_SOURCE_GPIO_1] = "mdp_gpio1",
+ [DPU_VSYNC_SOURCE_GPIO_2] = "mdp_gpio2",
+ [DPU_VSYNC_SOURCE_INTF_0] = "mdp_intf0",
+ [DPU_VSYNC_SOURCE_INTF_1] = "mdp_intf1",
+ [DPU_VSYNC_SOURCE_INTF_2] = "mdp_intf2",
+ [DPU_VSYNC_SOURCE_INTF_3] = "mdp_intf3",
+ [DPU_VSYNC_SOURCE_WD_TIMER_0] = "timer0",
+ [DPU_VSYNC_SOURCE_WD_TIMER_1] = "timer1",
+ [DPU_VSYNC_SOURCE_WD_TIMER_2] = "timer2",
+ [DPU_VSYNC_SOURCE_WD_TIMER_3] = "timer3",
+ [DPU_VSYNC_SOURCE_WD_TIMER_4] = "timer4",
+};
+
+static int dpu_kms_dsi_set_te_source(struct msm_display_info *info,
+struct msm_dsi *dsi)
+{
+ const char *te_source = msm_dsi_get_te_source(dsi);
+ int i;
+
+ if (!te_source) {
+ info->vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ return 0;
+ }
+
+ /* we can not use match_string since dpu_vsync_sources is a sparse
array */
+ for (i = 0; i < ARRAY_SIZE(dpu_vsync_sources); i++) {
+ if (dpu_vsync_sources[i] &&
+ !strcmp(dpu_vsync_sources[i], te_source)) {
+ info->vsync_source = i;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
@@ -543,7 +581,11 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
- info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ rc = dpu_kms_dsi_set_te_source(, priv->dsi[i]);
+ if (rc) {
+ DPU_ERROR("failed to identify TE source for dsi
display\n");
+ return rc;
+ }
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
if (IS_ERR(encoder)) {
--
2.39.2
[PATCH 6/7] drm/msm/dsi: parse vsync source from device tree
Allow board's device tree to specify the vsync source (aka TE source).
If the property is omitted, the display controller driver will use the
default setting.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c| 11 +++
drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +
drivers/gpu/drm/msm/msm_drv.h | 6 ++
4 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index afc290408ba4..87496db203d6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -37,6 +37,7 @@ struct msm_dsi {
struct mipi_dsi_host *host;
struct msm_dsi_phy *phy;
+ const char *te_source;
struct drm_bridge *next_bridge;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c4d72562c95a..c26ad0fed54d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1786,9 +1786,11 @@ static int dsi_populate_dsc_params(struct msm_dsi_host
*msm_host, struct drm_dsc
static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
{
+ struct msm_dsi *msm_dsi = platform_get_drvdata(msm_host->pdev);
struct device *dev = _host->pdev->dev;
struct device_node *np = dev->of_node;
struct device_node *endpoint;
+ const char *te_source;
int ret = 0;
/*
@@ -1811,6 +1813,15 @@ static int dsi_host_parse_dt(struct msm_dsi_host
*msm_host)
goto err;
}
+ ret = of_property_read_string(endpoint, "qcom,te-source", _source);
+ if (ret && ret != -EINVAL) {
+ DRM_DEV_ERROR(dev, "%s: invalid TE source configuration %d\n",
+ __func__, ret);
+ goto err;
+ }
+ if (!ret)
+ msm_dsi->te_source = devm_kstrdup(dev, te_source, GFP_KERNEL);
+
if (of_property_read_bool(np, "syscon-sfpb")) {
msm_host->sfpb = syscon_regmap_lookup_by_phandle(np,
"syscon-sfpb");
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 5b3f3068fd92..a210b7c9e5ca 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -603,3 +603,8 @@ bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
{
return IS_MASTER_DSI_LINK(msm_dsi->id);
}
+
+const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
+{
+ return msm_dsi->te_source;
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 912ebaa5df84..afd98dffea99 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -330,6 +330,7 @@ bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
bool msm_dsi_wide_bus_enabled(struct msm_dsi *msm_dsi);
struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
+const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi);
#else
static inline void __init msm_dsi_register(void)
{
@@ -367,6 +368,11 @@ static inline struct drm_dsc_config
*msm_dsi_get_dsc_config(struct msm_dsi *msm_
{
return NULL;
}
+
+static inline const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_DRM_MSM_DP
--
2.39.2
[PATCH 3/7] drm/msm/dsi: drop unused GPIOs handling
Neither disp-enable-gpios nor disp-te-gpios are defined in the schema.
None of the board DT files use those GPIO pins. Drop them from the
driver.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 37 -
1 file changed, 37 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a50f4dda5941..c4d72562c95a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -7,7 +7,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -130,9 +129,6 @@ struct msm_dsi_host {
unsigned long src_clk_rate;
- struct gpio_desc *disp_en_gpio;
- struct gpio_desc *te_gpio;
-
const struct msm_dsi_cfg_handler *cfg_hnd;
struct completion dma_comp;
@@ -1613,28 +1609,6 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr)
return IRQ_HANDLED;
}
-static int dsi_host_init_panel_gpios(struct msm_dsi_host *msm_host,
- struct device *panel_device)
-{
- msm_host->disp_en_gpio = devm_gpiod_get_optional(panel_device,
-"disp-enable",
-GPIOD_OUT_LOW);
- if (IS_ERR(msm_host->disp_en_gpio)) {
- DBG("cannot get disp-enable-gpios %ld",
- PTR_ERR(msm_host->disp_en_gpio));
- return PTR_ERR(msm_host->disp_en_gpio);
- }
-
- msm_host->te_gpio = devm_gpiod_get_optional(panel_device, "disp-te",
- GPIOD_IN);
- if (IS_ERR(msm_host->te_gpio)) {
- DBG("cannot get disp-te-gpios %ld", PTR_ERR(msm_host->te_gpio));
- return PTR_ERR(msm_host->te_gpio);
- }
-
- return 0;
-}
-
static int dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *dsi)
{
@@ -1651,11 +1625,6 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
if (dsi->dsc)
msm_host->dsc = dsi->dsc;
- /* Some gpios defined in panel DT need to be controlled by host */
- ret = dsi_host_init_panel_gpios(msm_host, >dev);
- if (ret)
- return ret;
-
ret = dsi_dev_attach(msm_host->pdev);
if (ret)
return ret;
@@ -2422,9 +2391,6 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
dsi_sw_reset(msm_host);
dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
- if (msm_host->disp_en_gpio)
- gpiod_set_value(msm_host->disp_en_gpio, 1);
-
msm_host->power_on = true;
mutex_unlock(_host->dev_mutex);
@@ -2454,9 +2420,6 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
dsi_ctrl_disable(msm_host);
- if (msm_host->disp_en_gpio)
- gpiod_set_value(msm_host->disp_en_gpio, 0);
-
pinctrl_pm_select_sleep_state(_host->pdev->dev);
cfg_hnd->ops->link_clk_disable(msm_host);
--
2.39.2
[PATCH 5/7] drm/msm/dpu: rework vsync_source handling
The struct msm_display_info has is_te_using_watchdog_timer field which
is neither used anywhere nor is flexible enough to specify different
sources. Replace it with the field specifying the vsync source using
enum dpu_vsync_source.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index bd37a56b4d03..b147f8814a18 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -743,10 +743,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
vsync_cfg.pp_count = dpu_enc->num_phys_encs;
vsync_cfg.frame_rate =
drm_mode_vrefresh(_enc->base.crtc->state->adjusted_mode);
- if (disp_info->is_te_using_watchdog_timer)
- vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
- else
- vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+ vsync_cfg.vsync_source = disp_info->vsync_source;
hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 76be77e30954..cb59bd4436f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -26,15 +26,14 @@
* @h_tile_instance:Controller instance used per tile. Number of elements
is
* based on num_of_h_tiles
* @is_cmd_modeBoolean to indicate if the CMD mode is requested
- * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is
- * used instead of panel TE in cmd mode panels
+ * @vsync_source: Source of the TE signal for DSI CMD devices
*/
struct msm_display_info {
enum dpu_intf_type intf_type;
uint32_t num_of_h_tiles;
uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
bool is_cmd_mode;
- bool is_te_using_watchdog_timer;
+ enum dpu_vsync_source vsync_source;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1955848b1b78..e9991f3756d4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -543,6 +543,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
+ info.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
+
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, );
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
--
2.39.2
[PATCH 4/7] drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source()
Setting vsync source makes sense only for DSI CMD panels. Pull the
is_cmd_mode condition out of the function into the calling code, so that
it becomes more explicit.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 4988a1029431..bd37a56b4d03 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -736,8 +736,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
return;
}
- if (hw_mdptop->ops.setup_vsync_source &&
- disp_info->is_cmd_mode) {
+ if (hw_mdptop->ops.setup_vsync_source) {
for (i = 0; i < dpu_enc->num_phys_encs; i++)
vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
@@ -1226,7 +1225,8 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select(
dpu_enc->cur_master->hw_mdptop);
- _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
+ if (dpu_enc->disp_info.is_cmd_mode)
+ _dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
if (dpu_enc->disp_info.intf_type == INTF_DSI &&
!WARN_ON(dpu_enc->num_phys_encs == 0)) {
--
2.39.2
[PATCH 2/7] drm/msm/dpu: convert vsync source defines to the enum
Add enum dpu_vsync_source instead of a series of defines. Use this enum
to pass vsync information.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 26 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..4988a1029431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -747,7 +747,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
if (disp_info->is_te_using_watchdog_timer)
vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_WD_TIMER_0;
else
- vsync_cfg.vsync_source = DPU_VSYNC0_SOURCE_GPIO;
+ vsync_cfg.vsync_source = DPU_VSYNC_SOURCE_GPIO_0;
hw_mdptop->ops.setup_vsync_source(hw_mdptop, _cfg);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 225c1c7768ff..96f6160cf607 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -462,7 +462,7 @@ static int dpu_hw_intf_get_vsync_info(struct dpu_hw_intf
*intf,
}
static void dpu_hw_intf_vsync_sel(struct dpu_hw_intf *intf,
- u32 vsync_source)
+ enum dpu_vsync_source vsync_source)
{
struct dpu_hw_blk_reg_map *c;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index f9015c67a574..ac244f0b33fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -107,7 +107,7 @@ struct dpu_hw_intf_ops {
int (*connect_external_te)(struct dpu_hw_intf *intf, bool
enable_external_te);
- void (*vsync_sel)(struct dpu_hw_intf *intf, u32 vsync_source);
+ void (*vsync_sel)(struct dpu_hw_intf *intf, enum dpu_vsync_source
vsync_source);
/**
* Disable autorefresh if enabled
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 66759623fc42..a2eff36a2224 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -54,18 +54,20 @@
#define DPU_BLEND_BG_INV_MOD_ALPHA (1 << 12)
#define DPU_BLEND_BG_TRANSP_EN (1 << 13)
-#define DPU_VSYNC0_SOURCE_GPIO 0
-#define DPU_VSYNC1_SOURCE_GPIO 1
-#define DPU_VSYNC2_SOURCE_GPIO 2
-#define DPU_VSYNC_SOURCE_INTF_03
-#define DPU_VSYNC_SOURCE_INTF_14
-#define DPU_VSYNC_SOURCE_INTF_25
-#define DPU_VSYNC_SOURCE_INTF_36
-#define DPU_VSYNC_SOURCE_WD_TIMER_411
-#define DPU_VSYNC_SOURCE_WD_TIMER_312
-#define DPU_VSYNC_SOURCE_WD_TIMER_213
-#define DPU_VSYNC_SOURCE_WD_TIMER_114
-#define DPU_VSYNC_SOURCE_WD_TIMER_015
+enum dpu_vsync_source {
+ DPU_VSYNC_SOURCE_GPIO_0,
+ DPU_VSYNC_SOURCE_GPIO_1,
+ DPU_VSYNC_SOURCE_GPIO_2,
+ DPU_VSYNC_SOURCE_INTF_0 = 3,
+ DPU_VSYNC_SOURCE_INTF_1,
+ DPU_VSYNC_SOURCE_INTF_2,
+ DPU_VSYNC_SOURCE_INTF_3,
+ DPU_VSYNC_SOURCE_WD_TIMER_4 = 11,
+ DPU_VSYNC_SOURCE_WD_TIMER_3,
+ DPU_VSYNC_SOURCE_WD_TIMER_2,
+ DPU_VSYNC_SOURCE_WD_TIMER_1,
+ DPU_VSYNC_SOURCE_WD_TIMER_0,
+};
enum dpu_hw_blk_type {
DPU_HW_BLK_TOP = 0,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index 6f3dc98087df..5c9a7ede991e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -64,7 +64,7 @@ struct dpu_vsync_source_cfg {
u32 pp_count;
u32 frame_rate;
u32 ppnumber[PINGPONG_MAX];
- u32 vsync_source;
+ enum dpu_vsync_source vsync_source;
};
/**
--
2.39.2
[PATCH 1/7] dt-bindings: display/msm/dsi: allow specifying TE source
Command mode panels provide TE signal back to the DSI host to signal
that the frame display has completed and update of the image will not
cause tearing. Usually it is connected to the first GPIO with the
mdp_vsync function, which is the default. In such case the property can
be skipped.
Signed-off-by: Dmitry Baryshkov
---
.../bindings/display/msm/dsi-controller-main.yaml| 16
1 file changed, 16 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 1fa28e976559..c1771c69b247 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -162,6 +162,21 @@ properties:
items:
enum: [ 0, 1, 2, 3 ]
+ qcom,te-source:
+$ref: /schemas/types.yaml#/definitions/string
+description:
+ Specifies the source of vsync signal from the panel used for
+ tearing elimination. The default is mdp_gpio0.
+enum:
+ - mdp_gpio0
+ - mdp_gpio1
+ - mdp_gpio2
+ - timer0
+ - timer1
+ - timer2
+ - timer3
+ - timer4
+
required:
- port@0
- port@1
@@ -452,6 +467,7 @@ examples:
dsi0_out: endpoint {
remote-endpoint = <_in>;
data-lanes = <0 1 2 3>;
+ qcom,te-source = "mdp_gpio2";
};
};
};
--
2.39.2
[PATCH 0/7] drm/msm/dpu: handle non-default TE source pins
Command-mode DSI panels need to signal the display controlller when vsync happens, so that the device can start sending the next frame. Some devices (Google Pixel 3) use a non-default pin, so additional configuration is required. Add a way to specify this information in DT and handle it in the DSI and DPU drivers. Signed-off-by: Dmitry Baryshkov --- Dmitry Baryshkov (7): dt-bindings: display/msm/dsi: allow specifying TE source drm/msm/dpu: convert vsync source defines to the enum drm/msm/dsi: drop unused GPIOs handling drm/msm/dpu: pull the is_cmd_mode out of _dpu_encoder_update_vsync_source() drm/msm/dpu: rework vsync_source handling drm/msm/dsi: parse vsync source from device tree drm/msm/dpu: support setting the TE source .../bindings/display/msm/dsi-controller-main.yaml | 16 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 11 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 5 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 26 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 44 drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 48 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 5 +++ drivers/gpu/drm/msm/msm_drv.h | 6 +++ 12 files changed, 106 insertions(+), 62 deletions(-) --- base-commit: 75fa778d74b786a1608d55d655d42b480a6fa8bd change-id: 20240514-dpu-handle-te-signal-82663c0211bd Best regards, -- Dmitry Baryshkov
