Re: [Freedreno] [PATCH v3 2/2] drm/msm/dp: Implement oob_hotplug_event()
On Fri 22 Apr 16:07 PDT 2022, Dmitry Baryshkov wrote:
> On 23/04/2022 01:32, Bjorn Andersson wrote:
[..]
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> > b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index 80f59cf99089..76904b1601b1 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -123,6 +123,14 @@ static enum drm_mode_status dp_connector_mode_valid(
> > return dp_display_validate_mode(dp_disp, mode->clock);
> > }
> > +static void dp_oob_hotplug_event(struct drm_connector *connector,
> > +enum drm_connector_hpd_state hpd_state)
> > +{
> > + struct msm_dp *dp_disp = to_dp_connector(connector)->dp_display;
> > +
> > + dp_display_oob_hotplug_event(dp_disp, hpd_state);
> > +}
> > +
> > static const struct drm_connector_funcs dp_connector_funcs = {
> > .detect = dp_connector_detect,
> > .fill_modes = drm_helper_probe_single_connector_modes,
> > @@ -130,6 +138,7 @@ static const struct drm_connector_funcs
> > dp_connector_funcs = {
> > .reset = drm_atomic_helper_connector_reset,
> > .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > + .oob_hotplug_event = dp_oob_hotplug_event,
>
> We were (are) going to switch dp driver to use drm_bridge_connector (to fix
> support for bridge chains, eDP panels, etc.
>
> So these changes must be ported to drm_bridge_connector (or we must
> abandon/defer the idea of using the bridge_connector).
>
> For the oob_hotplug_event() callback proper support might be as simple as
> calling drm_bridge_connector_hpd_cb().
>
Are you saying that you have code ready and being merged into linux-next
that I should redesign this on top of, or that you're planning to write
some code in the future and DisplayPort support have to wait until then?
> > };
> > static const struct drm_connector_helper_funcs dp_connector_helper_funcs
> > = {
> > @@ -160,6 +169,8 @@ struct drm_connector *dp_drm_connector_init(struct
> > msm_dp *dp_display)
> > if (ret)
> > return ERR_PTR(ret);
> > + connector->fwnode = fwnode_handle_get(dev_fwnode(dp_display->dev));
> > +
>
> This would be much more interesting. Supporting this in a generic way might
> be tricky. But we can still set the fwnode manually from the dp code.
>
There's a slight mishmash here, because the device used to initialize
the connector is the drm_dev, but we need the actual fwnode of the DP
device associated with the connector.
So I think this is how it needs to be done.
Regards,
Bjorn
Re: [Freedreno] [PATCH] clk: qcom: clk-rcg2: fix gfx3d frequency calculation
Quoting Maxime Ripard (2022-04-22 02:48:17)
> Hi,
>
> On Thu, Apr 21, 2022 at 07:49:12PM -0700, Stephen Boyd wrote:
> > +Maxime
> >
> > Quoting Dmitry Baryshkov (2022-04-19 16:54:47)
> > > Since the commit 948fb0969eae ("clk: Always clamp the rounded rate"),
> > > the clk_core_determine_round_nolock() would clamp the requested rate
> > > between min and max rates from the rate request. Normally these fields
> > > would be filled by clk_core_get_boundaries() called from
> > > clk_round_rate().
> > >
> > > However clk_gfx3d_determine_rate() uses a manually crafted rate request,
> > > which did not have these fields filled. Thus the requested frequency
> > > would be clamped to 0, resulting in weird frequencies being requested
> > > from the hardware.
> > >
> > > Fix this by filling min_rate and max_rate to the values valid for the
> > > respective PLLs (0 and ULONG_MAX).
> > >
> > > Fixes: 948fb0969eae ("clk: Always clamp the rounded rate")
> > > Signed-off-by: Dmitry Baryshkov
> > > ---
> >
> > I hope there aren't others like this lurking.
>
> The problem is larger than that (even though I overlooked this
> particular issue), and addressed partially by patches 12-19 here:
> https://lore.kernel.org/linux-clk/20220408091037.2041955-1-max...@cerno.tech/
>
> I wanted to have your feedback before fixing the relevant drivers, but
> these are:
Ok. Let me move the conversation over to that thread. I'm applying this
to clk-fixes.
Re: [Freedreno] [PATCH v9 4/4] drm/msm/dp: Support the eDP modes given by panel
Quoting Sankeerth Billakanti (2022-04-22 02:11:06) > The eDP controller does not have a reliable way keep panel > powered on to read the sink capabilities. So, the controller > driver cannot validate if a mode can be supported by the > source. We will rely on the panel driver to populate only > the supported modes for now. > > Signed-off-by: Sankeerth Billakanti > Reviewed-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
Re: [Freedreno] [PATCH v9 3/4] drm/msm/dp: wait for hpd high before aux transaction
Quoting Sankeerth Billakanti (2022-04-22 02:11:05) > The source device should ensure the sink is ready before proceeding to > read the sink capability or perform any aux transactions. The sink > will indicate its readiness by asserting the HPD line. The controller > driver needs to wait for the hpd line to be asserted by the sink before > it performs any aux transactions. > > The eDP sink is assumed to be always connected. It needs power from the > source and its HPD line will be asserted only after the panel is powered > on. The panel power will be enabled from the panel-edp driver and only > after that, the hpd line will be asserted. > > Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd > line gets asserted to indicate the sink is connected and ready. Hence > there is no need to wait for the hpd line to be asserted for a DP sink. > > Signed-off-by: Sankeerth Billakanti > Reviewed-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
Re: [Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Quoting Sankeerth Billakanti (2022-04-22 02:11:04) > The panel-edp enables the eDP panel power during probe, get_modes > and pre-enable. The eDP connect and disconnect interrupts for the eDP/DP > controller are directly dependent on panel power. As eDP display can be > assumed as always connected, the controller driver can skip the eDP > connect and disconnect interrupts. Any disruption in the link status > will be indicated via the IRQ_HPD interrupts. > > So, the eDP controller driver can just enable the IRQ_HPD and replug > interrupts. The DP controller driver still needs to enable all the > interrupts. > > Signed-off-by: Sankeerth Billakanti The sprinkling of if conditions and manual driving of the DP plug/unplug state machine is pretty convoluted. To make it better the driver needs an overhaul. Anyway, it looks mostly fine to me except for this replug interrupt question below. Otherwise Reviewed-by: Stephen Boyd > int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog) > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 055681a..dea4de9 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1096,6 +1097,13 @@ static void dp_display_config_hpd(struct > dp_display_private *dp) > dp_display_host_init(dp); > dp_catalog_ctrl_hpd_config(dp->catalog); > > + /* Enable plug and unplug interrupts only for external DisplayPort */ > + if (!dp->dp_display.is_edp) > + dp_catalog_hpd_config_intr(dp->catalog, > + DP_DP_HPD_PLUG_INT_MASK | > + DP_DP_HPD_UNPLUG_INT_MASK, > + true); > + It seems like only the plug and unplug is enabled for DP here. Is replug enabled for eDP when it shouldn't be? > /* Enable interrupt first time > * we are leaving dp clocks on during disconnect > * and never disable interrupt
Re: [Freedreno] [PATCH v9 1/4] drm/msm/dp: Add eDP support via aux_bus
Quoting Sankeerth Billakanti (2022-04-22 02:11:03)
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index d7a19d6..055681a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
Some nitpicks
Reviewed-by: Stephen Boyd
> @@ -1508,7 +1509,8 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
>
> dp_hpd_event_setup(dp);
>
> - dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
> + if (!dp_display->is_edp)
> + dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
Did it turn out that in fact DP isn't ready still to setup even after
delaying the irq?
> }
>
> void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
> @@ -1530,6 +1532,65 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display,
> struct drm_minor *minor)
> }
> }
>
> +static int dp_display_get_next_bridge(struct msm_dp *dp)
> +{
> + int rc;
> + struct dp_display_private *dp_priv;
> + struct device_node *aux_bus;
> + struct device *dev;
> +
> + dp_priv = container_of(dp, struct dp_display_private, dp_display);
> + dev = _priv->pdev->dev;
> + aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
> +
> + if (aux_bus && dp->is_edp) {
> + dp_display_host_init(dp_priv);
> + dp_catalog_ctrl_hpd_config(dp_priv->catalog);
> + dp_display_host_phy_init(dp_priv);
> + enable_irq(dp_priv->irq);
> +
> + /*
> +* The code below assumes that the panel will finish probing
> +* by the time devm_of_dp_aux_populate_ep_devices() returns.
> +* This isn't a great assumption since it will fail if the
> +* panel driver is probed asynchronously but is the best we
> +* can do without a bigger driver reorganization.
> +*/
> + rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
> + of_node_put(aux_bus);
> + if (rc)
> + goto error;
> + } else if (dp->is_edp) {
> + DRM_ERROR("eDP aux_bus not found\n");
> + return -ENODEV;
> + }
> +
> + /*
> +* External bridges are mandatory for eDP interfaces: one has to
> +* provide at least an eDP panel (which gets wrapped into
> panel-bridge).
> +*
> +* For DisplayPort interfaces external bridges are optional, so
> +* silently ignore an error if one is not present (-ENODEV).
> +*/
> + rc = dp_parser_find_next_bridge(dp_priv->parser);
> + if (!dp->is_edp && rc == -ENODEV)
> + return 0;
> + else if (rc)
Just an if because there's a return above.
> + goto error;
> +
> + dp->next_bridge = dp_priv->parser->next_bridge;
In which case it almost feels like it could be written
if (!dp->is_edp && rc == -ENODEV)
return 0;
if (!rc) {
dp->next_bridge = dp_priv->parser->next_bridge;
return 0;
}
error:
if (dp->is_edp) {
but I'm not worried either way, besides removing the else from the else-if.
> +
> + return 0;
> +
> +error:
> + if (dp->is_edp) {
> + disable_irq(dp_priv->irq);
> + dp_display_host_phy_exit(dp_priv);
> + dp_display_host_deinit(dp_priv);
> + }
> + return rc;
> +}
> +
> int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
> struct drm_encoder *encoder)
> {
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index d371bae..950416c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -125,7 +125,7 @@ struct dp_parser {
> u32 max_dp_lanes;
> struct drm_bridge *next_bridge;
>
> - int (*parse)(struct dp_parser *parser, int connector_type);
> + int (*parse)(struct dp_parser *parser);
> };
>
> /**
> @@ -141,4 +141,15 @@ struct dp_parser {
> */
> struct dp_parser *dp_parser_get(struct platform_device *pdev);
>
> +/**
> + * dp_parser_find_next_bridge() - find an additional bridge to DP
> + *
> + * @parser: dp_parser data from client
> + * return: 0 if able to get the bridge else return an error code
return comes after the description below. Also should be capitalized.
You can check this by compiling with W=1 I believe, or run the
kernel doc format script on the file..
> + *
> + * This function is used to find any additional bridge attached to
> + * the DP controller. The eDP interface requires a panel bridge.
Return: 0 if able to get the bridge, otherwise negative errno for failure
> + */
> +int dp_parser_find_next_bridge(struct dp_parser *parser);
> +
Re: [Freedreno] [PATCH] drm/msm/dp: move add fail safe mode to dp_connector_get_mode()
On 4/22/2022 5:07 PM, Dmitry Baryshkov wrote:
On 23/04/2022 02:45, Kuogee Hsieh wrote:
Current DP driver implementation has adding safe mode done at
dp_hpd_plug_handle() which is expected to be executed under event
thread context.
However there is possible circular locking happen (see blow stack trace)
after edp driver call dp_hpd_plug_handle() from dp_bridge_enable() which
is executed under drm_thread context.
To break this circular locking, this patch have safe mode added at
dp_connector_get_mode() which is executed under drm thread context.
Therefore no lock acquired required for >mode_config.mutex while
adding fail safe mode since it has been hold by drm thread already.
==
WARNING: possible circular locking dependency detected
5.15.35-lockdep #6 Tainted: G W
--
frecon/429 is trying to acquire lock:
ff808dc3c4e8 (>mode_config.mutex){+.+.}-{3:3}, at:
dp_panel_add_fail_safe_mode+0x4c/0xa0
but task is already holding lock:
ff808dc441e0 (>commit_lock[i]){+.+.}-{3:3}, at:
lock_crtcs+0xb4/0x124
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>commit_lock[i]){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
lock_crtcs+0xb4/0x124
msm_atomic_commit_tail+0x330/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #2 (crtc_ww_class_mutex){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
ww_mutex_lock+0xb8/0x278
modeset_lock+0x304/0x4ac
drm_modeset_lock+0x4c/0x7c
drmm_mode_config_init+0x4a8/0xc50
msm_drm_init+0x274/0xac0
msm_drm_bind+0x20/0x2c
try_to_bring_up_master+0x3dc/0x470
__component_add+0x18c/0x3c0
component_add+0x1c/0x28
dp_display_probe+0x954/0xa98
platform_probe+0x124/0x15c
really_probe+0x1b0/0x5f8
__driver_probe_device+0x174/0x20c
driver_probe_device+0x70/0x134
__device_attach_driver+0x130/0x1d0
bus_for_each_drv+0xfc/0x14c
__device_attach+0x1bc/0x2bc
device_initial_probe+0x1c/0x28
bus_probe_device+0x94/0x178
deferred_probe_work_func+0x1a4/0x1f0
process_one_work+0x5d4/0x9dc
worker_thread+0x898/0xccc
kthread+0x2d4/0x3d4
ret_from_fork+0x10/0x20
-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
ww_acquire_init+0x1c4/0x2c8
drm_modeset_acquire_init+0x44/0xc8
drm_helper_probe_single_connector_modes+0xb0/0x12dc
drm_mode_getconnector+0x5dc/0xfe8
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #0 (>mode_config.mutex){+.+.}-{3:3}:
__lock_acquire+0x2650/0x672c
lock_acquire+0x1b4/0x4ac
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
dp_panel_add_fail_safe_mode+0x4c/0xa0
dp_hpd_plug_handle+0x1f0/0x280
dp_bridge_enable+0x94/0x2b8
drm_atomic_bridge_chain_enable+0x11c/0x168
drm_atomic_helper_commit_modeset_enables+0x500/0x740
msm_atomic_commit_tail+0x3e4/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 6 --
drivers/gpu/drm/msm/dp/dp_panel.c | 23 +--
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 92cd50f..01453db 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -555,12 +555,6 @@ static int dp_hpd_plug_handle(struct
dp_display_private *dp, u32 data)
Re: [Freedreno] [PATCH] drm/msm/dp: move add fail safe mode to dp_connector_get_mode()
On 23/04/2022 02:45, Kuogee Hsieh wrote:
Current DP driver implementation has adding safe mode done at
dp_hpd_plug_handle() which is expected to be executed under event
thread context.
However there is possible circular locking happen (see blow stack trace)
after edp driver call dp_hpd_plug_handle() from dp_bridge_enable() which
is executed under drm_thread context.
To break this circular locking, this patch have safe mode added at
dp_connector_get_mode() which is executed under drm thread context.
Therefore no lock acquired required for >mode_config.mutex while
adding fail safe mode since it has been hold by drm thread already.
==
WARNING: possible circular locking dependency detected
5.15.35-lockdep #6 Tainted: GW
--
frecon/429 is trying to acquire lock:
ff808dc3c4e8 (>mode_config.mutex){+.+.}-{3:3}, at:
dp_panel_add_fail_safe_mode+0x4c/0xa0
but task is already holding lock:
ff808dc441e0 (>commit_lock[i]){+.+.}-{3:3}, at: lock_crtcs+0xb4/0x124
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>commit_lock[i]){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
lock_crtcs+0xb4/0x124
msm_atomic_commit_tail+0x330/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #2 (crtc_ww_class_mutex){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
ww_mutex_lock+0xb8/0x278
modeset_lock+0x304/0x4ac
drm_modeset_lock+0x4c/0x7c
drmm_mode_config_init+0x4a8/0xc50
msm_drm_init+0x274/0xac0
msm_drm_bind+0x20/0x2c
try_to_bring_up_master+0x3dc/0x470
__component_add+0x18c/0x3c0
component_add+0x1c/0x28
dp_display_probe+0x954/0xa98
platform_probe+0x124/0x15c
really_probe+0x1b0/0x5f8
__driver_probe_device+0x174/0x20c
driver_probe_device+0x70/0x134
__device_attach_driver+0x130/0x1d0
bus_for_each_drv+0xfc/0x14c
__device_attach+0x1bc/0x2bc
device_initial_probe+0x1c/0x28
bus_probe_device+0x94/0x178
deferred_probe_work_func+0x1a4/0x1f0
process_one_work+0x5d4/0x9dc
worker_thread+0x898/0xccc
kthread+0x2d4/0x3d4
ret_from_fork+0x10/0x20
-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
ww_acquire_init+0x1c4/0x2c8
drm_modeset_acquire_init+0x44/0xc8
drm_helper_probe_single_connector_modes+0xb0/0x12dc
drm_mode_getconnector+0x5dc/0xfe8
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #0 (>mode_config.mutex){+.+.}-{3:3}:
__lock_acquire+0x2650/0x672c
lock_acquire+0x1b4/0x4ac
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
dp_panel_add_fail_safe_mode+0x4c/0xa0
dp_hpd_plug_handle+0x1f0/0x280
dp_bridge_enable+0x94/0x2b8
drm_atomic_bridge_chain_enable+0x11c/0x168
drm_atomic_helper_commit_modeset_enables+0x500/0x740
msm_atomic_commit_tail+0x3e4/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 6 --
drivers/gpu/drm/msm/dp/dp_panel.c | 23 +--
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 92cd50f..01453db 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -555,12 +555,6 @@ static int dp_hpd_plug_handle(struct dp_display_private
*dp, u32 data)
mutex_unlock(>event_mutex);
- /*
-* add fail safe
Re: [Freedreno] [PATCH] drm/msm/dp: move add fail safe mode to dp_connector_get_mode()
Quoting Kuogee Hsieh (2022-04-22 16:45:23)
> Current DP driver implementation has adding safe mode done at
> dp_hpd_plug_handle() which is expected to be executed under event
> thread context.
>
> However there is possible circular locking happen (see blow stack trace)
> after edp driver call dp_hpd_plug_handle() from dp_bridge_enable() which
> is executed under drm_thread context.
>
> To break this circular locking, this patch have safe mode added at
> dp_connector_get_mode() which is executed under drm thread context.
> Therefore no lock acquired required for >mode_config.mutex while
> adding fail safe mode since it has been hold by drm thread already.
Reported-by: Douglas Anderson
Fixes: 8b2c181e3dcf ("drm/msm/dp: add fail safe mode outside of
event_mutex context")
Reviewed-by: Stephen Boyd
[Freedreno] [PATCH] drm/msm/dp: move add fail safe mode to dp_connector_get_mode()
Current DP driver implementation has adding safe mode done at
dp_hpd_plug_handle() which is expected to be executed under event
thread context.
However there is possible circular locking happen (see blow stack trace)
after edp driver call dp_hpd_plug_handle() from dp_bridge_enable() which
is executed under drm_thread context.
To break this circular locking, this patch have safe mode added at
dp_connector_get_mode() which is executed under drm thread context.
Therefore no lock acquired required for >mode_config.mutex while
adding fail safe mode since it has been hold by drm thread already.
==
WARNING: possible circular locking dependency detected
5.15.35-lockdep #6 Tainted: GW
--
frecon/429 is trying to acquire lock:
ff808dc3c4e8 (>mode_config.mutex){+.+.}-{3:3}, at:
dp_panel_add_fail_safe_mode+0x4c/0xa0
but task is already holding lock:
ff808dc441e0 (>commit_lock[i]){+.+.}-{3:3}, at: lock_crtcs+0xb4/0x124
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>commit_lock[i]){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
lock_crtcs+0xb4/0x124
msm_atomic_commit_tail+0x330/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #2 (crtc_ww_class_mutex){+.+.}-{3:3}:
__mutex_lock_common+0x174/0x1a64
ww_mutex_lock+0xb8/0x278
modeset_lock+0x304/0x4ac
drm_modeset_lock+0x4c/0x7c
drmm_mode_config_init+0x4a8/0xc50
msm_drm_init+0x274/0xac0
msm_drm_bind+0x20/0x2c
try_to_bring_up_master+0x3dc/0x470
__component_add+0x18c/0x3c0
component_add+0x1c/0x28
dp_display_probe+0x954/0xa98
platform_probe+0x124/0x15c
really_probe+0x1b0/0x5f8
__driver_probe_device+0x174/0x20c
driver_probe_device+0x70/0x134
__device_attach_driver+0x130/0x1d0
bus_for_each_drv+0xfc/0x14c
__device_attach+0x1bc/0x2bc
device_initial_probe+0x1c/0x28
bus_probe_device+0x94/0x178
deferred_probe_work_func+0x1a4/0x1f0
process_one_work+0x5d4/0x9dc
worker_thread+0x898/0xccc
kthread+0x2d4/0x3d4
ret_from_fork+0x10/0x20
-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
ww_acquire_init+0x1c4/0x2c8
drm_modeset_acquire_init+0x44/0xc8
drm_helper_probe_single_connector_modes+0xb0/0x12dc
drm_mode_getconnector+0x5dc/0xfe8
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
-> #0 (>mode_config.mutex){+.+.}-{3:3}:
__lock_acquire+0x2650/0x672c
lock_acquire+0x1b4/0x4ac
__mutex_lock_common+0x174/0x1a64
mutex_lock_nested+0x98/0xac
dp_panel_add_fail_safe_mode+0x4c/0xa0
dp_hpd_plug_handle+0x1f0/0x280
dp_bridge_enable+0x94/0x2b8
drm_atomic_bridge_chain_enable+0x11c/0x168
drm_atomic_helper_commit_modeset_enables+0x500/0x740
msm_atomic_commit_tail+0x3e4/0x748
commit_tail+0x19c/0x278
drm_atomic_helper_commit+0x1dc/0x1f0
drm_atomic_commit+0xc0/0xd8
drm_atomic_helper_set_config+0xb4/0x134
drm_mode_setcrtc+0x688/0x1248
drm_ioctl_kernel+0x1e4/0x338
drm_ioctl+0x3a4/0x684
__arm64_sys_ioctl+0x118/0x154
invoke_syscall+0x78/0x224
el0_svc_common+0x178/0x200
do_el0_svc+0x94/0x13c
el0_svc+0x5c/0xec
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 6 --
drivers/gpu/drm/msm/dp/dp_panel.c | 23 +--
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 92cd50f..01453db 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -555,12 +555,6 @@ static int dp_hpd_plug_handle(struct dp_display_private
*dp, u32 data)
mutex_unlock(>event_mutex);
- /*
-* add fail safe mode outside event_mutex scope
-* to avoid potiential circular lock with drm thread
-*/
-
Re: [Freedreno] [PATCH v2 04/17] drm/msm/dpu: add writeback blocks to the sm8250 DPU catalog
Hi Liviu
Thank you for the feedback.
I have fixed the order of copyright years in all the changes in the next
version.
Thanks
Abhinav
On 4/21/2022 5:16 AM, Liviu Dudau wrote:
On Tue, Apr 19, 2022 at 06:45:56PM -0700, Abhinav Kumar wrote:
Add writeback blocks to the sm8250 DPU hardware catalog. Other
chipsets support writeback too but add it to sm8250 to prototype
the feature so that it can be easily extended to other chipsets.
changes in v2:
- none
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 74 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 66 ++-
2 files changed, 138 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b0a0ef7..bcb5273 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
Hi Abhinav,
Nit: Order should be historical (i.e. QIC copyright comes last). Comment
applies to
all other copyright years additions.
Best regards,
Liviu
*/
#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
@@ -120,6 +121,16 @@
BIT(MDP_AD4_0_INTR) | \
BIT(MDP_AD4_1_INTR))
+#define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
+BIT(DPU_WB_UBWC) | \
+BIT(DPU_WB_YUV_CONFIG) | \
+BIT(DPU_WB_PIPE_ALPHA) | \
+BIT(DPU_WB_XY_ROI_OFFSET) | \
+BIT(DPU_WB_QOS) | \
+BIT(DPU_WB_QOS_8LVL) | \
+BIT(DPU_WB_CDP) | \
+BIT(DPU_WB_INPUT_CTRL))
+
#define DEFAULT_PIXEL_RAM_SIZE(50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH2048
#define DEFAULT_DPU_OUTPUT_LINE_WIDTH 2560
@@ -211,6 +222,40 @@ static const u32 rotation_v2_formats[] = {
/* TODO add formats after validation */
};
+static const uint32_t wb2_formats[] = {
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XBGR,
+};
+
/*
* DPU sub blocks config
*/
@@ -448,6 +493,8 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
+ .clk_ctrls[DPU_CLK_CTRL_WB2] = {
+ .reg_off = 0x3B8, .bit_off = 24},
},
};
@@ -1235,6 +1282,29 @@ static const struct dpu_intf_cfg qcm2290_intf[] = {
};
/*
+ * Writeback blocks config
+ */
+#define WB_BLK(_name, _id, _base, _features, _clk_ctrl, \
+ __xin_id, vbif_id, _reg, _wb_done_bit) \
+ { \
+ .name = _name, .id = _id, \
+ .base = _base, .len = 0x2c8, \
+ .features = _features, \
+ .format_list = wb2_formats, \
+ .num_formats = ARRAY_SIZE(wb2_formats), \
+ .clk_ctrl = _clk_ctrl, \
+ .xin_id = __xin_id, \
+ .vbif_idx = vbif_id, \
+ .maxlinewidth = DEFAULT_DPU_LINE_WIDTH, \
+ .intr_wb_done = DPU_IRQ_IDX(_reg, _wb_done_bit) \
+ }
+
+static const struct dpu_wb_cfg sm8250_wb[] = {
+ WB_BLK("wb_2", WB_2, 0x65000, WB_SM8250_MASK, DPU_CLK_CTRL_WB2, 6,
+ VBIF_RT, MDP_SSPP_TOP0_INTR, 4),
+};
+
+/*
* VBIF sub blocks config
*/
/* VBIF QOS remap */
@@ -1832,6 +1902,8 @@ static void
[Freedreno] [PATCH v4 20/20] drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder
Change the DRM traces to include both the intf_mode
and wb_idx similar to the DRM prints in the previous change.
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 -
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 26 ++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 6d093cf..3bd1790 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1354,8 +1354,9 @@ static void dpu_encoder_frame_done_callback(
* suppress frame_done without waiter,
* likely autorefresh
*/
- trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc),
- event, ready_phys->intf_idx);
+ trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc),
event,
+
dpu_encoder_helper_get_intf_type(ready_phys->intf_mode),
+ ready_phys->intf_idx,
ready_phys->wb_idx);
return;
}
@@ -1433,9 +1434,11 @@ static void _dpu_encoder_trigger_flush(struct
drm_encoder *drm_enc,
if (ctl->ops.get_pending_flush)
ret = ctl->ops.get_pending_flush(ctl);
- trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx,
- pending_kickoff_cnt, ctl->idx,
- extra_flush_bits, ret);
+ trace_dpu_enc_trigger_flush(DRMID(drm_enc),
+ dpu_encoder_helper_get_intf_type(phys->intf_mode),
+ phys->intf_idx, phys->wb_idx,
+ pending_kickoff_cnt, ctl->idx,
+ extra_flush_bits, ret);
}
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 58b411f..1106d44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -380,20 +380,26 @@ TRACE_EVENT(dpu_enc_rc,
);
TRACE_EVENT(dpu_enc_frame_done_cb_not_busy,
- TP_PROTO(uint32_t drm_id, u32 event, enum dpu_intf intf_idx),
- TP_ARGS(drm_id, event, intf_idx),
+ TP_PROTO(uint32_t drm_id, u32 event, char *intf_mode, enum dpu_intf
intf_idx,
+ enum dpu_wb wb_idx),
+ TP_ARGS(drm_id, event, intf_mode, intf_idx, wb_idx),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
__field(u32,event )
+ __string( intf_mode_str, intf_mode )
__field(enum dpu_intf, intf_idx)
+ __field(enum dpu_wb, wb_idx)
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->event = event;
+ __assign_str(intf_mode_str, intf_mode);
__entry->intf_idx = intf_idx;
+ __entry->wb_idx = wb_idx;
),
- TP_printk("id=%u, event=%u, intf=%d", __entry->drm_id, __entry->event,
- __entry->intf_idx)
+ TP_printk("id=%u, event=%u, intf_mode=%s intf=%d wb=%d",
__entry->drm_id,
+ __entry->event, __get_str(intf_mode_str),
+ __entry->intf_idx, __entry->wb_idx)
);
TRACE_EVENT(dpu_enc_frame_done_cb,
@@ -415,14 +421,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
);
TRACE_EVENT(dpu_enc_trigger_flush,
- TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx,
+ TP_PROTO(uint32_t drm_id, char *intf_mode, enum dpu_intf intf_idx, enum
dpu_wb wb_idx,
int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits,
u32 pending_flush_ret),
- TP_ARGS(drm_id, intf_idx, pending_kickoff_cnt, ctl_idx,
+ TP_ARGS(drm_id, intf_mode, intf_idx, pending_kickoff_cnt, ctl_idx,
extra_flush_bits, pending_flush_ret),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
+ __string( intf_mode_str, intf_mode )
__field(enum dpu_intf, intf_idx)
+ __field(enum dpu_wb, wb_idx)
__field(int,pending_kickoff_cnt )
__field(int,ctl_idx )
__field(u32,extra_flush_bits)
@@ -430,15 +438,17 @@ TRACE_EVENT(dpu_enc_trigger_flush,
),
TP_fast_assign(
__entry->drm_id = drm_id;
+ __assign_str(intf_mode_str, intf_mode);
__entry->intf_idx = intf_idx;
+ __entry->wb_idx = wb_idx;
__entry->pending_kickoff_cnt =
[Freedreno] [PATCH v4 18/20] drm/msm/dpu: add writeback blocks to the display snapshot
Add writeback block information while capturing the display snapshot. Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 9a406e1..b18bff8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -945,6 +945,11 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k msm_disp_snapshot_add_block(disp_state, cat->mixer[i].len, dpu_kms->mmio + cat->mixer[i].base, "lm_%d", i); + /* dump WB sub-blocks HW regs info */ + for (i = 0; i < cat->wb_count; i++) + msm_disp_snapshot_add_block(disp_state, cat->wb[i].len, + dpu_kms->mmio + cat->wb[i].base, "wb_%d", i); + msm_disp_snapshot_add_block(disp_state, top->hw.length, dpu_kms->mmio + top->hw.blk_off, "top"); -- 2.7.4
[Freedreno] [PATCH v4 19/20] drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder
Add wb_idx to existing DRM prints in dpu_encoder and also
print the intf_mode so that its clear that for any INTF_CMD/VID
there will be a valid intf_idx and any INTF_WB_* there will be a
valid wb_idx.
Update the debugfs to add the same information. Here is a sample
output with this change:
root:/sys/kernel/debug/dri/0/encoder31# cat status
intf:1 wb:-1 vsync: 31 underrun: 0mode: INTF_MODE_VIDEO
root:/sys/kernel/debug/dri/0/encoder33# cat status
intf:-1 wb:2 vsync: 7 underrun: 0mode: INTF_MODE_WB_LINE
Also remove DPU_DEBUG_PHYS macros as its unused because the
respective dpu_encoder_phys_* files have their own macros.
changes in v2:
- use switch case instead of if/else-if for get_intf_type
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 52 +
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 90ef807..6d093cf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -38,18 +38,6 @@
#define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
-#define DPU_DEBUG_PHYS(p, fmt, ...) DRM_DEBUG_ATOMIC("enc%d intf%d pp%d " fmt,\
- (p) ? (p)->parent->base.id : -1, \
- (p) ? (p)->intf_idx - INTF_0 : -1, \
- (p) ? ((p)->hw_pp ? (p)->hw_pp->idx - PINGPONG_0 : -1) : -1, \
- ##__VA_ARGS__)
-
-#define DPU_ERROR_PHYS(p, fmt, ...) DPU_ERROR("enc%d intf%d pp%d " fmt,\
- (p) ? (p)->parent->base.id : -1, \
- (p) ? (p)->intf_idx - INTF_0 : -1, \
- (p) ? ((p)->hw_pp ? (p)->hw_pp->idx - PINGPONG_0 : -1) : -1, \
- ##__VA_ARGS__)
-
/*
* Two to anticipate panels that can do cmd/vid dynamic switching
* plan is to create all possible physical encoder types, and switch between
@@ -263,12 +251,30 @@ static void _dpu_encoder_setup_dither(struct
dpu_hw_pingpong *hw_pp, unsigned bp
hw_pp->ops.setup_dither(hw_pp, _cfg);
}
+static char *dpu_encoder_helper_get_intf_type(enum dpu_intf_mode intf_mode)
+{
+ switch (intf_mode) {
+ case INTF_MODE_VIDEO:
+ return "INTF_MODE_VIDEO";
+ case INTF_MODE_CMD:
+ return "INTF_MODE_CMD";
+ case INTF_MODE_WB_BLOCK:
+ return "INTF_MODE_WB_BLOCK";
+ case INTF_MODE_WB_LINE:
+ return "INTF_MODE_WB_LINE";
+ default:
+ return "INTF_MODE_UNKNOWN";
+ }
+}
+
void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx)
{
- DRM_ERROR("irq timeout id=%u, intf=%d, pp=%d, intr=%d\n",
- DRMID(phys_enc->parent), phys_enc->intf_idx - INTF_0,
- phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
+ DRM_ERROR("irq timeout id=%u, intf_mode=%s intf=%d wb=%d, pp=%d,
intr=%d\n",
+ DRMID(phys_enc->parent),
+ dpu_encoder_helper_get_intf_type(phys_enc->intf_mode),
+ phys_enc->intf_idx - INTF_0, phys_enc->wb_idx - WB_0,
+ phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
if (phys_enc->parent_ops->handle_frame_done)
phys_enc->parent_ops->handle_frame_done(
@@ -2049,22 +2055,12 @@ static int _dpu_encoder_status_show(struct seq_file *s,
void *data)
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
- seq_printf(s, "intf:%dvsync:%8d underrun:%8d",
- phys->intf_idx - INTF_0,
+ seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d",
+ phys->intf_idx - INTF_0, phys->wb_idx - WB_0,
atomic_read(>vsync_cnt),
atomic_read(>underrun_cnt));
- switch (phys->intf_mode) {
- case INTF_MODE_VIDEO:
- seq_puts(s, "mode: video\n");
- break;
- case INTF_MODE_CMD:
- seq_puts(s, "mode: command\n");
- break;
- default:
- seq_puts(s, "mode: ???\n");
- break;
- }
+ seq_printf(s, "mode: %s\n",
dpu_encoder_helper_get_intf_type(phys->intf_mode));
}
mutex_unlock(_enc->enc_lock);
--
2.7.4
[Freedreno] [PATCH v4 17/20] drm/msm/dpu: gracefully handle null fb commits for writeback
kms_writeback test cases also verify with a null fb for the
writeback connector job. In addition there are also other
commit paths which can result in kickoffs without a valid
framebuffer like while closing the fb which results in the
callback to drm_atomic_helper_dirtyfb() which internally
triggers a commit.
Add protection in the dpu driver to ensure that commits for
writeback encoders without a valid fb are gracefully skipped.
changes in v2:
- rename dpu_encoder_has_valid_fb to dpu_encoder_is_valid_for_commit
changes in v3:
- none
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 9 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 12
5 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 7763558..d65e124 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -869,6 +869,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
DPU_ATRACE_BEGIN("crtc_commit");
+ drm_for_each_encoder_mask(encoder, crtc->dev,
+ crtc->state->encoder_mask) {
+ if (!dpu_encoder_is_valid_for_commit(encoder)) {
+ DRM_DEBUG_ATOMIC("invalid FB not kicking off crtc\n");
+ goto end;
+ }
+ }
/*
* Encoder will flush/start now, unless it has a tx pending. If so, it
* may delay and flush at an irq event (e.g. ppdone)
@@ -891,6 +898,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
dpu_encoder_kickoff(encoder);
reinit_completion(_crtc->frame_done_comp);
+
+end:
DPU_ATRACE_END("crtc_commit");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2d79002..90ef807 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1862,6 +1862,27 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder
*drm_enc)
dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc);
}
+bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc;
+ unsigned int i;
+ struct dpu_encoder_phys *phys;
+
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_VIRTUAL) {
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ phys = dpu_enc->phys_encs[i];
+ if (phys->ops.is_valid_for_commit &&
!phys->ops.is_valid_for_commit(phys)) {
+ DPU_DEBUG("invalid FB not kicking off\n");
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 6ceec1d..781d41c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -196,4 +196,10 @@ void dpu_encoder_prepare_wb_job(struct drm_encoder
*drm_enc,
void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
struct drm_writeback_job *job);
+/**
+ * dpu_encoder_is_valid_for_commit - check if encode has valid parameters for
commit.
+ * @drm_enc:Pointer to drm encoder structure
+ */
+bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc);
+
#endif /* __DPU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index bed4523..f2af07d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -142,6 +142,7 @@ struct dpu_encoder_phys_ops {
struct drm_writeback_job *job);
void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc,
struct drm_writeback_job *job);
+ bool (*is_valid_for_commit)(struct dpu_encoder_phys *phys_enc);
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 4ab2699..cb5c7da 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -667,6 +667,16 @@ static void dpu_encoder_phys_wb_cleanup_wb_job(struct
dpu_encoder_phys *phys_enc
wb_enc->wb_conn = NULL;
}
+static bool dpu_encoder_phys_wb_is_valid_for_commit(struct dpu_encoder_phys
*phys_enc)
+{
+ struct
[Freedreno] [PATCH v4 13/20] drm/msm/dpu: move _dpu_plane_get_qos_lut to dpu_hw_util file
_dpu_plane_get_qos_lut() is not specific to just dpu_plane.
It can take any fill level and return the LUT matching it.
This can be used even for other modules like dpu_writeback.
Move _dpu_plane_get_qos_lut() to the common dpu_hw_util file
and rename it to _dpu_hw_get_qos_lut().
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 25 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 +--
3 files changed, 30 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
index aad8511..512316f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
@@ -422,3 +422,28 @@ void dpu_hw_csc_setup(struct dpu_hw_blk_reg_map *c,
DPU_REG_WRITE(c, csc_reg_off + 0x3c, data->csc_post_bv[1]);
DPU_REG_WRITE(c, csc_reg_off + 0x40, data->csc_post_bv[2]);
}
+
+/**
+ * _dpu_hw_get_qos_lut - get LUT mapping based on fill level
+ * @tbl: Pointer to LUT table
+ * @total_fl: fill level
+ * Return: LUT setting corresponding to the fill level
+ */
+u64 _dpu_hw_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
+ u32 total_fl)
+{
+ int i;
+
+ if (!tbl || !tbl->nentry || !tbl->entries)
+ return 0;
+
+ for (i = 0; i < tbl->nentry; i++)
+ if (total_fl <= tbl->entries[i].fl)
+ return tbl->entries[i].lut;
+
+ /* if last fl is zero, use as default */
+ if (!tbl->entries[i-1].fl)
+ return tbl->entries[i-1].lut;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index a200df1..e4a65eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -9,6 +9,7 @@
#include
#include
#include "dpu_hw_mdss.h"
+#include "dpu_hw_catalog.h"
#define REG_MASK(n) ((BIT(n)) - 1)
@@ -339,4 +340,7 @@ void dpu_hw_csc_setup(struct dpu_hw_blk_reg_map *c,
u32 csc_reg_off,
const struct dpu_csc_cfg *data, bool csc10);
+u64 _dpu_hw_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
+ u32 total_fl);
+
#endif /* _DPU_HW_UTIL_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 08b8c64..9d2f036 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -280,31 +280,6 @@ static int _dpu_plane_calc_fill_level(struct drm_plane
*plane,
}
/**
- * _dpu_plane_get_qos_lut - get LUT mapping based on fill level
- * @tbl: Pointer to LUT table
- * @total_fl: fill level
- * Return: LUT setting corresponding to the fill level
- */
-static u64 _dpu_plane_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
- u32 total_fl)
-{
- int i;
-
- if (!tbl || !tbl->nentry || !tbl->entries)
- return 0;
-
- for (i = 0; i < tbl->nentry; i++)
- if (total_fl <= tbl->entries[i].fl)
- return tbl->entries[i].lut;
-
- /* if last fl is zero, use as default */
- if (!tbl->entries[i-1].fl)
- return tbl->entries[i-1].lut;
-
- return 0;
-}
-
-/**
* _dpu_plane_set_qos_lut - set QoS LUT of the given plane
* @plane: Pointer to drm plane
* @fb:Pointer to framebuffer associated with the
given plane
@@ -333,7 +308,7 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
lut_usage = DPU_QOS_LUT_USAGE_MACROTILE;
}
- qos_lut = _dpu_plane_get_qos_lut(
+ qos_lut = _dpu_hw_get_qos_lut(
>catalog->perf.qos_lut_tbl[lut_usage], total_fl);
trace_dpu_perf_set_qos_luts(pdpu->pipe - SSPP_VIG0,
--
2.7.4
[Freedreno] [PATCH v4 15/20] drm/msm/dpu: add the writeback connector layer
Introduce the dpu_writeback module which serves as the
interface between dpu operations and the drm_writeback.
This module manages the connector related operations for
dpu writeback.
changes in v2:
- start using drm_writeback_connector_init_with_encoder()
- drop unnecessary arguments from dpu_writeback_init()
- rebase on msm-next tip and remove usage of priv->connectors
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 68 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h | 25 ++
3 files changed, 94 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 0387f22..66395ee 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -80,6 +80,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_plane.o \
disp/dpu1/dpu_rm.o \
disp/dpu1/dpu_vbif.o \
+ disp/dpu1/dpu_writeback.o
msm-$(CONFIG_DRM_MSM_MDSS) += \
msm_mdss.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
new file mode 100644
index 000..526d884
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include "dpu_writeback.h"
+
+static int dpu_wb_conn_get_modes(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+
+ return drm_add_modes_noedid(connector, dev->mode_config.max_width,
+ dev->mode_config.max_height);
+}
+
+static const struct drm_connector_funcs dpu_wb_conn_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int dpu_wb_conn_prepare_job(struct drm_writeback_connector *connector,
+ struct drm_writeback_job *job)
+{
+ if (!job->fb)
+ return 0;
+
+ dpu_encoder_prepare_wb_job(connector->encoder, job);
+
+ return 0;
+}
+
+static void dpu_wb_conn_cleanup_job(struct drm_writeback_connector *connector,
+ struct drm_writeback_job *job)
+{
+ if (!job->fb)
+ return;
+
+ dpu_encoder_cleanup_wb_job(connector->encoder, job);
+}
+
+static const struct drm_connector_helper_funcs dpu_wb_conn_helper_funcs = {
+ .get_modes = dpu_wb_conn_get_modes,
+ .prepare_writeback_job = dpu_wb_conn_prepare_job,
+ .cleanup_writeback_job = dpu_wb_conn_cleanup_job,
+};
+
+int dpu_writeback_init(struct drm_device *dev, struct drm_encoder *enc,
+ const u32 *format_list, u32 num_formats)
+{
+ struct dpu_wb_connector *dpu_wb_conn;
+ int rc = 0;
+
+ dpu_wb_conn = devm_kzalloc(dev->dev, sizeof(*dpu_wb_conn), GFP_KERNEL);
+
+ drm_connector_helper_add(_wb_conn->base.base,
_wb_conn_helper_funcs);
+
+ /* DPU initializes the encoder and sets it up completely for writeback
+* cases and hence should use the new API
drm_writeback_connector_init_with_encoder
+* to initialize the writeback connector
+*/
+ rc = drm_writeback_connector_init_with_encoder(dev, _wb_conn->base,
enc,
+ _wb_conn_funcs, format_list, num_formats);
+
+ return rc;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h
new file mode 100644
index 000..05aff05
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _DPU_WRITEBACK_H
+#define _DPU_WRITEBACK_H
+
+#include
+#include
+#include
+#include
+
+#include "msm_drv.h"
+#include "dpu_kms.h"
+#include "dpu_encoder_phys.h"
+
+struct dpu_wb_connector {
+ struct drm_writeback_connector base;
+};
+
+int dpu_writeback_init(struct drm_device *dev, struct drm_encoder *enc,
+ const u32 *format_list, u32 num_formats);
+
+#endif /*_DPU_WRITEBACK_H */
--
2.7.4
[Freedreno] [PATCH v4 16/20] drm/msm/dpu: initialize dpu encoder and connector for writeback
Initialize dpu encoder and connector for writeback if the
target supports it in the catalog.
changes in v2:
- start initialing the encoder for writeback since we
have migrated to using drm_writeback_connector_init_with_encoder()
- instead of checking for WB_2 inside _dpu_kms_initialize_writeback
call it only when its WB_2
- rebase on tip of msm-next and remove usage of priv->encoders
changes in v3:
- none
changes in v4:
- fix copyright years order
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 61 -
2 files changed, 80 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 24870eb..2d79002 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2102,7 +2102,7 @@ static void dpu_encoder_early_unregister(struct
drm_encoder *encoder)
}
static int dpu_encoder_virt_add_phys_encs(
- u32 display_caps,
+ struct msm_display_info *disp_info,
struct dpu_encoder_virt *dpu_enc,
struct dpu_enc_phys_init_params *params)
{
@@ -2121,7 +2121,7 @@ static int dpu_encoder_virt_add_phys_encs(
return -EINVAL;
}
- if (display_caps & MSM_DISPLAY_CAP_VID_MODE) {
+ if (disp_info->capabilities & MSM_DISPLAY_CAP_VID_MODE) {
enc = dpu_encoder_phys_vid_init(params);
if (IS_ERR_OR_NULL(enc)) {
@@ -2134,7 +2134,7 @@ static int dpu_encoder_virt_add_phys_encs(
++dpu_enc->num_phys_encs;
}
- if (display_caps & MSM_DISPLAY_CAP_CMD_MODE) {
+ if (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
enc = dpu_encoder_phys_cmd_init(params);
if (IS_ERR_OR_NULL(enc)) {
@@ -2147,6 +2147,19 @@ static int dpu_encoder_virt_add_phys_encs(
++dpu_enc->num_phys_encs;
}
+ if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
+ enc = dpu_encoder_phys_wb_init(params);
+
+ if (IS_ERR_OR_NULL(enc)) {
+ DPU_ERROR_ENC(dpu_enc, "failed to init wb enc: %ld\n",
+ PTR_ERR(enc));
+ return enc == NULL ? -EINVAL : PTR_ERR(enc);
+ }
+
+ dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
+ ++dpu_enc->num_phys_encs;
+ }
+
if (params->split_role == ENC_ROLE_SLAVE)
dpu_enc->cur_slave = enc;
else
@@ -2248,9 +2261,8 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
}
if (!ret) {
- ret =
dpu_encoder_virt_add_phys_encs(disp_info->capabilities,
-
dpu_enc,
-
_params);
+ ret = dpu_encoder_virt_add_phys_encs(disp_info,
+ dpu_enc, _params);
if (ret)
DPU_ERROR_ENC(dpu_enc, "failed to add phys
encs\n");
}
@@ -2367,8 +2379,9 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
*dev,
if (!dpu_enc)
return ERR_PTR(-ENOMEM);
+
rc = drm_encoder_init(dev, _enc->base, _encoder_funcs,
- drm_enc_mode, NULL);
+ drm_enc_mode, NULL);
if (rc) {
devm_kfree(dev->dev, dpu_enc);
return ERR_PTR(rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index c683cab..9a406e1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
* Copyright (C) 2013 Red Hat
+ * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ *
* Author: Rob Clark
*/
@@ -15,6 +17,7 @@
#include
#include
#include
+#include
#include "msm_drv.h"
#include "msm_mmu.h"
@@ -29,6 +32,7 @@
#include "dpu_kms.h"
#include "dpu_plane.h"
#include "dpu_vbif.h"
+#include "dpu_writeback.h"
#define CREATE_TRACE_POINTS
#include "dpu_trace.h"
@@ -648,6 +652,45 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
return 0;
}
+static int _dpu_kms_initialize_writeback(struct drm_device *dev,
+ struct msm_drm_private *priv, struct dpu_kms *dpu_kms,
+
[Freedreno] [PATCH v4 12/20] drm/msm/dpu: add encoder operations to prepare/cleanup wb job
add dpu encoder APIs to prepare and cleanup writeback job
for the writeback encoder. These shall be invoked from the
prepare_wb_job/cleanup_wb_job hooks of the drm_writeback
framework.
changes in v3:
- none
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 34
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 16 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 5
3 files changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d1e92d89..24870eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -913,6 +913,40 @@ static int dpu_encoder_resource_control(struct drm_encoder
*drm_enc,
return 0;
}
+void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc,
+ struct drm_writeback_job *job)
+{
+ struct dpu_encoder_virt *dpu_enc;
+ int i;
+
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+
+ if (phys->ops.prepare_wb_job)
+ phys->ops.prepare_wb_job(phys, job);
+
+ }
+}
+
+void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
+ struct drm_writeback_job *job)
+{
+ struct dpu_encoder_virt *dpu_enc;
+ int i;
+
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+
+ if (phys->ops.cleanup_wb_job)
+ phys->ops.cleanup_wb_job(phys, job);
+
+ }
+}
+
static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
struct drm_crtc_state *crtc_state,
struct drm_connector_state
*conn_state)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 2903e65..6ceec1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -180,4 +180,20 @@ bool dpu_encoder_is_widebus_enabled(const struct
drm_encoder *drm_enc);
*/
bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc);
+/**
+ * dpu_encoder_prepare_wb_job - prepare writeback job for the encoder.
+ * @drm_enc:Pointer to previously created drm encoder structure
+ * @job:Pointer to the current drm writeback job
+ */
+void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc,
+ struct drm_writeback_job *job);
+
+/**
+ * dpu_encoder_cleanup_wb_job - cleanup writeback job for the encoder.
+ * @drm_enc:Pointer to previously created drm encoder structure
+ * @job:Pointer to the current drm writeback job
+ */
+void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
+ struct drm_writeback_job *job);
+
#endif /* __DPU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index c84b8e8..a42ed4b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -7,6 +7,7 @@
#ifndef __DPU_ENCODER_PHYS_H__
#define __DPU_ENCODER_PHYS_H__
+#include
#include
#include "dpu_kms.h"
@@ -137,6 +138,10 @@ struct dpu_encoder_phys_ops {
void (*restore)(struct dpu_encoder_phys *phys);
int (*get_line_count)(struct dpu_encoder_phys *phys);
int (*get_frame_count)(struct dpu_encoder_phys *phys);
+ void (*prepare_wb_job)(struct dpu_encoder_phys *phys_enc,
+ struct drm_writeback_job *job);
+ void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc,
+ struct drm_writeback_job *job);
};
/**
--
2.7.4
[Freedreno] [PATCH v4 14/20] drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback
Introduce the dpu_encoder_phys_* for the writeback interface
to handle writeback specific hardware programming.
changes in v4:
- squash the encoder_phys_wb bits from [1]
- since its a trivial change of a previously acked change
preserving the ack
[1] https://patchwork.freedesktop.org/patch/483099/?series=102964=2
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 30 +
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 751 +
3 files changed, 782 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index ca779c1..0387f22 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -60,6 +60,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_encoder.o \
disp/dpu1/dpu_encoder_phys_cmd.o \
disp/dpu1/dpu_encoder_phys_vid.o \
+ disp/dpu1/dpu_encoder_phys_wb.o \
disp/dpu1/dpu_formats.o \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index a42ed4b..bed4523 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -150,6 +150,7 @@ struct dpu_encoder_phys_ops {
* @INTR_IDX_PINGPONG: Pingpong done unterrupt for cmd mode panel
* @INTR_IDX_UNDERRUN: Underrun unterrupt for video and cmd mode panel
* @INTR_IDX_RDPTR:Readpointer done unterrupt for cmd mode panel
+ * @INTR_IDX_WB_DONE: Writeback fone interrupt for virtual connector
*/
enum dpu_intr_idx {
INTR_IDX_VSYNC,
@@ -157,6 +158,7 @@ enum dpu_intr_idx {
INTR_IDX_UNDERRUN,
INTR_IDX_CTL_START,
INTR_IDX_RDPTR,
+ INTR_IDX_WB_DONE,
INTR_IDX_MAX,
};
@@ -226,6 +228,27 @@ static inline int dpu_encoder_phys_inc_pending(struct
dpu_encoder_phys *phys)
}
/**
+ * struct dpu_encoder_phys_wb - sub-class of dpu_encoder_phys to handle command
+ * mode specific operations
+ * @base: Baseclass physical encoder structure
+ * @wbirq_refcount: Reference count of writeback interrupt
+ * @wb_done_timeout_cnt: number of wb done irq timeout errors
+ * @wb_cfg: writeback block config to store fb related details
+ * @wb_conn: backpointer to writeback connector
+ * @wb_job: backpointer to current writeback job
+ * @dest: dpu buffer layout for current writeback output buffer
+ */
+struct dpu_encoder_phys_wb {
+ struct dpu_encoder_phys base;
+ atomic_t wbirq_refcount;
+ int wb_done_timeout_cnt;
+ struct dpu_hw_wb_cfg wb_cfg;
+ struct drm_writeback_connector *wb_conn;
+ struct drm_writeback_job *wb_job;
+ struct dpu_hw_fmt_layout dest;
+};
+
+/**
* struct dpu_encoder_phys_cmd - sub-class of dpu_encoder_phys to handle
command
* mode specific operations
* @base: Baseclass physical encoder structure
@@ -295,6 +318,13 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
struct dpu_enc_phys_init_params *p);
/**
+ * dpu_encoder_phys_wb_init - initialize writeback encoder
+ * @init: Pointer to init info structure with initialization params
+ */
+struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
+ struct dpu_enc_phys_init_params *p);
+
+/**
* dpu_encoder_helper_trigger_start - control start helper function
* This helper function may be optionally specified by physical
* encoders if they require ctl_start triggering.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
new file mode 100644
index 000..4ab2699
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -0,0 +1,751 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
+
+#include
+
+#include "dpu_encoder_phys.h"
+#include "dpu_formats.h"
+#include "dpu_hw_top.h"
+#include "dpu_hw_wb.h"
+#include "dpu_hw_lm.h"
+#include "dpu_hw_blk.h"
+#include "dpu_hw_merge3d.h"
+#include "dpu_hw_interrupts.h"
+#include "dpu_core_irq.h"
+#include "dpu_vbif.h"
+#include "dpu_crtc.h"
+#include "disp/msm_disp_snapshot.h"
+
+#define DEFAULT_MAX_WRITEBACK_WIDTH 2048
+
+#define to_dpu_encoder_phys_wb(x) \
+ container_of(x, struct dpu_encoder_phys_wb, base)
+
+/**
+ * dpu_encoder_phys_wb_is_master - report wb always as master encoder
+ */
+static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys *phys_enc)
+{
+ /* there is only one physical enc for dpu_writeback */
+ return true;
+}
+
+/**
+ * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback interface
+ * @phys_enc: Pointer
Re: [Freedreno] [PATCH v3 2/2] drm/msm/dp: Implement oob_hotplug_event()
On 23/04/2022 01:32, Bjorn Andersson wrote:
The Qualcomm DisplayPort driver contains traces of the necessary
plumbing to hook up USB HPD, in the form of the dp_hpd module and the
dp_usbpd_cb struct. Use this as basis for implementing the
oob_hotplug_event() callback, by amending the dp_hpd module with the
missing logic.
Overall the solution is similar to what's done downstream, but upstream
all the code to disect the HPD notification lives on the calling side of
drm_connector_oob_hotplug_event().
drm_connector_oob_hotplug_event() performs the lookup of the
drm_connector based on fwnode, hence the need to assign the fwnode in
dp_drm_connector_init().
Signed-off-by: Bjorn Andersson
---
Changes since v2:
- Rebased patch
drivers/gpu/drm/msm/dp/dp_display.c | 9 +
drivers/gpu/drm/msm/dp/dp_display.h | 3 +++
drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++
drivers/gpu/drm/msm/dp/dp_hpd.c | 21 +
drivers/gpu/drm/msm/dp/dp_hpd.h | 5 +
5 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a42732b67349..1019f6d8fd03 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -449,6 +449,14 @@ static int dp_display_usbpd_configure_cb(struct device
*dev)
return dp_display_process_hpd_high(dp);
}
+void dp_display_oob_hotplug_event(struct msm_dp *dp_display,
+ enum drm_connector_hpd_state hpd_state)
+{
+ struct dp_display_private *dp = container_of(dp_display, struct
dp_display_private, dp_display);
+
+ dp->usbpd->oob_event(dp->usbpd, hpd_state);
+}
+
static int dp_display_usbpd_disconnect_cb(struct device *dev)
{
struct dp_display_private *dp = dev_get_dp_display_private(dev);
@@ -1302,6 +1310,7 @@ static int dp_display_probe(struct platform_device *pdev)
dp->pdev = pdev;
dp->name = "drm_dp";
dp->dp_display.connector_type = desc->connector_type;
+ dp->dp_display.dev = >dev;
rc = dp_init_sub_modules(dp);
if (rc) {
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..16658270df2c 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"
struct msm_dp {
+ struct device *dev;
struct drm_device *drm_dev;
struct device *codec_dev;
struct drm_bridge *bridge;
@@ -40,5 +41,7 @@ bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
void dp_display_signal_audio_start(struct msm_dp *dp_display);
void dp_display_signal_audio_complete(struct msm_dp *dp_display);
+void dp_display_oob_hotplug_event(struct msm_dp *dp_display,
+ enum drm_connector_hpd_state hpd_state);
#endif /* _DP_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..76904b1601b1 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -123,6 +123,14 @@ static enum drm_mode_status dp_connector_mode_valid(
return dp_display_validate_mode(dp_disp, mode->clock);
}
+static void dp_oob_hotplug_event(struct drm_connector *connector,
+enum drm_connector_hpd_state hpd_state)
+{
+ struct msm_dp *dp_disp = to_dp_connector(connector)->dp_display;
+
+ dp_display_oob_hotplug_event(dp_disp, hpd_state);
+}
+
static const struct drm_connector_funcs dp_connector_funcs = {
.detect = dp_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -130,6 +138,7 @@ static const struct drm_connector_funcs dp_connector_funcs
= {
.reset = drm_atomic_helper_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .oob_hotplug_event = dp_oob_hotplug_event,
We were (are) going to switch dp driver to use drm_bridge_connector (to
fix support for bridge chains, eDP panels, etc.
So these changes must be ported to drm_bridge_connector (or we must
abandon/defer the idea of using the bridge_connector).
For the oob_hotplug_event() callback proper support might be as simple
as calling drm_bridge_connector_hpd_cb().
};
static const struct drm_connector_helper_funcs dp_connector_helper_funcs = {
@@ -160,6 +169,8 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display)
if (ret)
return ERR_PTR(ret);
+ connector->fwnode = fwnode_handle_get(dev_fwnode(dp_display->dev));
+
This would be much more interesting. Supporting this in a generic way
might be tricky. But we can still set the fwnode manually from the dp code.
drm_connector_helper_add(connector,
[Freedreno] [PATCH v4 06/20] drm/msm/dpu: rename dpu_hw_pipe_cdp_cfg to dpu_hw_cdp_cfg
Rename dpu_hw_pipe_cdp_cfg to dpu_hw_cdp_cfg and move it
to dpu_hw_utils file so that other modules in addition to
SSPP such as writeback can use it as all the fields can
be used by writeback as well.
Suggested-by: Dmitry Baryshkov
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 15 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 ++--
4 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index 09cdc35..0a0864d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -627,7 +627,7 @@ static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_pipe
*ctx,
}
static void dpu_hw_sspp_setup_cdp(struct dpu_hw_pipe *ctx,
- struct dpu_hw_pipe_cdp_cfg *cfg,
+ struct dpu_hw_cdp_cfg *cfg,
enum dpu_sspp_multirect_index index)
{
u32 idx;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 92b071b..a81e166 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -193,22 +193,6 @@ enum {
};
/**
- * struct dpu_hw_pipe_cdp_cfg : CDP configuration
- * @enable: true to enable CDP
- * @ubwc_meta_enable: true to enable ubwc metadata preload
- * @tile_amortize_enable: true to enable amortization control for tile format
- * @preload_ahead: number of request to preload ahead
- * DPU_SSPP_CDP_PRELOAD_AHEAD_32,
- * DPU_SSPP_CDP_PRELOAD_AHEAD_64
- */
-struct dpu_hw_pipe_cdp_cfg {
- bool enable;
- bool ubwc_meta_enable;
- bool tile_amortize_enable;
- u32 preload_ahead;
-};
-
-/**
* struct dpu_hw_pipe_ts_cfg - traffic shaper configuration
* @size: size to prefill in bytes, or zero to disable
* @time: time to prefill in usec, or zero to disable
@@ -359,7 +343,7 @@ struct dpu_hw_sspp_ops {
* @index: rectangle index in multirect
*/
void (*setup_cdp)(struct dpu_hw_pipe *ctx,
- struct dpu_hw_pipe_cdp_cfg *cfg,
+ struct dpu_hw_cdp_cfg *cfg,
enum dpu_sspp_multirect_index index);
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
index 3913475..a200df1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -298,6 +298,21 @@ struct dpu_drm_scaler_v2 {
struct dpu_drm_de_v1 de;
};
+/**
+ * struct dpu_hw_cdp_cfg : CDP configuration
+ * @enable: true to enable CDP
+ * @ubwc_meta_enable: true to enable ubwc metadata preload
+ * @tile_amortize_enable: true to enable amortization control for tile format
+ * @preload_ahead: number of request to preload ahead
+ * DPU_*_CDP_PRELOAD_AHEAD_32,
+ * DPU_*_CDP_PRELOAD_AHEAD_64
+ */
+struct dpu_hw_cdp_cfg {
+ bool enable;
+ bool ubwc_meta_enable;
+ bool tile_amortize_enable;
+ u32 preload_ahead;
+};
u32 *dpu_hw_util_get_log_mask_ptr(void);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c77c3d9d..08b8c64 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1246,9 +1246,9 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane
*plane)
pstate->multirect_index);
if (pdpu->pipe_hw->ops.setup_cdp) {
- struct dpu_hw_pipe_cdp_cfg cdp_cfg;
+ struct dpu_hw_cdp_cfg cdp_cfg;
- memset(_cfg, 0, sizeof(struct dpu_hw_pipe_cdp_cfg));
+ memset(_cfg, 0, sizeof(struct dpu_hw_cdp_cfg));
cdp_cfg.enable = pdpu->catalog->perf.cdp_cfg
[DPU_PERF_CDP_USAGE_RT].rd_enable;
--
2.7.4
[Freedreno] [PATCH v4 11/20] drm/msm/dpu: make changes to dpu_encoder to support virtual encoder
Make changes to dpu_encoder to support virtual encoder needed
to support writeback for dpu.
changes in v4:
- squash dpu_encoder pieces from [1]
[1] https://patchwork.freedesktop.org/patch/483099/?series=102964=2
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 94 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 ++
2 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 25c7eda..d1e92d89 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1013,9 +1013,18 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
- if (!phys->hw_intf) {
+ if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
+ phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
+
+ if (!phys->hw_intf && !phys->hw_wb) {
DPU_ERROR_ENC(dpu_enc,
- "no intf block assigned at idx: %d\n", i);
+ "no intf or wb block assigned at idx:
%d\n", i);
+ return;
+ }
+
+ if (phys->hw_intf && phys->hw_wb) {
+ DPU_ERROR_ENC(dpu_enc,
+ "invalid phys both intf and wb block at
idx: %d\n", i);
return;
}
@@ -1163,16 +1172,35 @@ static enum dpu_intf dpu_encoder_get_intf(struct
dpu_mdss_cfg *catalog,
{
int i = 0;
- for (i = 0; i < catalog->intf_count; i++) {
- if (catalog->intf[i].type == type
- && catalog->intf[i].controller_id == controller_id) {
- return catalog->intf[i].id;
+ if (type != INTF_WB) {
+ for (i = 0; i < catalog->intf_count; i++) {
+ if (catalog->intf[i].type == type
+ && catalog->intf[i].controller_id ==
controller_id) {
+ return catalog->intf[i].id;
+ }
}
}
return INTF_MAX;
}
+static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
+ enum dpu_intf_type type, u32 controller_id)
+{
+ int i = 0;
+
+ if (type != INTF_WB)
+ goto end;
+
+ for (i = 0; i < catalog->wb_count; i++) {
+ if (catalog->wb[i].id == controller_id)
+ return catalog->wb[i].id;
+ }
+
+end:
+ return WB_MAX;
+}
+
static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phy_enc)
{
@@ -1887,16 +1915,32 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
dpu_encoder_helper_reset_mixers(phys_enc);
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
- if (dpu_enc->phys_encs[i] &&
phys_enc->hw_intf->ops.bind_pingpong_blk)
- phys_enc->hw_intf->ops.bind_pingpong_blk(
- dpu_enc->phys_encs[i]->hw_intf, false,
- dpu_enc->phys_encs[i]->hw_pp->idx);
-
- /* mark INTF flush as pending */
- if (phys_enc->hw_ctl->ops.update_pending_flush_intf)
-
phys_enc->hw_ctl->ops.update_pending_flush_intf(phys_enc->hw_ctl,
- dpu_enc->phys_encs[i]->hw_intf->idx);
+ /*
+* TODO: move the once-only operation like CTL flush/trigger
+* into dpu_encoder_virt_disable() and all operations which need
+* to be done per phys encoder into the phys_disable() op.
+*/
+ if (phys_enc->hw_wb) {
+ /* disable the PP block */
+ if (phys_enc->hw_wb->ops.bind_pingpong_blk)
+ phys_enc->hw_wb->ops.bind_pingpong_blk(phys_enc->hw_wb,
false,
+ phys_enc->hw_pp->idx);
+
+ /* mark WB flush as pending */
+ if (phys_enc->hw_ctl->ops.update_pending_flush_wb)
+ phys_enc->hw_ctl->ops.update_pending_flush_wb(ctl,
phys_enc->hw_wb->idx);
+ } else {
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ if (dpu_enc->phys_encs[i] &&
phys_enc->hw_intf->ops.bind_pingpong_blk)
+ phys_enc->hw_intf->ops.bind_pingpong_blk(
+ dpu_enc->phys_encs[i]->hw_intf,
false,
+
dpu_enc->phys_encs[i]->hw_pp->idx);
+
+ /* mark INTF flush as pending */
+
[Freedreno] [PATCH v4 10/20] drm/msm/dpu: add an API to reset the encoder related hw blocks
Add an API to reset the encoder related hw blocks to ensure
a proper teardown of the pipeline. At the moment this is being
used only for the writeback encoder but eventually we can start
using this for all interfaces.
changes in v4:
- none
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 85 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 7 ++
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 4523693..25c7eda 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights
reserved.
* Copyright (C) 2013 Red Hat
+ * Copyright (c) 2014-2018, 2020-2021 The Linux Foundation. All rights
reserved.
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ *
* Author: Rob Clark
*/
@@ -22,6 +24,7 @@
#include "dpu_hw_ctl.h"
#include "dpu_hw_dspp.h"
#include "dpu_hw_dsc.h"
+#include "dpu_hw_merge3d.h"
#include "dpu_formats.h"
#include "dpu_encoder_phys.h"
#include "dpu_crtc.h"
@@ -1838,6 +1841,86 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
DPU_ATRACE_END("encoder_kickoff");
}
+static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
+{
+ struct dpu_hw_mixer_cfg mixer;
+ int i, num_lm;
+ u32 flush_mask = 0;
+ struct dpu_global_state *global_state;
+ struct dpu_hw_blk *hw_lm[2];
+ struct dpu_hw_mixer *hw_mixer[2];
+ struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
+
+ memset(, 0, sizeof(mixer));
+
+ /* reset all mixers for this encoder */
+ if (phys_enc->hw_ctl->ops.clear_all_blendstages)
+ phys_enc->hw_ctl->ops.clear_all_blendstages(phys_enc->hw_ctl);
+
+ global_state = dpu_kms_get_existing_global_state(phys_enc->dpu_kms);
+
+ num_lm = dpu_rm_get_assigned_resources(_enc->dpu_kms->rm,
global_state,
+ phys_enc->parent->base.id, DPU_HW_BLK_LM, hw_lm,
ARRAY_SIZE(hw_lm));
+
+ for (i = 0; i < num_lm; i++) {
+ hw_mixer[i] = to_dpu_hw_mixer(hw_lm[i]);
+ flush_mask = phys_enc->hw_ctl->ops.get_bitmask_mixer(ctl,
hw_mixer[i]->idx);
+ if (phys_enc->hw_ctl->ops.update_pending_flush)
+ phys_enc->hw_ctl->ops.update_pending_flush(ctl,
flush_mask);
+
+ /* clear all blendstages */
+ if (phys_enc->hw_ctl->ops.setup_blendstage)
+ phys_enc->hw_ctl->ops.setup_blendstage(ctl,
hw_mixer[i]->idx, NULL);
+ }
+}
+
+void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
+{
+ struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
+ struct dpu_hw_intf_cfg intf_cfg = { 0 };
+ int i;
+ struct dpu_encoder_virt *dpu_enc;
+
+ dpu_enc = to_dpu_encoder_virt(phys_enc->parent);
+
+ phys_enc->hw_ctl->ops.reset(ctl);
+
+ dpu_encoder_helper_reset_mixers(phys_enc);
+
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ if (dpu_enc->phys_encs[i] &&
phys_enc->hw_intf->ops.bind_pingpong_blk)
+ phys_enc->hw_intf->ops.bind_pingpong_blk(
+ dpu_enc->phys_encs[i]->hw_intf, false,
+ dpu_enc->phys_encs[i]->hw_pp->idx);
+
+ /* mark INTF flush as pending */
+ if (phys_enc->hw_ctl->ops.update_pending_flush_intf)
+
phys_enc->hw_ctl->ops.update_pending_flush_intf(phys_enc->hw_ctl,
+ dpu_enc->phys_encs[i]->hw_intf->idx);
+ }
+
+ /* reset the merge 3D HW block */
+ if (phys_enc->hw_pp->merge_3d) {
+
phys_enc->hw_pp->merge_3d->ops.setup_3d_mode(phys_enc->hw_pp->merge_3d,
+ BLEND_3D_NONE);
+ if (phys_enc->hw_ctl->ops.update_pending_flush_merge_3d)
+ phys_enc->hw_ctl->ops.update_pending_flush_merge_3d(ctl,
+ phys_enc->hw_pp->merge_3d->idx);
+ }
+
+ intf_cfg.stream_sel = 0; /* Don't care value for video mode */
+ intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ if (phys_enc->hw_pp->merge_3d)
+ intf_cfg.merge_3d = phys_enc->hw_pp->merge_3d->idx;
+
+ if (ctl->ops.reset_intf_cfg)
+ ctl->ops.reset_intf_cfg(ctl, _cfg);
+
+ ctl->ops.trigger_flush(ctl);
+ ctl->ops.trigger_start(ctl);
+ ctl->ops.clear_pending_flush(ctl);
+}
+
void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
[Freedreno] [PATCH v4 09/20] drm/msm/dpu: add changes to support writeback in hw_ctl
Add changes to support writeback module in the dpu_hw_ctl
interface.
changes in v4:
- fix the copyright year order
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 52 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 524f024..254fdf0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include
@@ -23,10 +24,12 @@
#define CTL_SW_RESET 0x030
#define CTL_LAYER_EXTN_OFFSET 0x40
#define CTL_MERGE_3D_ACTIVE 0x0E4
+#define CTL_WB_ACTIVE 0x0EC
#define CTL_INTF_ACTIVE 0x0F4
#define CTL_MERGE_3D_FLUSH0x100
#define CTL_DSC_ACTIVE0x0E8
#define CTL_DSC_FLUSH0x104
+#define CTL_WB_FLUSH 0x108
#define CTL_INTF_FLUSH0x110
#define CTL_INTF_MASTER 0x134
#define CTL_FETCH_PIPE_ACTIVE 0x0FC
@@ -38,6 +41,7 @@
#define MERGE_3D_IDX 23
#define DSC_IDX22
#define INTF_IDX 31
+#define WB_IDX 16
#define CTL_INVALID_BIT 0x
#define CTL_DEFAULT_GROUP_ID 0xf
@@ -135,6 +139,9 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct
dpu_hw_ctl *ctx)
if (ctx->pending_flush_mask & BIT(INTF_IDX))
DPU_REG_WRITE(>hw, CTL_INTF_FLUSH,
ctx->pending_intf_flush_mask);
+ if (ctx->pending_flush_mask & BIT(WB_IDX))
+ DPU_REG_WRITE(>hw, CTL_WB_FLUSH,
+ ctx->pending_wb_flush_mask);
DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask);
}
@@ -255,6 +262,26 @@ static void dpu_hw_ctl_update_pending_flush_intf(struct
dpu_hw_ctl *ctx,
}
}
+static void dpu_hw_ctl_update_pending_flush_wb(struct dpu_hw_ctl *ctx,
+ enum dpu_wb wb)
+{
+ switch (wb) {
+ case WB_0:
+ case WB_1:
+ case WB_2:
+ ctx->pending_flush_mask |= BIT(WB_IDX);
+ default:
+ break;
+ }
+}
+
+static void dpu_hw_ctl_update_pending_flush_wb_v1(struct dpu_hw_ctl *ctx,
+ enum dpu_wb wb)
+{
+ ctx->pending_wb_flush_mask |= BIT(wb - WB_0);
+ ctx->pending_flush_mask |= BIT(WB_IDX);
+}
+
static void dpu_hw_ctl_update_pending_flush_intf_v1(struct dpu_hw_ctl *ctx,
enum dpu_intf intf)
{
@@ -504,6 +531,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
{
struct dpu_hw_blk_reg_map *c = >hw;
u32 intf_active = 0;
+ u32 wb_active = 0;
u32 mode_sel = 0;
/* CTL_TOP[31:28] carries group_id to collate CTL paths
@@ -520,10 +548,18 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
mode_sel |= BIT(17);
intf_active = DPU_REG_READ(c, CTL_INTF_ACTIVE);
- intf_active |= BIT(cfg->intf - INTF_0);
+ wb_active = DPU_REG_READ(c, CTL_WB_ACTIVE);
+
+ if (cfg->intf)
+ intf_active |= BIT(cfg->intf - INTF_0);
+
+ if (cfg->wb)
+ wb_active |= BIT(cfg->wb - WB_0);
DPU_REG_WRITE(c, CTL_TOP, mode_sel);
DPU_REG_WRITE(c, CTL_INTF_ACTIVE, intf_active);
+ DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
+
if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
BIT(cfg->merge_3d - MERGE_3D_0));
@@ -546,6 +582,9 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
intf_cfg |= (cfg->mode_3d - 0x1) << 20;
}
+ if (cfg->wb)
+ intf_cfg |= (cfg->wb & 0x3) + 2;
+
switch (cfg->intf_mode_sel) {
case DPU_CTL_MODE_SEL_VID:
intf_cfg &= ~BIT(17);
@@ -568,12 +607,13 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct
dpu_hw_ctl *ctx,
{
struct dpu_hw_blk_reg_map *c = >hw;
u32 intf_active = 0;
+ u32 wb_active = 0;
u32 merge3d_active = 0;
/*
* This API resets each portion of the CTL path namely,
* clearing the sspps staged on the lm, merge_3d block,
-* interfaces etc to ensure clean teardown of the pipeline.
+* interfaces , writeback etc to ensure clean teardown of the pipeline.
* This will be used for writeback to begin with to have a
* proper teardown of the writeback session but upon further
* validation, this can be extended to all interfaces.
@@ -592,6 +632,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl
[Freedreno] [PATCH v4 08/20] drm/msm/dpu: add writeback blocks to DPU RM
Add writeback blocks to DPU resource manager so that
the encoders can directly request them through RM.
changes in v4:
- absorb dpu_rm.h header change from [1]
- since its a trivial change absorbed from an approved
patch, preserving the previous ack on this
[1] https://patchwork.freedesktop.org/patch/483099/?series=102964=2
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 22 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 12
2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 0e6634b..06f03e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -9,6 +9,7 @@
#include "dpu_hw_ctl.h"
#include "dpu_hw_pingpong.h"
#include "dpu_hw_intf.h"
+#include "dpu_hw_wb.h"
#include "dpu_hw_dspp.h"
#include "dpu_hw_merge3d.h"
#include "dpu_hw_dsc.h"
@@ -87,6 +88,9 @@ int dpu_rm_destroy(struct dpu_rm *rm)
}
}
+ for (i = 0; i < ARRAY_SIZE(rm->hw_wb); i++)
+ dpu_hw_wb_destroy(rm->hw_wb[i]);
+
return 0;
}
@@ -186,6 +190,24 @@ int dpu_rm_init(struct dpu_rm *rm,
rm->hw_intf[intf->id - INTF_0] = hw;
}
+ for (i = 0; i < cat->wb_count; i++) {
+ struct dpu_hw_wb *hw;
+ const struct dpu_wb_cfg *wb = >wb[i];
+
+ if (wb->id < WB_0 || wb->id >= WB_MAX) {
+ DPU_ERROR("skip intf %d with invalid id\n", wb->id);
+ continue;
+ }
+
+ hw = dpu_hw_wb_init(wb->id, mmio, cat);
+ if (IS_ERR(hw)) {
+ rc = PTR_ERR(hw);
+ DPU_ERROR("failed wb object creation: err %d\n", rc);
+ goto fail;
+ }
+ rm->hw_wb[wb->id - WB_0] = hw;
+ }
+
for (i = 0; i < cat->ctl_count; i++) {
struct dpu_hw_ctl *hw;
const struct dpu_ctl_cfg *ctl = >ctl[i];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 32e0d8a..2f34a31 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -19,6 +19,7 @@ struct dpu_global_state;
* @mixer_blks: array of layer mixer hardware resources
* @ctl_blks: array of ctl hardware resources
* @hw_intf: array of intf hardware resources
+ * @hw_wb: array of wb hardware resources
* @dspp_blks: array of dspp hardware resources
*/
struct dpu_rm {
@@ -26,6 +27,7 @@ struct dpu_rm {
struct dpu_hw_blk *mixer_blks[LM_MAX - LM_0];
struct dpu_hw_blk *ctl_blks[CTL_MAX - CTL_0];
struct dpu_hw_intf *hw_intf[INTF_MAX - INTF_0];
+ struct dpu_hw_wb *hw_wb[WB_MAX - WB_0];
struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
@@ -96,5 +98,15 @@ static inline struct dpu_hw_intf *dpu_rm_get_intf(struct
dpu_rm *rm, enum dpu_in
return rm->hw_intf[intf_idx - INTF_0];
}
+/**
+ * dpu_rm_get_wb - Return a struct dpu_hw_wb instance given it's index.
+ * @rm: DPU Resource Manager handle
+ * @wb_idx: WB index
+ */
+static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb
wb_idx)
+{
+ return rm->hw_wb[wb_idx - WB_0];
+}
+
#endif /* __DPU_RM_H__ */
--
2.7.4
[Freedreno] [PATCH v4 07/20] drm/msm/dpu: add dpu_hw_wb abstraction for writeback blocks
Add the dpu_hw_wb abstraction to program registers related to the
writeback block. These will be invoked once all the configuration
is set and ready to be programmed to the registers.
changes in v3:
- start using the common struct dpu_hw_cdp_cfg
- leave a comment about DPU non-DPU_WB_QOS_8LVL chipsets
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 279 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 115
3 files changed, 395 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index d5ca2e6..ca779c1 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -74,6 +74,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_hw_top.o \
disp/dpu1/dpu_hw_util.o \
disp/dpu1/dpu_hw_vbif.o \
+ disp/dpu1/dpu_hw_wb.o \
disp/dpu1/dpu_kms.o \
disp/dpu1/dpu_plane.o \
disp/dpu1/dpu_rm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
new file mode 100644
index 000..be2
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0-only
+ /*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include "dpu_hw_mdss.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hw_wb.h"
+#include "dpu_formats.h"
+#include "dpu_kms.h"
+
+#define WB_DST_FORMAT 0x000
+#define WB_DST_OP_MODE0x004
+#define WB_DST_PACK_PATTERN 0x008
+#define WB_DST0_ADDR 0x00C
+#define WB_DST1_ADDR 0x010
+#define WB_DST2_ADDR 0x014
+#define WB_DST3_ADDR 0x018
+#define WB_DST_YSTRIDE0 0x01C
+#define WB_DST_YSTRIDE1 0x020
+#define WB_DST_YSTRIDE1 0x020
+#define WB_DST_DITHER_BITDEPTH0x024
+#define WB_DST_MATRIX_ROW00x030
+#define WB_DST_MATRIX_ROW10x034
+#define WB_DST_MATRIX_ROW20x038
+#define WB_DST_MATRIX_ROW30x03C
+#define WB_DST_WRITE_CONFIG 0x048
+#define WB_ROTATION_DNSCALER 0x050
+#define WB_ROTATOR_PIPE_DOWNSCALER0x054
+#define WB_N16_INIT_PHASE_X_C03 0x060
+#define WB_N16_INIT_PHASE_X_C12 0x064
+#define WB_N16_INIT_PHASE_Y_C03 0x068
+#define WB_N16_INIT_PHASE_Y_C12 0x06C
+#define WB_OUT_SIZE 0x074
+#define WB_ALPHA_X_VALUE 0x078
+#define WB_DANGER_LUT 0x084
+#define WB_SAFE_LUT 0x088
+#define WB_QOS_CTRL 0x090
+#define WB_CREQ_LUT_0 0x098
+#define WB_CREQ_LUT_1 0x09C
+#define WB_UBWC_STATIC_CTRL 0x144
+#define WB_MUX0x150
+#define WB_CROP_CTRL 0x154
+#define WB_CROP_OFFSET0x158
+#define WB_CSC_BASE 0x260
+#define WB_DST_ADDR_SW_STATUS 0x2B0
+#define WB_CDP_CNTL 0x2B4
+#define WB_OUT_IMAGE_SIZE 0x2C0
+#define WB_OUT_XY 0x2C4
+
+/* WB_QOS_CTRL */
+#define WB_QOS_CTRL_DANGER_SAFE_ENBIT(0)
+
+static const struct dpu_wb_cfg *_wb_offset(enum dpu_wb wb,
+ const struct dpu_mdss_cfg *m, void __iomem *addr,
+ struct dpu_hw_blk_reg_map *b)
+{
+ int i;
+
+ for (i = 0; i < m->wb_count; i++) {
+ if (wb == m->wb[i].id) {
+ b->base_off = addr;
+ b->blk_off = m->wb[i].base;
+ b->length = m->wb[i].len;
+ b->hwversion = m->hwversion;
+ return >wb[i];
+ }
+ }
+ return ERR_PTR(-EINVAL);
+}
+
+static void dpu_hw_wb_setup_outaddress(struct dpu_hw_wb *ctx,
+ struct dpu_hw_wb_cfg *data)
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+
+ DPU_REG_WRITE(c, WB_DST0_ADDR, data->dest.plane_addr[0]);
+ DPU_REG_WRITE(c, WB_DST1_ADDR, data->dest.plane_addr[1]);
+ DPU_REG_WRITE(c, WB_DST2_ADDR, data->dest.plane_addr[2]);
+ DPU_REG_WRITE(c, WB_DST3_ADDR, data->dest.plane_addr[3]);
+}
+
+static void dpu_hw_wb_setup_format(struct dpu_hw_wb *ctx,
+ struct dpu_hw_wb_cfg *data)
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+ const struct dpu_format *fmt = data->dest.format;
[Freedreno] [PATCH v4 05/20] drm/msm/dpu: add reset_intf_cfg operation for dpu_hw_ctl
Add a reset_intf_cfg operation for dpu_hw_ctl to reset the
entire CTL path by disabling each component namely layer mixer,
3d-merge and interface blocks.
changes in v3:
- none
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 32 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 8
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index dc27579..524f024 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -563,6 +563,37 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
DPU_REG_WRITE(c, CTL_TOP, intf_cfg);
}
+static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
+ struct dpu_hw_intf_cfg *cfg)
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+ u32 intf_active = 0;
+ u32 merge3d_active = 0;
+
+ /*
+* This API resets each portion of the CTL path namely,
+* clearing the sspps staged on the lm, merge_3d block,
+* interfaces etc to ensure clean teardown of the pipeline.
+* This will be used for writeback to begin with to have a
+* proper teardown of the writeback session but upon further
+* validation, this can be extended to all interfaces.
+*/
+ if (cfg->merge_3d) {
+ merge3d_active = DPU_REG_READ(c, CTL_MERGE_3D_ACTIVE);
+ merge3d_active &= ~BIT(cfg->merge_3d - MERGE_3D_0);
+ DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
+ merge3d_active);
+ }
+
+ dpu_hw_ctl_clear_all_blendstages(ctx);
+
+ if (cfg->intf) {
+ intf_active = DPU_REG_READ(c, CTL_INTF_ACTIVE);
+ intf_active &= ~BIT(cfg->intf - INTF_0);
+ DPU_REG_WRITE(c, CTL_INTF_ACTIVE, intf_active);
+ }
+}
+
static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
unsigned long *fetch_active)
{
@@ -586,6 +617,7 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
if (cap & BIT(DPU_CTL_ACTIVE_CFG)) {
ops->trigger_flush = dpu_hw_ctl_trigger_flush_v1;
ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg_v1;
+ ops->reset_intf_cfg = dpu_hw_ctl_reset_intf_cfg_v1;
ops->update_pending_flush_intf =
dpu_hw_ctl_update_pending_flush_intf_v1;
ops->update_pending_flush_merge_3d =
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 97f326d..c61a8fd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -140,6 +140,14 @@ struct dpu_hw_ctl_ops {
void (*setup_intf_cfg)(struct dpu_hw_ctl *ctx,
struct dpu_hw_intf_cfg *cfg);
+ /**
+* reset ctl_path interface config
+* @ctx: ctl path ctx pointer
+* @cfg: interface config structure pointer
+*/
+ void (*reset_intf_cfg)(struct dpu_hw_ctl *ctx,
+ struct dpu_hw_intf_cfg *cfg);
+
int (*reset)(struct dpu_hw_ctl *c);
/*
--
2.7.4
[Freedreno] [PATCH v4 04/20] drm/msm/dpu: add writeback blocks to the sm8250 DPU catalog
Add writeback blocks to the sm8250 DPU hardware catalog. Other
chipsets support writeback too but add it to sm8250 to prototype
the feature so that it can be easily extended to other chipsets.
changes in v4:
- fix the copyright year order
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 72 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 66 ++-
2 files changed, 137 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b0a0ef7..7e3f0f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
@@ -120,6 +121,16 @@
BIT(MDP_AD4_0_INTR) | \
BIT(MDP_AD4_1_INTR))
+#define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
+BIT(DPU_WB_UBWC) | \
+BIT(DPU_WB_YUV_CONFIG) | \
+BIT(DPU_WB_PIPE_ALPHA) | \
+BIT(DPU_WB_XY_ROI_OFFSET) | \
+BIT(DPU_WB_QOS) | \
+BIT(DPU_WB_QOS_8LVL) | \
+BIT(DPU_WB_CDP) | \
+BIT(DPU_WB_INPUT_CTRL))
+
#define DEFAULT_PIXEL_RAM_SIZE (50 * 1024)
#define DEFAULT_DPU_LINE_WIDTH 2048
#define DEFAULT_DPU_OUTPUT_LINE_WIDTH 2560
@@ -211,6 +222,40 @@ static const u32 rotation_v2_formats[] = {
/* TODO add formats after validation */
};
+static const uint32_t wb2_formats[] = {
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_BGR565,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XBGR,
+};
+
/*
* DPU sub blocks config
*/
@@ -448,6 +493,8 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
+ .clk_ctrls[DPU_CLK_CTRL_WB2] = {
+ .reg_off = 0x3B8, .bit_off = 24},
},
};
@@ -1235,6 +1282,29 @@ static const struct dpu_intf_cfg qcm2290_intf[] = {
};
/*
+ * Writeback blocks config
+ */
+#define WB_BLK(_name, _id, _base, _features, _clk_ctrl, \
+ __xin_id, vbif_id, _reg, _wb_done_bit) \
+ { \
+ .name = _name, .id = _id, \
+ .base = _base, .len = 0x2c8, \
+ .features = _features, \
+ .format_list = wb2_formats, \
+ .num_formats = ARRAY_SIZE(wb2_formats), \
+ .clk_ctrl = _clk_ctrl, \
+ .xin_id = __xin_id, \
+ .vbif_idx = vbif_id, \
+ .maxlinewidth = DEFAULT_DPU_LINE_WIDTH, \
+ .intr_wb_done = DPU_IRQ_IDX(_reg, _wb_done_bit) \
+ }
+
+static const struct dpu_wb_cfg sm8250_wb[] = {
+ WB_BLK("wb_2", WB_2, 0x65000, WB_SM8250_MASK, DPU_CLK_CTRL_WB2, 6,
+ VBIF_RT, MDP_SSPP_TOP0_INTR, 4),
+};
+
+/*
* VBIF sub blocks config
*/
/* VBIF QOS remap */
@@ -1832,6 +1902,8 @@ static void sm8250_cfg_init(struct dpu_mdss_cfg *dpu_cfg)
.intf = sm8150_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),
.vbif = sdm845_vbif,
+ .wb_count = ARRAY_SIZE(sm8250_wb),
+ .wb = sm8250_wb,
.reg_dma_count = 1,
.dma_cfg = sm8250_regdma,
.perf = sm8250_perf_data,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 866fd7a..8cb6d1f
[Freedreno] [PATCH v4 03/20] drm: allow real encoder to be passed for drm_writeback_connector
For some vendor driver implementations, display hardware can
be shared between the encoder used for writeback and the physical
display.
In addition resources such as clocks and interrupts can
also be shared between writeback and the real encoder.
To accommodate such vendor drivers and hardware, allow
real encoder to be passed for drm_writeback_connector.
For existing clients, drm_writeback_connector_init() will use
an internal_encoder under the hood and hence no changes will
be needed.
changes in v7:
- move this change before the vc4 change in the series
to minimize the changes to vendor drivers in drm core
changes
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_writeback.c | 18 --
drivers/gpu/drm/vc4/vc4_txp.c | 4 ++--
include/drm/drm_writeback.h | 22 --
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 92658ad..0538674 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -180,21 +180,21 @@ int drm_writeback_connector_init(struct drm_device *dev,
{
int ret = 0;
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
+ drm_encoder_helper_add(_connector->internal_encoder,
enc_helper_funcs);
- wb_connector->encoder.possible_crtcs = possible_crtcs;
+ wb_connector->internal_encoder.possible_crtcs = possible_crtcs;
- ret = drm_encoder_init(dev, _connector->encoder,
+ ret = drm_encoder_init(dev, _connector->internal_encoder,
_writeback_encoder_funcs,
DRM_MODE_ENCODER_VIRTUAL, NULL);
if (ret)
return ret;
- ret = drm_writeback_connector_init_with_encoder(dev, wb_connector,
_connector->encoder,
- con_funcs, formats, n_formats);
+ ret = drm_writeback_connector_init_with_encoder(dev, wb_connector,
+ _connector->internal_encoder, con_funcs, formats,
n_formats);
if (ret)
- drm_encoder_cleanup(_connector->encoder);
+ drm_encoder_cleanup(_connector->internal_encoder);
return ret;
}
@@ -239,6 +239,12 @@ int drm_writeback_connector_init_with_encoder(struct
drm_device *dev,
struct drm_mode_config *config = >mode_config;
int ret = create_writeback_properties(dev);
+ /*
+* Assign the encoder passed to this API to the wb_connector's encoder.
+* For drm_writeback_connector_init(), this shall be the
internal_encoder
+*/
+ wb_connector->encoder = enc;
+
if (ret != 0)
return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 3447eb6..7e063a9 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -159,7 +159,7 @@ struct vc4_txp {
static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder)
{
- return container_of(encoder, struct vc4_txp, connector.encoder);
+ return container_of(encoder, struct vc4_txp,
connector.internal_encoder);
}
static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn)
@@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
- encoder = >connector.encoder;
+ encoder = txp->connector.encoder;
encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index bb306fa..3fbae9d 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -25,13 +25,31 @@ struct drm_writeback_connector {
struct drm_connector base;
/**
-* @encoder: Internal encoder used by the connector to fulfill
+* @encoder: handle to drm_encoder used by the connector to fulfill
* the DRM framework requirements. The users of the
* @drm_writeback_connector control the behaviour of the @encoder
* by passing the @enc_funcs parameter to drm_writeback_connector_init()
* function.
+*
+* For some vendor drivers, the hardware resources are shared between
+* writeback encoder and rest of the display pipeline.
+* To accommodate such cases, encoder is a handle to the real encoder
+* hardware.
+*
+* For current existing writeback users, this shall continue to be the
+* embedded encoder for the writeback connector.
+*/
+ struct drm_encoder *encoder;
+
+ /**
+* @internal_encoder: internal encoder used by writeback when
+* drm_writeback_connector_init() is used.
+* @encoder will be assigned to this for those cases
+*
+* This will be unused when
[Freedreno] [PATCH v4 02/20] drm: introduce drm_writeback_connector_init_with_encoder() API
For vendors drivers which pass an already allocated and
initialized encoder especially for cases where the encoder
hardware is shared OR the writeback encoder shares the resources
with the rest of the display pipeline introduce a new API,
drm_writeback_connector_init_with_encoder() which expects
an initialized encoder as a parameter and only sets up the
writeback connector.
changes in v4:
- removed the possible_crtcs part
changes in v5:
- reorder this change to come before in the series
to avoid incorrect functionality in subsequent changes
- continue using struct drm_encoder instead of
struct drm_encoder * and switch it in next change
changes in v6:
- remove drm_writeback_connector_setup() and instead
directly call drm_writeback_connector_init_with_encoder()
- fix a drm_writeback_connector typo and function doc which
incorrectly shows that the function accepts enc_helper_funcs
- pass encoder as a parameter explicitly to the new API
for better readability
changes in v7:
- fix the function doc slightly as suggested by Liviu
Reviewed-by: Liviu Dudau
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_writeback.c | 72 +
include/drm/drm_writeback.h | 6
2 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 9e0b845..92658ad 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -178,6 +178,62 @@ int drm_writeback_connector_init(struct drm_device *dev,
const u32 *formats, int n_formats,
u32 possible_crtcs)
{
+ int ret = 0;
+
+ drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
+
+ wb_connector->encoder.possible_crtcs = possible_crtcs;
+
+ ret = drm_encoder_init(dev, _connector->encoder,
+ _writeback_encoder_funcs,
+ DRM_MODE_ENCODER_VIRTUAL, NULL);
+ if (ret)
+ return ret;
+
+ ret = drm_writeback_connector_init_with_encoder(dev, wb_connector,
_connector->encoder,
+ con_funcs, formats, n_formats);
+
+ if (ret)
+ drm_encoder_cleanup(_connector->encoder);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_writeback_connector_init);
+
+/**
+ * drm_writeback_connector_init_with_encoder - Initialize a writeback
connector and its properties
+ * using the encoder which already assigned and initialized
+ *
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @enc: handle to the already initialized drm encoder
+ * @con_funcs: Connector funcs vtable
+ * @formats: Array of supported pixel formats for the writeback engine
+ * @n_formats: Length of the formats array
+ *
+ * This function creates the writeback-connector-specific properties if they
+ * have not been already created, initializes the connector as
+ * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
+ * values.
+ *
+ * This function assumes that the drm_writeback_connector's encoder has
already been
+ * created and initialized before invoking this function.
+ *
+ * In addition, this function also assumes that callers of this API will manage
+ * assigning the encoder helper functions, possible_crtcs and any other encoder
+ * specific operation.
+ *
+ * Drivers should always use this function instead of drm_connector_init() to
+ * set up writeback connectors if they want to manage themselves the lifetime
of the
+ * associated encoder.
+ *
+ * Returns: 0 on success, or a negative error code
+ */
+int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
+ struct drm_writeback_connector *wb_connector, struct
drm_encoder *enc,
+ const struct drm_connector_funcs *con_funcs, const u32 *formats,
+ int n_formats)
+{
struct drm_property_blob *blob;
struct drm_connector *connector = _connector->base;
struct drm_mode_config *config = >mode_config;
@@ -191,15 +247,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (IS_ERR(blob))
return PTR_ERR(blob);
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
-
- wb_connector->encoder.possible_crtcs = possible_crtcs;
-
- ret = drm_encoder_init(dev, _connector->encoder,
- _writeback_encoder_funcs,
- DRM_MODE_ENCODER_VIRTUAL, NULL);
- if (ret)
- goto fail;
connector->interlace_allowed = 0;
@@ -208,8 +255,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (ret)
goto connector_fail;
- ret = drm_connector_attach_encoder(connector,
-
[Freedreno] [PATCH v4 00/20] Add writeback block support for DPU
This series adds support for writeback block on DPU. Writeback block is extremely useful to validate boards having no physical displays in addition to many other use-cases where we want to get the output of the display pipeline to examine whether issue is with the display pipeline or with the panel. These changes have been validated on SM8250 RB5 boards with IGT KMS writeback test-suite thereby further increasing the IGT test coverage for DPU. I am sharing the test results below. root@linaro-developer:~/igt_repo/igt-gpu-tools/build/tests# ./kms_writeback [ 35.066157] Console: switching to colour dummy device 80x25 [ 35.071964] [IGT] kms_writeback: executing IGT-Version: 1.26-gae2eb9e1 (aarch64) (Linux: 5.16.0-rc2-62171-g132577e2697b aarch64) [ 35.611418] [IGT] kms_writeback: starting subtest writeback-pixel-formats Starting subtest: writeback-pixel-formats [ 35.618528] [IGT] kms_writeback: starting subtest writeback-invalid-parameters Subtest writeback-pixel-formats: SUCCESS (0.000s) Starting subtest: writeback-invalid-parameters Subtest writeback-invalid-parameters: SUCCESS (0.028s) 35.657437] [IGT] kms_writeback: starting subtest writeback-fb-id Starting subtest: writeback-fb-id Subtest writeback-fb-id: SUCCESS (0.030s) [ 35.698957] [IGT] kms_writeback: starting subtest writeback-check-output Starting subtest: writeback-check-output [ 35.852834] [IGT] kms_writeback: exiting, ret=0 Subtest writeback-check-output: SUCCESS (0.142s) [ 35.861291] Console: switching to colour frame buffer device 240x67 root@linaro-developer:~/igt_repo/igt-gpu-tools/build/tests# The changes can easily be extended to support any other chipset using the DPU driver by adding the support in the catalog. Writeback block supports various formats and features. The support for all of them can be incrementally added on top of this framework when validation is improved and the test frameworks are extended to validate them. changes in v4: - absorb https://patchwork.freedesktop.org/series/102964/ into this and also preserve necessary acks for trivial changes - fix order of copyright years for all the changes Abhinav Kumar (20): drm: allow passing possible_crtcs to drm_writeback_connector_init() drm: introduce drm_writeback_connector_init_with_encoder() API drm: allow real encoder to be passed for drm_writeback_connector drm/msm/dpu: add writeback blocks to the sm8250 DPU catalog drm/msm/dpu: add reset_intf_cfg operation for dpu_hw_ctl drm/msm/dpu: rename dpu_hw_pipe_cdp_cfg to dpu_hw_cdp_cfg drm/msm/dpu: add dpu_hw_wb abstraction for writeback blocks drm/msm/dpu: add writeback blocks to DPU RM drm/msm/dpu: add changes to support writeback in hw_ctl drm/msm/dpu: add an API to reset the encoder related hw blocks drm/msm/dpu: make changes to dpu_encoder to support virtual encoder drm/msm/dpu: add encoder operations to prepare/cleanup wb job drm/msm/dpu: move _dpu_plane_get_qos_lut to dpu_hw_util file drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback drm/msm/dpu: add the writeback connector layer drm/msm/dpu: initialize dpu encoder and connector for writeback drm/msm/dpu: gracefully handle null fb commits for writeback drm/msm/dpu: add writeback blocks to the display snapshot drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder .../drm/arm/display/komeda/komeda_wb_connector.c | 4 +- drivers/gpu/drm/arm/malidp_mw.c| 4 +- drivers/gpu/drm/drm_writeback.c| 79 ++- drivers/gpu/drm/msm/Makefile | 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 306 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 22 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 50 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 763 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 72 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 66 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 82 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 21 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c| 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h| 18 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c| 25 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h| 19 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 279 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 115 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 66 +- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 22 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 12 + drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 68 ++
[Freedreno] [PATCH v4 01/20] drm: allow passing possible_crtcs to drm_writeback_connector_init()
Clients of drm_writeback_connector_init() initialize the
possible_crtcs and then invoke the call to this API.
To simplify things, allow passing possible_crtcs as a parameter
to drm_writeback_connector_init() and make changes to the
other drm drivers to make them compatible with this change.
changes in v2:
- split the changes according to their functionality
changes in v3:
- allow passing possible_crtcs for existing users of
drm_writeback_connector_init()
- squash the vendor changes into the same commit so
that each patch in the series can compile individually
changes in v4:
- keep only changes related to possible_crtcs
- add line breaks after ARRAY_SIZE
- stop using temporary variables for possible_crtcs
changes in v5:
- None
changes in v6:
- None
changes in v7:
- wrap long lines to match the coding style of existing drivers
- Fix indentation and remove parenthesis where not needed
- use u32 instead of uint32_t for possible_crtcs
Signed-off-by: Abhinav Kumar
Acked-by: Liviu Dudau
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 4 ++--
drivers/gpu/drm/arm/malidp_mw.c | 4 ++--
drivers/gpu/drm/drm_writeback.c | 7 ++-
drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 4 ++--
drivers/gpu/drm/vc4/vc4_txp.c| 3 ++-
drivers/gpu/drm/vkms/vkms_writeback.c| 4 ++--
include/drm/drm_writeback.h | 3 ++-
7 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4..ce4b760 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -155,7 +155,6 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
wb_conn = _conn->base;
- wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(>base));
formats = komeda_get_layer_fourcc_list(>fmt_tbl,
kwb_conn->wb_layer->layer_type,
@@ -164,7 +163,8 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
err = drm_writeback_connector_init(>base, wb_conn,
_wb_connector_funcs,
_wb_encoder_helper_funcs,
- formats, n_formats);
+ formats, n_formats,
+ BIT(drm_crtc_index(>base)));
komeda_put_fourcc_list(formats);
if (err) {
kfree(kwb_conn);
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index f5847a7..204c869 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -212,7 +212,6 @@ int malidp_mw_connector_init(struct drm_device *drm)
if (!malidp->dev->hw->enable_memwrite)
return 0;
- malidp->mw_connector.encoder.possible_crtcs = 1 <<
drm_crtc_index(>crtc);
drm_connector_helper_add(>mw_connector.base,
_mw_connector_helper_funcs);
@@ -223,7 +222,8 @@ int malidp_mw_connector_init(struct drm_device *drm)
ret = drm_writeback_connector_init(drm, >mw_connector,
_mw_connector_funcs,
_mw_encoder_helper_funcs,
- formats, n_formats);
+ formats, n_formats,
+ 1 << drm_crtc_index(>crtc));
kfree(formats);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504..9e0b845 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -157,6 +157,7 @@ static const struct drm_encoder_funcs
drm_writeback_encoder_funcs = {
* @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal
encoder
* @formats: Array of supported pixel formats for the writeback engine
* @n_formats: Length of the formats array
+ * @possible_crtcs: possible crtcs for the internal writeback encoder
*
* This function creates the writeback-connector-specific properties if they
* have not been already created, initializes the connector as
@@ -174,7 +175,8 @@ int drm_writeback_connector_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,
const struct drm_connector_funcs *con_funcs,
const struct drm_encoder_helper_funcs
*enc_helper_funcs,
-
Re: [Freedreno] [PATCH] drm: msm: fix error check return value of irq_of_parse_and_map()
On 22/04/2022 13:42, Dmitry Baryshkov wrote:
On Fri, 22 Apr 2022 at 11:52, wrote:
From: Lv Ruyi
The irq_of_parse_and_map() function returns 0 on failure, and does not
return an negative value.
Reported-by: Zeal Robot
Signed-off-by: Lv Ruyi
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 3b92372e7bdf..1fb1ed9e95d9 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -570,7 +570,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
}
irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
- if (irq < 0) {
+ if (!irq) {
ret = irq;
As noted by Stephen, this will cause the function to prematurely return
0 (success).
DRM_DEV_ERROR(>dev, "failed to get irq: %d\n", ret);
goto fail;
--
2.25.1
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH] drm/msm/dp: fix error check return value of irq_of_parse_and_map()
On 22/04/2022 21:39, Stephen Boyd wrote:
Quoting cgel@gmail.com (2022-04-22 01:49:51)
From: Lv Ruyi
The irq_of_parse_and_map() function returns 0 on failure, and does not
return an negative value.
Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Reported-by: Zeal Robot
Signed-off-by: Lv Ruyi
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a42732b67349..3926d2ac107d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1239,7 +1239,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
Why can't platform_get_irq() be used?
- if (dp->irq < 0) {
+ if (!dp->irq) {
rc = dp->irq;
zero as an error return value is an error?
Hmm, nice catch. Please fix it. And the other patch too.
DRM_ERROR("failed to get irq: %d\n", rc);
return rc;
--
With best wishes
Dmitry
[Freedreno] [PATCH v3 2/2] drm/msm/dp: Implement oob_hotplug_event()
The Qualcomm DisplayPort driver contains traces of the necessary
plumbing to hook up USB HPD, in the form of the dp_hpd module and the
dp_usbpd_cb struct. Use this as basis for implementing the
oob_hotplug_event() callback, by amending the dp_hpd module with the
missing logic.
Overall the solution is similar to what's done downstream, but upstream
all the code to disect the HPD notification lives on the calling side of
drm_connector_oob_hotplug_event().
drm_connector_oob_hotplug_event() performs the lookup of the
drm_connector based on fwnode, hence the need to assign the fwnode in
dp_drm_connector_init().
Signed-off-by: Bjorn Andersson
---
Changes since v2:
- Rebased patch
drivers/gpu/drm/msm/dp/dp_display.c | 9 +
drivers/gpu/drm/msm/dp/dp_display.h | 3 +++
drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++
drivers/gpu/drm/msm/dp/dp_hpd.c | 21 +
drivers/gpu/drm/msm/dp/dp_hpd.h | 5 +
5 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a42732b67349..1019f6d8fd03 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -449,6 +449,14 @@ static int dp_display_usbpd_configure_cb(struct device
*dev)
return dp_display_process_hpd_high(dp);
}
+void dp_display_oob_hotplug_event(struct msm_dp *dp_display,
+ enum drm_connector_hpd_state hpd_state)
+{
+ struct dp_display_private *dp = container_of(dp_display, struct
dp_display_private, dp_display);
+
+ dp->usbpd->oob_event(dp->usbpd, hpd_state);
+}
+
static int dp_display_usbpd_disconnect_cb(struct device *dev)
{
struct dp_display_private *dp = dev_get_dp_display_private(dev);
@@ -1302,6 +1310,7 @@ static int dp_display_probe(struct platform_device *pdev)
dp->pdev = pdev;
dp->name = "drm_dp";
dp->dp_display.connector_type = desc->connector_type;
+ dp->dp_display.dev = >dev;
rc = dp_init_sub_modules(dp);
if (rc) {
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 7af2b186d2d9..16658270df2c 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"
struct msm_dp {
+ struct device *dev;
struct drm_device *drm_dev;
struct device *codec_dev;
struct drm_bridge *bridge;
@@ -40,5 +41,7 @@ bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
void dp_display_signal_audio_start(struct msm_dp *dp_display);
void dp_display_signal_audio_complete(struct msm_dp *dp_display);
+void dp_display_oob_hotplug_event(struct msm_dp *dp_display,
+ enum drm_connector_hpd_state hpd_state);
#endif /* _DP_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..76904b1601b1 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -123,6 +123,14 @@ static enum drm_mode_status dp_connector_mode_valid(
return dp_display_validate_mode(dp_disp, mode->clock);
}
+static void dp_oob_hotplug_event(struct drm_connector *connector,
+enum drm_connector_hpd_state hpd_state)
+{
+ struct msm_dp *dp_disp = to_dp_connector(connector)->dp_display;
+
+ dp_display_oob_hotplug_event(dp_disp, hpd_state);
+}
+
static const struct drm_connector_funcs dp_connector_funcs = {
.detect = dp_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -130,6 +138,7 @@ static const struct drm_connector_funcs dp_connector_funcs
= {
.reset = drm_atomic_helper_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .oob_hotplug_event = dp_oob_hotplug_event,
};
static const struct drm_connector_helper_funcs dp_connector_helper_funcs = {
@@ -160,6 +169,8 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display)
if (ret)
return ERR_PTR(ret);
+ connector->fwnode = fwnode_handle_get(dev_fwnode(dp_display->dev));
+
drm_connector_helper_add(connector, _connector_helper_funcs);
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_hpd.c b/drivers/gpu/drm/msm/dp/dp_hpd.c
index db98a1d431eb..cdb1feea5ebf 100644
--- a/drivers/gpu/drm/msm/dp/dp_hpd.c
+++ b/drivers/gpu/drm/msm/dp/dp_hpd.c
@@ -7,6 +7,8 @@
#include
#include
+#include
+#include
#include "dp_hpd.h"
@@ -45,6 +47,24 @@ int dp_hpd_connect(struct dp_usbpd *dp_usbpd, bool hpd)
return rc;
}
+static void dp_hpd_oob_event(struct dp_usbpd *dp_usbpd,
+enum drm_connector_hpd_state hpd_state)
+{
+ struct dp_hpd_private *hpd_priv =
[Freedreno] [PATCH v3 1/2] drm: Add HPD state to drm_connector_oob_hotplug_event()
In some implementations, such as the Qualcomm platforms, the display
driver has no way to query the current HPD state and as such it's
impossible to distinguish between disconnect and attention events.
Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
state.
Also push the test for unchanged state in the displayport altmode driver
into the i915 driver, to allow other drivers to act upon each update.
Signed-off-by: Bjorn Andersson
---
Changs since v2:
- The i915 cached hpd_state is tracked per encoder.
drivers/gpu/drm/drm_connector.c | 6 --
drivers/gpu/drm/i915/display/intel_dp.c | 17 ++---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/usb/typec/altmodes/displayport.c | 10 +++---
include/drm/drm_connector.h | 11 +--
5 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 76a8c707c34b..fff8c74d1ae6 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2828,6 +2828,7 @@ struct drm_connector *drm_connector_find_by_fwnode(struct
fwnode_handle *fwnode)
/**
* drm_connector_oob_hotplug_event - Report out-of-band hotplug event to
connector
* @connector_fwnode: fwnode_handle to report the event on
+ * @hpd_state: hot plug detect logical state
*
* On some hardware a hotplug event notification may come from outside the
display
* driver / device. An example of this is some USB Type-C setups where the
hardware
@@ -2837,7 +2838,8 @@ struct drm_connector *drm_connector_find_by_fwnode(struct
fwnode_handle *fwnode)
* This function can be used to report these out-of-band events after obtaining
* a drm_connector reference through calling drm_connector_find_by_fwnode().
*/
-void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
+void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
+enum drm_connector_hpd_state hpd_state)
{
struct drm_connector *connector;
@@ -2846,7 +2848,7 @@ void drm_connector_oob_hotplug_event(struct fwnode_handle
*connector_fwnode)
return;
if (connector->funcs->oob_hotplug_event)
- connector->funcs->oob_hotplug_event(connector);
+ connector->funcs->oob_hotplug_event(connector, hpd_state);
drm_connector_put(connector);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index d55acc4a028a..2907d8e1f80e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4950,15 +4950,26 @@ static int intel_dp_connector_atomic_check(struct
drm_connector *conn,
return intel_modeset_synced_crtcs(state, conn);
}
-static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
+static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
+ enum drm_connector_hpd_state hpd_state)
{
struct intel_encoder *encoder =
intel_attached_encoder(to_intel_connector(connector));
struct drm_i915_private *i915 = to_i915(connector->dev);
+ bool hpd_high = hpd_state == DRM_CONNECTOR_HPD_HIGH;
+ unsigned int hpd_pin = encoder->hpd_pin;
+ bool need_work = false;
spin_lock_irq(>irq_lock);
- i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
+ if (hpd_high != test_bit(hpd_pin,
>hotplug.oob_hotplug_last_state)) {
+ i915->hotplug.event_bits |= BIT(hpd_pin);
+
+ __assign_bit(hpd_pin, >hotplug.oob_hotplug_last_state,
hpd_high);
+ need_work = true;
+ }
spin_unlock_irq(>irq_lock);
- queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
+
+ if (need_work)
+ queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
}
static const struct drm_connector_funcs intel_dp_connector_funcs = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3711d618a372..71d0c7130ddd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -134,6 +134,9 @@ struct i915_hotplug {
/* Whether or not to count short HPD IRQs in HPD storms */
u8 hpd_short_storm_enabled;
+ /* Last state reported by oob_hotplug_event for each encoder */
+ unsigned long oob_hotplug_last_state;
+
/*
* if we get a HPD irq from DP and a HPD irq from non-DP
* the non-DP HPD could block the workqueue on a mode config
diff --git a/drivers/usb/typec/altmodes/displayport.c
b/drivers/usb/typec/altmodes/displayport.c
index c1d8c23baa39..ea9cb1d71fd2 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -59,7 +59,6 @@ struct dp_altmode {
struct typec_displayport_data data;
enum dp_state state;
- bool hpd;
struct mutex lock; /*
Re: [Freedreno] [PATCH v2 1/3] drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces
On 22/04/2022 22:45, Abhinav Kumar wrote:
Using intf_idx even for writeback interfaces is confusing
because intf_idx is of type enum dpu_intf but the index used
for writeback is of type enum dpu_wb.
In addition, this makes it easier to separately check the
availability of the two as its possible that there are boards
which don't have any physical display connected but can still
work in writeback mode.
changes in v2:
- add check for invalid phys having both intf and wb
- merge the next change of using wb_idx in phys_wb
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 68 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 10 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +-
4 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c12841..9ae9bd4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -962,7 +962,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
int num_lm, num_ctl, num_pp, num_dsc;
unsigned int dsc_mask = 0;
- enum dpu_hw_blk_type blk_type;
int i;
if (!drm_enc) {
@@ -1044,17 +1043,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
phys->hw_pp = dpu_enc->hw_pp[i];
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
- if (dpu_encoder_get_intf_mode(_enc->base) == INTF_MODE_WB_LINE)
- blk_type = DPU_HW_BLK_WB;
- else
- blk_type = DPU_HW_BLK_INTF;
+ if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
+ phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
- if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) {
- if (blk_type == DPU_HW_BLK_INTF)
- phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
- else if (blk_type == DPU_HW_BLK_WB)
- phys->hw_wb = dpu_rm_get_wb(_kms->rm,
phys->intf_idx);
- }
+ if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
+ phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
if (!phys->hw_intf && !phys->hw_wb) {
DPU_ERROR_ENC(dpu_enc,
@@ -1062,6 +1055,12 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
return;
}
+ if (phys->hw_intf && phys->hw_wb) {
+ DPU_ERROR_ENC(dpu_enc,
+ "invalid phys both intf and wb block at idx:
%d\n", i);
+ return;
+ }
+
phys->cached_mode = crtc_state->adjusted_mode;
if (phys->ops.atomic_mode_set)
phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
@@ -1201,7 +1200,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
mutex_unlock(_enc->enc_lock);
}
-static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg *catalog,
+static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
enum dpu_intf_type type, u32 controller_id)
{
int i = 0;
@@ -1213,16 +1212,28 @@ static enum dpu_intf dpu_encoder_get_intf_or_wb(struct
dpu_mdss_cfg *catalog,
return catalog->intf[i].id;
}
}
- } else {
- for (i = 0; i < catalog->wb_count; i++) {
- if (catalog->wb[i].id == controller_id)
- return catalog->wb[i].id;
- }
}
return INTF_MAX;
}
+static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
+ enum dpu_intf_type type, u32 controller_id)
+{
+ int i = 0;
+
+ if (type != INTF_WB)
+ goto end;
+
+ for (i = 0; i < catalog->wb_count; i++) {
+ if (catalog->wb[i].id == controller_id)
+ return catalog->wb[i].id;
+ }
+
+end:
+ return WB_MAX;
+}
+
static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phy_enc)
{
@@ -2249,18 +2260,21 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
- /*
-* FIXME: have separate intf_idx and wb_idx to avoid using
-* enum dpu_intf type for wb_idx and also to be
[Freedreno] [PATCH v2 3/3] drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder
Change the DRM traces to include both the intf_mode
and wb_idx similar to the DRM prints in the previous change.
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 -
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 26 ++
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7bd20fa..8255c46 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1353,8 +1353,9 @@ static void dpu_encoder_frame_done_callback(
* suppress frame_done without waiter,
* likely autorefresh
*/
- trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc),
- event, ready_phys->intf_idx);
+ trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc),
event,
+
dpu_encoder_helper_get_intf_type(ready_phys->intf_mode),
+ ready_phys->intf_idx,
ready_phys->wb_idx);
return;
}
@@ -1432,9 +1433,11 @@ static void _dpu_encoder_trigger_flush(struct
drm_encoder *drm_enc,
if (ctl->ops.get_pending_flush)
ret = ctl->ops.get_pending_flush(ctl);
- trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx,
- pending_kickoff_cnt, ctl->idx,
- extra_flush_bits, ret);
+ trace_dpu_enc_trigger_flush(DRMID(drm_enc),
+ dpu_encoder_helper_get_intf_type(phys->intf_mode),
+ phys->intf_idx, phys->wb_idx,
+ pending_kickoff_cnt, ctl->idx,
+ extra_flush_bits, ret);
}
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 58b411f..1106d44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -380,20 +380,26 @@ TRACE_EVENT(dpu_enc_rc,
);
TRACE_EVENT(dpu_enc_frame_done_cb_not_busy,
- TP_PROTO(uint32_t drm_id, u32 event, enum dpu_intf intf_idx),
- TP_ARGS(drm_id, event, intf_idx),
+ TP_PROTO(uint32_t drm_id, u32 event, char *intf_mode, enum dpu_intf
intf_idx,
+ enum dpu_wb wb_idx),
+ TP_ARGS(drm_id, event, intf_mode, intf_idx, wb_idx),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
__field(u32,event )
+ __string( intf_mode_str, intf_mode )
__field(enum dpu_intf, intf_idx)
+ __field(enum dpu_wb, wb_idx)
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->event = event;
+ __assign_str(intf_mode_str, intf_mode);
__entry->intf_idx = intf_idx;
+ __entry->wb_idx = wb_idx;
),
- TP_printk("id=%u, event=%u, intf=%d", __entry->drm_id, __entry->event,
- __entry->intf_idx)
+ TP_printk("id=%u, event=%u, intf_mode=%s intf=%d wb=%d",
__entry->drm_id,
+ __entry->event, __get_str(intf_mode_str),
+ __entry->intf_idx, __entry->wb_idx)
);
TRACE_EVENT(dpu_enc_frame_done_cb,
@@ -415,14 +421,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
);
TRACE_EVENT(dpu_enc_trigger_flush,
- TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx,
+ TP_PROTO(uint32_t drm_id, char *intf_mode, enum dpu_intf intf_idx, enum
dpu_wb wb_idx,
int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits,
u32 pending_flush_ret),
- TP_ARGS(drm_id, intf_idx, pending_kickoff_cnt, ctl_idx,
+ TP_ARGS(drm_id, intf_mode, intf_idx, pending_kickoff_cnt, ctl_idx,
extra_flush_bits, pending_flush_ret),
TP_STRUCT__entry(
__field(uint32_t, drm_id )
+ __string( intf_mode_str, intf_mode )
__field(enum dpu_intf, intf_idx)
+ __field(enum dpu_wb, wb_idx)
__field(int,pending_kickoff_cnt )
__field(int,ctl_idx )
__field(u32,extra_flush_bits)
@@ -430,15 +438,17 @@ TRACE_EVENT(dpu_enc_trigger_flush,
),
TP_fast_assign(
__entry->drm_id = drm_id;
+ __assign_str(intf_mode_str, intf_mode);
__entry->intf_idx = intf_idx;
+ __entry->wb_idx = wb_idx;
__entry->pending_kickoff_cnt =
[Freedreno] [PATCH v2 2/3] drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder
Add wb_idx to existing DRM prints in dpu_encoder and also
print the intf_mode so that its clear that for any INTF_CMD/VID
there will be a valid intf_idx and any INTF_WB_* there will be a
valid wb_idx.
Update the debugfs to add the same information. Here is a sample
output with this change:
root:/sys/kernel/debug/dri/0/encoder31# cat status
intf:1 wb:-1 vsync: 31 underrun: 0mode: INTF_MODE_VIDEO
root:/sys/kernel/debug/dri/0/encoder33# cat status
intf:-1 wb:2 vsync: 7 underrun: 0mode: INTF_MODE_WB_LINE
Also remove DPU_DEBUG_PHYS macros as its unused because the
respective dpu_encoder_phys_* files have their own macros.
changes in v2:
- use switch case instead of if/else-if for get_intf_type
Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 52 +
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9ae9bd4..7bd20fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -37,18 +37,6 @@
#define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\
(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
-#define DPU_DEBUG_PHYS(p, fmt, ...) DRM_DEBUG_ATOMIC("enc%d intf%d pp%d " fmt,\
- (p) ? (p)->parent->base.id : -1, \
- (p) ? (p)->intf_idx - INTF_0 : -1, \
- (p) ? ((p)->hw_pp ? (p)->hw_pp->idx - PINGPONG_0 : -1) : -1, \
- ##__VA_ARGS__)
-
-#define DPU_ERROR_PHYS(p, fmt, ...) DPU_ERROR("enc%d intf%d pp%d " fmt,\
- (p) ? (p)->parent->base.id : -1, \
- (p) ? (p)->intf_idx - INTF_0 : -1, \
- (p) ? ((p)->hw_pp ? (p)->hw_pp->idx - PINGPONG_0 : -1) : -1, \
- ##__VA_ARGS__)
-
/*
* Two to anticipate panels that can do cmd/vid dynamic switching
* plan is to create all possible physical encoder types, and switch between
@@ -262,12 +250,30 @@ static void _dpu_encoder_setup_dither(struct
dpu_hw_pingpong *hw_pp, unsigned bp
hw_pp->ops.setup_dither(hw_pp, _cfg);
}
+static char *dpu_encoder_helper_get_intf_type(enum dpu_intf_mode intf_mode)
+{
+ switch (intf_mode) {
+ case INTF_MODE_VIDEO:
+ return "INTF_MODE_VIDEO";
+ case INTF_MODE_CMD:
+ return "INTF_MODE_CMD";
+ case INTF_MODE_WB_BLOCK:
+ return "INTF_MODE_WB_BLOCK";
+ case INTF_MODE_WB_LINE:
+ return "INTF_MODE_WB_LINE";
+ default:
+ return "INTF_MODE_UNKNOWN";
+ }
+}
+
void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx)
{
- DRM_ERROR("irq timeout id=%u, intf=%d, pp=%d, intr=%d\n",
- DRMID(phys_enc->parent), phys_enc->intf_idx - INTF_0,
- phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
+ DRM_ERROR("irq timeout id=%u, intf_mode=%s intf=%d wb=%d, pp=%d,
intr=%d\n",
+ DRMID(phys_enc->parent),
+ dpu_encoder_helper_get_intf_type(phys_enc->intf_mode),
+ phys_enc->intf_idx - INTF_0, phys_enc->wb_idx - WB_0,
+ phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
if (phys_enc->parent_ops->handle_frame_done)
phys_enc->parent_ops->handle_frame_done(
@@ -2048,22 +2054,12 @@ static int _dpu_encoder_status_show(struct seq_file *s,
void *data)
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
- seq_printf(s, "intf:%dvsync:%8d underrun:%8d",
- phys->intf_idx - INTF_0,
+ seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d",
+ phys->intf_idx - INTF_0, phys->wb_idx - WB_0,
atomic_read(>vsync_cnt),
atomic_read(>underrun_cnt));
- switch (phys->intf_mode) {
- case INTF_MODE_VIDEO:
- seq_puts(s, "mode: video\n");
- break;
- case INTF_MODE_CMD:
- seq_puts(s, "mode: command\n");
- break;
- default:
- seq_puts(s, "mode: ???\n");
- break;
- }
+ seq_printf(s, "mode: %s\n",
dpu_encoder_helper_get_intf_type(phys->intf_mode));
}
mutex_unlock(_enc->enc_lock);
--
2.7.4
[Freedreno] [PATCH v2 1/3] drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces
Using intf_idx even for writeback interfaces is confusing
because intf_idx is of type enum dpu_intf but the index used
for writeback is of type enum dpu_wb.
In addition, this makes it easier to separately check the
availability of the two as its possible that there are boards
which don't have any physical display connected but can still
work in writeback mode.
changes in v2:
- add check for invalid phys having both intf and wb
- merge the next change of using wb_idx in phys_wb
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 68 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 10 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +-
4 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c12841..9ae9bd4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -962,7 +962,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
int num_lm, num_ctl, num_pp, num_dsc;
unsigned int dsc_mask = 0;
- enum dpu_hw_blk_type blk_type;
int i;
if (!drm_enc) {
@@ -1044,17 +1043,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
phys->hw_pp = dpu_enc->hw_pp[i];
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
- if (dpu_encoder_get_intf_mode(_enc->base) ==
INTF_MODE_WB_LINE)
- blk_type = DPU_HW_BLK_WB;
- else
- blk_type = DPU_HW_BLK_INTF;
+ if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
+ phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
- if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) {
- if (blk_type == DPU_HW_BLK_INTF)
- phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
- else if (blk_type == DPU_HW_BLK_WB)
- phys->hw_wb = dpu_rm_get_wb(_kms->rm,
phys->intf_idx);
- }
+ if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
+ phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
if (!phys->hw_intf && !phys->hw_wb) {
DPU_ERROR_ENC(dpu_enc,
@@ -1062,6 +1055,12 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
return;
}
+ if (phys->hw_intf && phys->hw_wb) {
+ DPU_ERROR_ENC(dpu_enc,
+ "invalid phys both intf and wb block at
idx: %d\n", i);
+ return;
+ }
+
phys->cached_mode = crtc_state->adjusted_mode;
if (phys->ops.atomic_mode_set)
phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
@@ -1201,7 +1200,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
mutex_unlock(_enc->enc_lock);
}
-static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg *catalog,
+static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
enum dpu_intf_type type, u32 controller_id)
{
int i = 0;
@@ -1213,16 +1212,28 @@ static enum dpu_intf dpu_encoder_get_intf_or_wb(struct
dpu_mdss_cfg *catalog,
return catalog->intf[i].id;
}
}
- } else {
- for (i = 0; i < catalog->wb_count; i++) {
- if (catalog->wb[i].id == controller_id)
- return catalog->wb[i].id;
- }
}
return INTF_MAX;
}
+static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
+ enum dpu_intf_type type, u32 controller_id)
+{
+ int i = 0;
+
+ if (type != INTF_WB)
+ goto end;
+
+ for (i = 0; i < catalog->wb_count; i++) {
+ if (catalog->wb[i].id == controller_id)
+ return catalog->wb[i].id;
+ }
+
+end:
+ return WB_MAX;
+}
+
static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phy_enc)
{
@@ -2249,18 +2260,21 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
- /*
-* FIXME: have separate intf_idx and wb_idx to avoid using
-* enum dpu_intf type for wb_idx and also to be able to
-* not bail
[Freedreno] [PATCH v2 0/3] Separate wb_idx and intf_idx in dpu_encoder
As promised here [1], this is a follow up change to separate out wb_idx and intf_idx for better clarity in dpu_encoder. This also helps to easily handle boards which do not have a physical display but can still be validated using writeback interface. In addition, this also takes care of adding wb_idx to existing DRM prints and traces. Re-posting this without RFC tag to plan to absorb this in the DPU writeback series [2] [1] https://patchwork.freedesktop.org/patch/482637/?series=99724=2#comment_868460 [2] https://patchwork.freedesktop.org/series/99724/#rev3 changes in v2: - drop the RFC tag - add the check for phys->intf && phys->wb - squash changes 1&2 of the previous series Abhinav Kumar (3): drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces drm/msm/dpu: add wb_idx to existing DRM prints in dpu_encoder drm/msm/dpu: add wb_idx to DRM traces in dpu_encoder drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 133 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 26 ++-- 5 files changed, 101 insertions(+), 74 deletions(-) -- 2.7.4
Re: [Freedreno] [RFC 1/4] drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces
On Fri, 22 Apr 2022 at 21:18, Abhinav Kumar wrote:
>
> Hi Dmitry
>
> On 4/22/2022 3:37 AM, Dmitry Baryshkov wrote:
> > On Fri, 22 Apr 2022 at 04:59, Abhinav Kumar
> > wrote:
> >>
> >>
> >>
> >> On 4/21/2022 5:22 PM, Dmitry Baryshkov wrote:
> >>> On Fri, 22 Apr 2022 at 02:07, Abhinav Kumar
> >>> wrote:
>
> Hi Dmitry
>
> Thanks for the review.
>
> One question below.
>
> On 4/21/2022 3:40 PM, Dmitry Baryshkov wrote:
> > On 21/04/2022 23:48, Abhinav Kumar wrote:
> >> Using intf_idx even for writeback interfaces is confusing
> >> because intf_idx is of type enum dpu_intf but the index used
> >> for writeback is of type enum dpu_wb.
> >>
> >> In addition, this makes it easier to separately check the
> >> availability of the two as its possible that there are boards
> >> which don't have any physical display connected but can still
> >> work in writeback mode.
> >>
> >> Signed-off-by: Abhinav Kumar
> >
> > Looks good, two minor issues bellow.
> >
> > With them fixed, I'd even squash this patch into the corresponding patch
> > of the previous patchset.
> >
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 62
> >> +---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +-
> >> 3 files changed, 40 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 9c12841..054d7e4 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -962,7 +962,6 @@ static void
> >> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> >> struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> >> int num_lm, num_ctl, num_pp, num_dsc;
> >> unsigned int dsc_mask = 0;
> >> -enum dpu_hw_blk_type blk_type;
> >> int i;
> >> if (!drm_enc) {
> >> @@ -1044,17 +1043,11 @@ static void
> >> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> >> phys->hw_pp = dpu_enc->hw_pp[i];
> >> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
> >> -if (dpu_encoder_get_intf_mode(_enc->base) ==
> >> INTF_MODE_WB_LINE)
> >> -blk_type = DPU_HW_BLK_WB;
> >> -else
> >> -blk_type = DPU_HW_BLK_INTF;
> >> +if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
> >> +phys->hw_intf = dpu_rm_get_intf(_kms->rm,
> >> phys->intf_idx);
> >> -if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) {
> >> -if (blk_type == DPU_HW_BLK_INTF)
> >> -phys->hw_intf = dpu_rm_get_intf(_kms->rm,
> >> phys->intf_idx);
> >> -else if (blk_type == DPU_HW_BLK_WB)
> >> -phys->hw_wb = dpu_rm_get_wb(_kms->rm,
> >> phys->intf_idx);
> >> -}
> >> +if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
> >> +phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
> >
> > We also need a check for if (phus->hw_intf && phys->hw_wb) HERE
>
> So there is an error if
>
> 1) Neither wb NOR intf are present
> 2) Both wb AND intf are present for the physical encoder?
>
> The second check is okay for now to add but considering concurrent
> writeback then that wouldn't assumption be wrong since both physical and
> wb interfaces can go with the same encoder?
> >>>
> >>> To the same encoder, but not to the same physical encoder. Here we
> >>> check the phys_enc parameters.
> >>
> >> Ok got it, let me re-spin this RFC with patches 2 & 3 squashed.
> >> Get the acks on them.
> >>
> >> Then will absorb into WB series and re-post it.
> >
> > Sounds like a good plan!
> >
> >>
> >>>
>
> >
> >> if (!phys->hw_intf && !phys->hw_wb) {
> >> DPU_ERROR_ENC(dpu_enc,
> >> @@ -1201,7 +1194,7 @@ static void dpu_encoder_virt_disable(struct
> >> drm_encoder *drm_enc)
> >> mutex_unlock(_enc->enc_lock);
> >> }
> >> -static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg
> >> *catalog,
> >> +static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg
> >> *catalog,
> >> enum dpu_intf_type type, u32 controller_id)
> >> {
> >> int i = 0;
> >> @@ -1213,16 +1206,28 @@ static enum dpu_intf
> >> dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg *catalog,
> >> return catalog->intf[i].id;
> >> }
> >> }
> >> -} else {
> >> -for (i = 0; i < catalog->wb_count; i++) {
> >> -
Re: [Freedreno] [PATCH] drm/msm/dp: fix error check return value of irq_of_parse_and_map()
Quoting cgel@gmail.com (2022-04-22 01:49:51)
> From: Lv Ruyi
>
> The irq_of_parse_and_map() function returns 0 on failure, and does not
> return an negative value.
>
> Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
> Chipsets")
> Reported-by: Zeal Robot
> Signed-off-by: Lv Ruyi
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index a42732b67349..3926d2ac107d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1239,7 +1239,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
> dp = container_of(dp_display, struct dp_display_private, dp_display);
>
> dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
Why can't platform_get_irq() be used?
> - if (dp->irq < 0) {
> + if (!dp->irq) {
> rc = dp->irq;
zero as an error return value is an error?
> DRM_ERROR("failed to get irq: %d\n", rc);
> return rc;
Re: [Freedreno] [RFC 1/4] drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces
Hi Dmitry
On 4/22/2022 3:37 AM, Dmitry Baryshkov wrote:
On Fri, 22 Apr 2022 at 04:59, Abhinav Kumar wrote:
On 4/21/2022 5:22 PM, Dmitry Baryshkov wrote:
On Fri, 22 Apr 2022 at 02:07, Abhinav Kumar wrote:
Hi Dmitry
Thanks for the review.
One question below.
On 4/21/2022 3:40 PM, Dmitry Baryshkov wrote:
On 21/04/2022 23:48, Abhinav Kumar wrote:
Using intf_idx even for writeback interfaces is confusing
because intf_idx is of type enum dpu_intf but the index used
for writeback is of type enum dpu_wb.
In addition, this makes it easier to separately check the
availability of the two as its possible that there are boards
which don't have any physical display connected but can still
work in writeback mode.
Signed-off-by: Abhinav Kumar
Looks good, two minor issues bellow.
With them fixed, I'd even squash this patch into the corresponding patch
of the previous patchset.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 62
+---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +-
3 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c12841..054d7e4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -962,7 +962,6 @@ static void
dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
int num_lm, num_ctl, num_pp, num_dsc;
unsigned int dsc_mask = 0;
-enum dpu_hw_blk_type blk_type;
int i;
if (!drm_enc) {
@@ -1044,17 +1043,11 @@ static void
dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
phys->hw_pp = dpu_enc->hw_pp[i];
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
-if (dpu_encoder_get_intf_mode(_enc->base) ==
INTF_MODE_WB_LINE)
-blk_type = DPU_HW_BLK_WB;
-else
-blk_type = DPU_HW_BLK_INTF;
+if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
+phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
-if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) {
-if (blk_type == DPU_HW_BLK_INTF)
-phys->hw_intf = dpu_rm_get_intf(_kms->rm,
phys->intf_idx);
-else if (blk_type == DPU_HW_BLK_WB)
-phys->hw_wb = dpu_rm_get_wb(_kms->rm,
phys->intf_idx);
-}
+if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
+phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
We also need a check for if (phus->hw_intf && phys->hw_wb) HERE
So there is an error if
1) Neither wb NOR intf are present
2) Both wb AND intf are present for the physical encoder?
The second check is okay for now to add but considering concurrent
writeback then that wouldn't assumption be wrong since both physical and
wb interfaces can go with the same encoder?
To the same encoder, but not to the same physical encoder. Here we
check the phys_enc parameters.
Ok got it, let me re-spin this RFC with patches 2 & 3 squashed.
Get the acks on them.
Then will absorb into WB series and re-post it.
Sounds like a good plan!
if (!phys->hw_intf && !phys->hw_wb) {
DPU_ERROR_ENC(dpu_enc,
@@ -1201,7 +1194,7 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
mutex_unlock(_enc->enc_lock);
}
-static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg
*catalog,
+static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
enum dpu_intf_type type, u32 controller_id)
{
int i = 0;
@@ -1213,16 +1206,28 @@ static enum dpu_intf
dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg *catalog,
return catalog->intf[i].id;
}
}
-} else {
-for (i = 0; i < catalog->wb_count; i++) {
-if (catalog->wb[i].id == controller_id)
-return catalog->wb[i].id;
-}
}
return INTF_MAX;
}
+static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
+enum dpu_intf_type type, u32 controller_id)
+{
+int i = 0;
+
+if (type != INTF_WB)
+goto end;
+
+for (i = 0; i < catalog->wb_count; i++) {
+if (catalog->wb[i].id == controller_id)
+return catalog->wb[i].id;
+}
+
+end:
+return WB_MAX;
I'd return INTF_NONE/WB_NONE if the interface or WB unit was not found.
ack, i guess in that case even the places checking the return value of
this function need to be changed.
Yes, of course.
INTF_NONE/WB_NONE is not of enum dpu_intf or enum dpu_wb, its of type
enum dpu_intf_mode
Do we want to add them to dpu_intf/dpu_wb with a -1 value OR leave it
as-it-is.
+}
+
static void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct
Re: [Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Hi, On Fri, Apr 22, 2022 at 9:05 AM Abhinav Kumar wrote: > > Hi Doug > > For the lockdep error, the splat looks similar to what kuogee fixed > recently. > > Can you please check if below patch is present in your tree? > > https://patchwork.freedesktop.org/patch/481396/ Indeed I did have that in my tree already, but the lockdep splat is still there. I think the problem is that we're now calling dp_hpd_plug_handle() directly in dp_bridge_enable() -Doug
Re: [Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Hi Doug
For the lockdep error, the splat looks similar to what kuogee fixed
recently.
Can you please check if below patch is present in your tree?
https://patchwork.freedesktop.org/patch/481396/
Thanks
Abhinav
On 4/22/2022 8:55 AM, Doug Anderson wrote:
Hi,
On Fri, Apr 22, 2022 at 2:11 AM Sankeerth Billakanti
wrote:
The panel-edp enables the eDP panel power during probe, get_modes
and pre-enable. The eDP connect and disconnect interrupts for the eDP/DP
controller are directly dependent on panel power. As eDP display can be
assumed as always connected, the controller driver can skip the eDP
connect and disconnect interrupts. Any disruption in the link status
will be indicated via the IRQ_HPD interrupts.
So, the eDP controller driver can just enable the IRQ_HPD and replug
interrupts. The DP controller driver still needs to enable all the
interrupts.
Signed-off-by: Sankeerth Billakanti
---
Changes in v9:
- add comment explaining the interrupt status register
Changes in v8:
- add comment explaining the interrupt status return
Changes in v7:
- reordered the patch in the series
- modified the return statement for isr
- connector check modified to just check for eDP
drivers/gpu/drm/msm/dp/dp_catalog.c | 16 ++--
drivers/gpu/drm/msm/dp/dp_display.c | 22 +-
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index fac815f..df9670d 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
*dp_catalog)
u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
- /* enable HPD plug and unplug interrupts */
- dp_catalog_hpd_config_intr(dp_catalog,
- DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true);
-
/* Configure REFTIMER and enable it */
reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer);
@@ -599,13 +595,21 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog
*dp_catalog)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- int isr = 0;
+ int isr, mask;
isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK,
(isr & DP_DP_HPD_INT_MASK));
+ mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK);
- return isr;
+ /*
+* We only want to return interrupts that are unmasked to the caller.
+* However, the interrupt status field also contains other
+* informational bits about the HPD state status, so we only mask
+* out the part of the register that tells us about which interrupts
+* are pending.
+*/
+ return isr & (mask | ~DP_DP_HPD_INT_MASK);
}
int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 055681a..dea4de9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -683,7 +683,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
dp_display_handle_plugged_change(>dp_display, false);
/* enable HDP plug interrupt to prepare for next plugin */
- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK, true);
DRM_DEBUG_DP("After, type=%d hpd_state=%d\n",
dp->dp_display.connector_type, state);
@@ -1096,6 +1097,13 @@ static void dp_display_config_hpd(struct
dp_display_private *dp)
dp_display_host_init(dp);
dp_catalog_ctrl_hpd_config(dp->catalog);
+ /* Enable plug and unplug interrupts only for external DisplayPort */
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
/* Enable interrupt first time
* we are leaving dp clocks on during disconnect
* and never disable interrupt
@@ -1381,6 +1389,12 @@ static int dp_pm_resume(struct device *dev)
dp_catalog_ctrl_hpd_config(dp->catalog);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
if (dp_catalog_link_is_connected(dp->catalog)) {
/*
* set sink to normal operation mode -- D0
Re: [Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Hi,
On Fri, Apr 22, 2022 at 2:11 AM Sankeerth Billakanti
wrote:
>
> The panel-edp enables the eDP panel power during probe, get_modes
> and pre-enable. The eDP connect and disconnect interrupts for the eDP/DP
> controller are directly dependent on panel power. As eDP display can be
> assumed as always connected, the controller driver can skip the eDP
> connect and disconnect interrupts. Any disruption in the link status
> will be indicated via the IRQ_HPD interrupts.
>
> So, the eDP controller driver can just enable the IRQ_HPD and replug
> interrupts. The DP controller driver still needs to enable all the
> interrupts.
>
> Signed-off-by: Sankeerth Billakanti
> ---
> Changes in v9:
> - add comment explaining the interrupt status register
>
> Changes in v8:
> - add comment explaining the interrupt status return
>
> Changes in v7:
> - reordered the patch in the series
> - modified the return statement for isr
> - connector check modified to just check for eDP
>
> drivers/gpu/drm/msm/dp/dp_catalog.c | 16 ++--
> drivers/gpu/drm/msm/dp/dp_display.c | 22 +-
> 2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index fac815f..df9670d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
> *dp_catalog)
>
> u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
>
> - /* enable HPD plug and unplug interrupts */
> - dp_catalog_hpd_config_intr(dp_catalog,
> - DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true);
> -
> /* Configure REFTIMER and enable it */
> reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
> dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer);
> @@ -599,13 +595,21 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog
> *dp_catalog)
> {
> struct dp_catalog_private *catalog = container_of(dp_catalog,
> struct dp_catalog_private, dp_catalog);
> - int isr = 0;
> + int isr, mask;
>
> isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
> dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK,
> (isr & DP_DP_HPD_INT_MASK));
> + mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK);
>
> - return isr;
> + /*
> +* We only want to return interrupts that are unmasked to the caller.
> +* However, the interrupt status field also contains other
> +* informational bits about the HPD state status, so we only mask
> +* out the part of the register that tells us about which interrupts
> +* are pending.
> +*/
> + return isr & (mask | ~DP_DP_HPD_INT_MASK);
> }
>
> int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog)
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 055681a..dea4de9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -683,7 +683,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private
> *dp, u32 data)
> dp_display_handle_plugged_change(>dp_display, false);
>
> /* enable HDP plug interrupt to prepare for next plugin */
> - dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK,
> true);
> + if (!dp->dp_display.is_edp)
> + dp_catalog_hpd_config_intr(dp->catalog,
> DP_DP_HPD_PLUG_INT_MASK, true);
>
> DRM_DEBUG_DP("After, type=%d hpd_state=%d\n",
> dp->dp_display.connector_type, state);
> @@ -1096,6 +1097,13 @@ static void dp_display_config_hpd(struct
> dp_display_private *dp)
> dp_display_host_init(dp);
> dp_catalog_ctrl_hpd_config(dp->catalog);
>
> + /* Enable plug and unplug interrupts only for external DisplayPort */
> + if (!dp->dp_display.is_edp)
> + dp_catalog_hpd_config_intr(dp->catalog,
> + DP_DP_HPD_PLUG_INT_MASK |
> + DP_DP_HPD_UNPLUG_INT_MASK,
> + true);
> +
> /* Enable interrupt first time
> * we are leaving dp clocks on during disconnect
> * and never disable interrupt
> @@ -1381,6 +1389,12 @@ static int dp_pm_resume(struct device *dev)
> dp_catalog_ctrl_hpd_config(dp->catalog);
>
>
> + if (!dp->dp_display.is_edp)
> + dp_catalog_hpd_config_intr(dp->catalog,
> + DP_DP_HPD_PLUG_INT_MASK |
> + DP_DP_HPD_UNPLUG_INT_MASK,
> + true);
> +
> if (dp_catalog_link_is_connected(dp->catalog)) {
> /*
> * set sink to normal operation mode -- D0
> @@ -1659,6 +1673,9 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
Re: [Freedreno] [PATCH v7 1/5] drm/msm/disp/dpu1: set mdp clk to the maximum frequency in opp table during probe
Hi,
On Mon, Mar 21, 2022 at 8:27 PM Vinod Polimera
wrote:
>
> Set mdp clock to max clock rate during probe/bind sequence from the
> opp table so that rails are not at undetermined state. Since we do not
> know what will be the rate set in boot loader, it would be ideal to
> vote at max frequency. There could be a firmware display programmed
> in bootloader and we want to transition it to kernel without underflowing.
> The clock will be scaled down later when framework sends an update.
>
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Vinod Polimera
> Reviewed-by: Dmitry Baryshkov
> Reviewed-by: Douglas Anderson
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8
> 1 file changed, 8 insertions(+)
Just wanted to confirm that this patch will be queued up somewhat
soon. I think it's good to go but I don't see it in any trees yet. ;-)
FWIW, I can also say that I've tested this patch and it fixes the
underrun issues on sc7280-herobrine-rev1.
Tested-by: Douglas Anderson
-Doug
Re: [Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Hi, On Fri, Apr 22, 2022 at 2:11 AM Sankeerth Billakanti wrote: > > The panel-edp enables the eDP panel power during probe, get_modes > and pre-enable. The eDP connect and disconnect interrupts for the eDP/DP > controller are directly dependent on panel power. As eDP display can be > assumed as always connected, the controller driver can skip the eDP > connect and disconnect interrupts. Any disruption in the link status > will be indicated via the IRQ_HPD interrupts. > > So, the eDP controller driver can just enable the IRQ_HPD and replug > interrupts. The DP controller driver still needs to enable all the > interrupts. > > Signed-off-by: Sankeerth Billakanti > --- > Changes in v9: > - add comment explaining the interrupt status register > > Changes in v8: > - add comment explaining the interrupt status return > > Changes in v7: > - reordered the patch in the series > - modified the return statement for isr > - connector check modified to just check for eDP > > drivers/gpu/drm/msm/dp/dp_catalog.c | 16 ++-- > drivers/gpu/drm/msm/dp/dp_display.c | 22 +- > 2 files changed, 31 insertions(+), 7 deletions(-) Reviewed-by: Douglas Anderson
Re: [Freedreno] [PATCH v9 1/4] drm/msm/dp: Add eDP support via aux_bus
Hi, On Fri, Apr 22, 2022 at 2:11 AM Sankeerth Billakanti wrote: > > This patch adds support for generic eDP sink through aux_bus. The eDP/DP > controller driver should support aux transactions originating from the > panel-edp driver and hence should be initialized and ready. > > The panel bridge supporting the panel should be ready before the bridge > connector is initialized. The generic panel probe needs the controller > resources to be enabled to support the aux transactions originating from > the panel probe. > > Signed-off-by: Sankeerth Billakanti > --- > Changes in v9: > - add comments for panel probe > - modify the error handling checks > > Changes in v8: > - handle corner cases > - add comment for the bridge ops > > Changes in v7: > - aux_bus is mandatory for eDP > - connector type check modified to just check for eDP > > Changes in v6: > - Remove initialization > - Fix aux_bus node leak > - Split the patches > > drivers/gpu/drm/msm/dp/dp_display.c | 73 > +++-- > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dp/dp_drm.c | 21 +-- > drivers/gpu/drm/msm/dp/dp_parser.c | 23 +--- > drivers/gpu/drm/msm/dp/dp_parser.h | 13 ++- > 5 files changed, 101 insertions(+), 30 deletions(-) Reviewed-by: Douglas Anderson
Re: [Freedreno] [PATCH] drm: msm: fix error check return value of irq_of_parse_and_map()
Il 22/04/22 10:52, cgel@gmail.com ha scritto: From: Lv Ruyi The irq_of_parse_and_map() function returns 0 on failure, and does not return an negative value. Reported-by: Zeal Robot Signed-off-by: Lv Ruyi Reviewed-by: AngeloGioacchino Del Regno
Re: [Freedreno] [PATCH] drm: msm: fix error check return value of irq_of_parse_and_map()
On Fri, 22 Apr 2022 at 11:52, wrote:
>
> From: Lv Ruyi
>
> The irq_of_parse_and_map() function returns 0 on failure, and does not
> return an negative value.
>
> Reported-by: Zeal Robot
> Signed-off-by: Lv Ruyi
Reviewed-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 3b92372e7bdf..1fb1ed9e95d9 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -570,7 +570,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
> }
>
> irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> - if (irq < 0) {
> + if (!irq) {
> ret = irq;
> DRM_DEV_ERROR(>dev, "failed to get irq: %d\n", ret);
> goto fail;
> --
> 2.25.1
>
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH] drm/msm/dp: fix error check return value of irq_of_parse_and_map()
On Fri, 22 Apr 2022 at 11:50, wrote:
>
> From: Lv Ruyi
>
> The irq_of_parse_and_map() function returns 0 on failure, and does not
> return an negative value.
>
> Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
> Chipsets")
> Reported-by: Zeal Robot
> Signed-off-by: Lv Ruyi
Reviewed-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index a42732b67349..3926d2ac107d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1239,7 +1239,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
> dp = container_of(dp_display, struct dp_display_private, dp_display);
>
> dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
> - if (dp->irq < 0) {
> + if (!dp->irq) {
> rc = dp->irq;
> DRM_ERROR("failed to get irq: %d\n", rc);
> return rc;
> --
> 2.25.1
>
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH] drm/msm/hdmi: check return value after calling platform_get_resource_byname()
On Fri, 22 Apr 2022 at 06:10, Yang Yingliang wrote:
>
> It will cause null-ptr-deref if platform_get_resource_byname() returns NULL,
> we need check the return value.
>
> Fixes: c6a57a50ad56 ("drm/msm/hdmi: add hdmi hdcp support (V3)")
> Signed-off-by: Yang Yingliang
Reviewed-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/hdmi/hdmi.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index ec324352e862..07e2ad527af9 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -142,6 +142,10 @@ static struct hdmi *msm_hdmi_init(struct platform_device
> *pdev)
> /* HDCP needs physical address of hdmi register */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> config->mmio_name);
> + if (!res) {
> + ret = -EINVAL;
> + goto fail;
> + }
> hdmi->mmio_phy_addr = res->start;
>
> hdmi->qfprom_mmio = msm_ioremap(pdev, config->qfprom_mmio_name);
> --
> 2.25.1
>
--
With best wishes
Dmitry
Re: [Freedreno] [RFC 1/4] drm/msm/dpu: introduce a wb_idx to be used for writeback interfaces
On Fri, 22 Apr 2022 at 04:59, Abhinav Kumar wrote:
>
>
>
> On 4/21/2022 5:22 PM, Dmitry Baryshkov wrote:
> > On Fri, 22 Apr 2022 at 02:07, Abhinav Kumar
> > wrote:
> >>
> >> Hi Dmitry
> >>
> >> Thanks for the review.
> >>
> >> One question below.
> >>
> >> On 4/21/2022 3:40 PM, Dmitry Baryshkov wrote:
> >>> On 21/04/2022 23:48, Abhinav Kumar wrote:
> Using intf_idx even for writeback interfaces is confusing
> because intf_idx is of type enum dpu_intf but the index used
> for writeback is of type enum dpu_wb.
>
> In addition, this makes it easier to separately check the
> availability of the two as its possible that there are boards
> which don't have any physical display connected but can still
> work in writeback mode.
>
> Signed-off-by: Abhinav Kumar
> >>>
> >>> Looks good, two minor issues bellow.
> >>>
> >>> With them fixed, I'd even squash this patch into the corresponding patch
> >>> of the previous patchset.
> >>>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 62
> +---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +-
> 3 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 9c12841..054d7e4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -962,7 +962,6 @@ static void
> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> int num_lm, num_ctl, num_pp, num_dsc;
> unsigned int dsc_mask = 0;
> -enum dpu_hw_blk_type blk_type;
> int i;
> if (!drm_enc) {
> @@ -1044,17 +1043,11 @@ static void
> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> phys->hw_pp = dpu_enc->hw_pp[i];
> phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
> -if (dpu_encoder_get_intf_mode(_enc->base) ==
> INTF_MODE_WB_LINE)
> -blk_type = DPU_HW_BLK_WB;
> -else
> -blk_type = DPU_HW_BLK_INTF;
> +if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
> +phys->hw_intf = dpu_rm_get_intf(_kms->rm,
> phys->intf_idx);
> -if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX) {
> -if (blk_type == DPU_HW_BLK_INTF)
> -phys->hw_intf = dpu_rm_get_intf(_kms->rm,
> phys->intf_idx);
> -else if (blk_type == DPU_HW_BLK_WB)
> -phys->hw_wb = dpu_rm_get_wb(_kms->rm,
> phys->intf_idx);
> -}
> +if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
> +phys->hw_wb = dpu_rm_get_wb(_kms->rm, phys->wb_idx);
> >>>
> >>> We also need a check for if (phus->hw_intf && phys->hw_wb) HERE
> >>
> >> So there is an error if
> >>
> >> 1) Neither wb NOR intf are present
> >> 2) Both wb AND intf are present for the physical encoder?
> >>
> >> The second check is okay for now to add but considering concurrent
> >> writeback then that wouldn't assumption be wrong since both physical and
> >> wb interfaces can go with the same encoder?
> >
> > To the same encoder, but not to the same physical encoder. Here we
> > check the phys_enc parameters.
>
> Ok got it, let me re-spin this RFC with patches 2 & 3 squashed.
> Get the acks on them.
>
> Then will absorb into WB series and re-post it.
Sounds like a good plan!
>
> >
> >>
> >>>
> if (!phys->hw_intf && !phys->hw_wb) {
> DPU_ERROR_ENC(dpu_enc,
> @@ -1201,7 +1194,7 @@ static void dpu_encoder_virt_disable(struct
> drm_encoder *drm_enc)
> mutex_unlock(_enc->enc_lock);
> }
> -static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg
> *catalog,
> +static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
> enum dpu_intf_type type, u32 controller_id)
> {
> int i = 0;
> @@ -1213,16 +1206,28 @@ static enum dpu_intf
> dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg *catalog,
> return catalog->intf[i].id;
> }
> }
> -} else {
> -for (i = 0; i < catalog->wb_count; i++) {
> -if (catalog->wb[i].id == controller_id)
> -return catalog->wb[i].id;
> -}
> }
> return INTF_MAX;
> }
> +static enum dpu_wb dpu_encoder_get_wb(struct dpu_mdss_cfg *catalog,
> +enum dpu_intf_type type, u32 controller_id)
> +{
> +int i = 0;
> +
> +if (type != INTF_WB)
> +goto end;
>
Re: [Freedreno] [PATCH] clk: qcom: clk-rcg2: fix gfx3d frequency calculation
Hi,
On Thu, Apr 21, 2022 at 07:49:12PM -0700, Stephen Boyd wrote:
> +Maxime
>
> Quoting Dmitry Baryshkov (2022-04-19 16:54:47)
> > Since the commit 948fb0969eae ("clk: Always clamp the rounded rate"),
> > the clk_core_determine_round_nolock() would clamp the requested rate
> > between min and max rates from the rate request. Normally these fields
> > would be filled by clk_core_get_boundaries() called from
> > clk_round_rate().
> >
> > However clk_gfx3d_determine_rate() uses a manually crafted rate request,
> > which did not have these fields filled. Thus the requested frequency
> > would be clamped to 0, resulting in weird frequencies being requested
> > from the hardware.
> >
> > Fix this by filling min_rate and max_rate to the values valid for the
> > respective PLLs (0 and ULONG_MAX).
> >
> > Fixes: 948fb0969eae ("clk: Always clamp the rounded rate")
> > Signed-off-by: Dmitry Baryshkov
> > ---
>
> I hope there aren't others like this lurking.
The problem is larger than that (even though I overlooked this
particular issue), and addressed partially by patches 12-19 here:
https://lore.kernel.org/linux-clk/20220408091037.2041955-1-max...@cerno.tech/
I wanted to have your feedback before fixing the relevant drivers, but
these are:
* clk_divider:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-divider.c#L389
Only sunxi-ng is using divider_round_rate_parent, and I couldn't
find any clock with CLK_SET_RATE_PARENT, so this one is probably
minor. This one doesn't setup the boundaries and would probably
benefit from using clk_hw_init_rate_req.
* clk_composite:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-composite.c#L88
https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-composite.c#L107
Both are doing an ok job at setting up the parent, but won't set up
the boundaries there. They setup the parent properly though, and
will update the "parent" request best_parent_* fields too. Switching
to clk_core_forward_rate_req would probably help maintenance a bit.
It's used in davinci_pll_obsclk_register,
mtk_clk_register_composite, lpc18xx_cgu_register_div,
lpc18xx_register_base_clk, lpc18xx_cgu_register_pll,
lpc32xx_clk_register, clk_pxa_cken_init and of_ti_composite_clk_setup.
It's not really clear to me whether these clocks have a
clk_round_rate / clk_set_rate on them, but it looks pretty bad.
* at91:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/at91/clk-generated.c#L135
https://elixir.bootlin.com/linux/latest/source/drivers/clk/at91/clk-master.c#L381
https://elixir.bootlin.com/linux/latest/source/drivers/clk/at91/clk-peripheral.c#L272
The issue is the same addressed in my series. The clk_rate_request
structure is forwarded as is and only the rate is updated. The
best_parent_* and boundaries aren't updated. It should be switched
to use clk_core_forward_rate_req or something equivalent.
* qcom:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/qcom/clk-rcg2.c#L821
This is the one affected by the patch. It's doing a better job at
filling the clk_rate_request, but indeed doesn't update the
boundaries. This patch is probably sane from an clk_hw
point-of-view, but it's broken if any user has set a boundary. It
should probably be switched to clk_core_forward_rate_req or similar
as well. Since the logic seems fairly intricate, I'm not sure if it
would be convenient though.
Maxime
signature.asc
Description: PGP signature
[Freedreno] [PATCH v9 4/4] drm/msm/dp: Support the eDP modes given by panel
The eDP controller does not have a reliable way keep panel powered on to read the sink capabilities. So, the controller driver cannot validate if a mode can be supported by the source. We will rely on the panel driver to populate only the supported modes for now. Signed-off-by: Sankeerth Billakanti Reviewed-by: Douglas Anderson --- Changes in v9: - none Changes in v8: - add the drm/msm/dp tag in the commit title drivers/gpu/drm/msm/dp/dp_display.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index f197694..49fac955 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -998,6 +998,14 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, return -EINVAL; } + /* +* The eDP controller currently does not have a reliable way of +* enabling panel power to read sink capabilities. So, we rely +* on the panel driver to populate only supported modes for now. +*/ + if (dp->is_edp) + return MODE_OK; + if ((dp->max_pclk_khz <= 0) || (dp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) || (mode->clock > dp->max_pclk_khz)) -- 2.7.4
[Freedreno] [PATCH v9 3/4] drm/msm/dp: wait for hpd high before aux transaction
The source device should ensure the sink is ready before proceeding to
read the sink capability or perform any aux transactions. The sink
will indicate its readiness by asserting the HPD line. The controller
driver needs to wait for the hpd line to be asserted by the sink before
it performs any aux transactions.
The eDP sink is assumed to be always connected. It needs power from the
source and its HPD line will be asserted only after the panel is powered
on. The panel power will be enabled from the panel-edp driver and only
after that, the hpd line will be asserted.
Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd
line gets asserted to indicate the sink is connected and ready. Hence
there is no need to wait for the hpd line to be asserted for a DP sink.
Signed-off-by: Sankeerth Billakanti
Reviewed-by: Douglas Anderson
---
These changes may be handled in is_hpd_asserted when
https://lore.kernel.org/r/20220408193536.RFC.3.Icf57bb12233a47727013c6ab69eebf803e22ebc1@changeid/
is accepted upstream.
Changes in v9:
- none
Changes in v8:
- correct the indentation
Changes in v7:
- add a comment to say why the wait is done for eDP
- correct the commit text
Changes in v6:
- Wait for hpd high only for eDP
- Split into smaller patches
drivers/gpu/drm/msm/dp/dp_aux.c | 21 -
drivers/gpu/drm/msm/dp/dp_aux.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 13 +
drivers/gpu/drm/msm/dp/dp_catalog.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
5 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index 6d36f63..d030a93 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -34,6 +34,7 @@ struct dp_aux_private {
bool no_send_addr;
bool no_send_stop;
bool initted;
+ bool is_edp;
u32 offset;
u32 segment;
@@ -337,6 +338,22 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
goto exit;
}
+ /*
+* For eDP it's important to give a reasonably long wait here for HPD
+* to be asserted. This is because the panel driver may have _just_
+* turned on the panel and then tried to do an AUX transfer. The panel
+* driver has no way of knowing when the panel is ready, so it's up
+* to us to wait. For DP we never get into this situation so let's
+* avoid ever doing the extra long wait for DP.
+*/
+ if (aux->is_edp) {
+ ret = dp_catalog_aux_wait_for_hpd_connect_state(aux->catalog);
+ if (ret) {
+ DRM_DEBUG_DP("Panel not ready for aux transactions\n");
+ goto exit;
+ }
+ }
+
dp_aux_update_offset_and_segment(aux, msg);
dp_aux_transfer_helper(aux, msg, true);
@@ -491,7 +508,8 @@ void dp_aux_unregister(struct drm_dp_aux *dp_aux)
drm_dp_aux_unregister(dp_aux);
}
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog)
+struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
+ bool is_edp)
{
struct dp_aux_private *aux;
@@ -506,6 +524,7 @@ struct drm_dp_aux *dp_aux_get(struct device *dev, struct
dp_catalog *catalog)
init_completion(>comp);
aux->cmd_busy = false;
+ aux->is_edp = is_edp;
mutex_init(>mutex);
aux->dev = dev;
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
index 82afc8d..5a50c08 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.h
+++ b/drivers/gpu/drm/msm/dp/dp_aux.h
@@ -16,7 +16,8 @@ void dp_aux_init(struct drm_dp_aux *dp_aux);
void dp_aux_deinit(struct drm_dp_aux *dp_aux);
void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog);
+struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
+ bool is_edp);
void dp_aux_put(struct drm_dp_aux *aux);
#endif /*__DP_AUX_H_*/
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index df9670d..0c6a96e 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -242,6 +242,19 @@ void dp_catalog_aux_update_cfg(struct dp_catalog
*dp_catalog)
phy_calibrate(phy);
}
+int dp_catalog_aux_wait_for_hpd_connect_state(struct dp_catalog *dp_catalog)
+{
+ u32 state;
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ /* poll for hpd connected status every 2ms and timeout after 500ms */
+ return readl_poll_timeout(catalog->io->dp_controller.aux.base +
+ REG_DP_DP_HPD_INT_STATUS,
+ state, state & DP_DP_HPD_STATE_STATUS_CONNECTED,
[Freedreno] [PATCH v9 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
The panel-edp enables the eDP panel power during probe, get_modes
and pre-enable. The eDP connect and disconnect interrupts for the eDP/DP
controller are directly dependent on panel power. As eDP display can be
assumed as always connected, the controller driver can skip the eDP
connect and disconnect interrupts. Any disruption in the link status
will be indicated via the IRQ_HPD interrupts.
So, the eDP controller driver can just enable the IRQ_HPD and replug
interrupts. The DP controller driver still needs to enable all the
interrupts.
Signed-off-by: Sankeerth Billakanti
---
Changes in v9:
- add comment explaining the interrupt status register
Changes in v8:
- add comment explaining the interrupt status return
Changes in v7:
- reordered the patch in the series
- modified the return statement for isr
- connector check modified to just check for eDP
drivers/gpu/drm/msm/dp/dp_catalog.c | 16 ++--
drivers/gpu/drm/msm/dp/dp_display.c | 22 +-
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index fac815f..df9670d 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
*dp_catalog)
u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
- /* enable HPD plug and unplug interrupts */
- dp_catalog_hpd_config_intr(dp_catalog,
- DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true);
-
/* Configure REFTIMER and enable it */
reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer);
@@ -599,13 +595,21 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog
*dp_catalog)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- int isr = 0;
+ int isr, mask;
isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK,
(isr & DP_DP_HPD_INT_MASK));
+ mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK);
- return isr;
+ /*
+* We only want to return interrupts that are unmasked to the caller.
+* However, the interrupt status field also contains other
+* informational bits about the HPD state status, so we only mask
+* out the part of the register that tells us about which interrupts
+* are pending.
+*/
+ return isr & (mask | ~DP_DP_HPD_INT_MASK);
}
int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 055681a..dea4de9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -683,7 +683,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
dp_display_handle_plugged_change(>dp_display, false);
/* enable HDP plug interrupt to prepare for next plugin */
- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK, true);
DRM_DEBUG_DP("After, type=%d hpd_state=%d\n",
dp->dp_display.connector_type, state);
@@ -1096,6 +1097,13 @@ static void dp_display_config_hpd(struct
dp_display_private *dp)
dp_display_host_init(dp);
dp_catalog_ctrl_hpd_config(dp->catalog);
+ /* Enable plug and unplug interrupts only for external DisplayPort */
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
/* Enable interrupt first time
* we are leaving dp clocks on during disconnect
* and never disable interrupt
@@ -1381,6 +1389,12 @@ static int dp_pm_resume(struct device *dev)
dp_catalog_ctrl_hpd_config(dp->catalog);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
if (dp_catalog_link_is_connected(dp->catalog)) {
/*
* set sink to normal operation mode -- D0
@@ -1659,6 +1673,9 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
return;
}
+ if (dp->is_edp)
+ dp_hpd_plug_handle(dp_display, 0);
+
mutex_lock(_display->event_mutex);
/* stop sentinel checking */
@@ -1723,6 +1740,9 @@ void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
[Freedreno] [PATCH v9 1/4] drm/msm/dp: Add eDP support via aux_bus
This patch adds support for generic eDP sink through aux_bus. The eDP/DP
controller driver should support aux transactions originating from the
panel-edp driver and hence should be initialized and ready.
The panel bridge supporting the panel should be ready before the bridge
connector is initialized. The generic panel probe needs the controller
resources to be enabled to support the aux transactions originating from
the panel probe.
Signed-off-by: Sankeerth Billakanti
---
Changes in v9:
- add comments for panel probe
- modify the error handling checks
Changes in v8:
- handle corner cases
- add comment for the bridge ops
Changes in v7:
- aux_bus is mandatory for eDP
- connector type check modified to just check for eDP
Changes in v6:
- Remove initialization
- Fix aux_bus node leak
- Split the patches
drivers/gpu/drm/msm/dp/dp_display.c | 73 +++--
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 21 +--
drivers/gpu/drm/msm/dp/dp_parser.c | 23 +---
drivers/gpu/drm/msm/dp/dp_parser.h | 13 ++-
5 files changed, 101 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d7a19d6..055681a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include "msm_drv.h"
#include "msm_kms.h"
@@ -259,14 +260,12 @@ static int dp_display_bind(struct device *dev, struct
device *master,
dp->dp_display.drm_dev = drm;
priv->dp[dp->id] = >dp_display;
- rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
+ rc = dp->parser->parse(dp->parser);
if (rc) {
DRM_ERROR("device tree parsing failed\n");
goto end;
}
- dp->dp_display.next_bridge = dp->parser->next_bridge;
-
dp->aux->drm_dev = drm;
rc = dp_aux_register(dp->aux);
if (rc) {
@@ -1319,6 +1318,8 @@ static int dp_display_probe(struct platform_device *pdev)
dp->pdev = pdev;
dp->name = "drm_dp";
dp->dp_display.connector_type = desc->connector_type;
+ dp->dp_display.is_edp =
+ (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
rc = dp_init_sub_modules(dp);
if (rc) {
@@ -1508,7 +1509,8 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp_hpd_event_setup(dp);
- dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
+ if (!dp_display->is_edp)
+ dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
@@ -1530,6 +1532,65 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display,
struct drm_minor *minor)
}
}
+static int dp_display_get_next_bridge(struct msm_dp *dp)
+{
+ int rc;
+ struct dp_display_private *dp_priv;
+ struct device_node *aux_bus;
+ struct device *dev;
+
+ dp_priv = container_of(dp, struct dp_display_private, dp_display);
+ dev = _priv->pdev->dev;
+ aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
+
+ if (aux_bus && dp->is_edp) {
+ dp_display_host_init(dp_priv);
+ dp_catalog_ctrl_hpd_config(dp_priv->catalog);
+ dp_display_host_phy_init(dp_priv);
+ enable_irq(dp_priv->irq);
+
+ /*
+* The code below assumes that the panel will finish probing
+* by the time devm_of_dp_aux_populate_ep_devices() returns.
+* This isn't a great assumption since it will fail if the
+* panel driver is probed asynchronously but is the best we
+* can do without a bigger driver reorganization.
+*/
+ rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
+ of_node_put(aux_bus);
+ if (rc)
+ goto error;
+ } else if (dp->is_edp) {
+ DRM_ERROR("eDP aux_bus not found\n");
+ return -ENODEV;
+ }
+
+ /*
+* External bridges are mandatory for eDP interfaces: one has to
+* provide at least an eDP panel (which gets wrapped into panel-bridge).
+*
+* For DisplayPort interfaces external bridges are optional, so
+* silently ignore an error if one is not present (-ENODEV).
+*/
+ rc = dp_parser_find_next_bridge(dp_priv->parser);
+ if (!dp->is_edp && rc == -ENODEV)
+ return 0;
+ else if (rc)
+ goto error;
+
+ dp->next_bridge = dp_priv->parser->next_bridge;
+
+ return 0;
+
+error:
+ if (dp->is_edp) {
+ disable_irq(dp_priv->irq);
+ dp_display_host_phy_exit(dp_priv);
+ dp_display_host_deinit(dp_priv);
+ }
+ return rc;
+}
+
int
[Freedreno] [PATCH v9 0/4] Add support for the eDP panel over aux_bus
This series adds support for generic eDP panel over aux_bus. These changes are dependent on the following patches: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220211224006.1797846-5-dmitry.barysh...@linaro.org/ https://patchwork.kernel.org/project/linux-arm-msm/patch/20220211224006.1797846-6-dmitry.barysh...@linaro.org/ Sankeerth Billakanti (4): drm/msm/dp: Add eDP support via aux_bus drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP drm/msm/dp: wait for hpd high before aux transaction drm/msm/dp: Support the eDP modes given by panel drivers/gpu/drm/msm/dp/dp_aux.c | 21 +++- drivers/gpu/drm/msm/dp/dp_aux.h | 3 +- drivers/gpu/drm/msm/dp/dp_catalog.c | 29 +++--- drivers/gpu/drm/msm/dp/dp_catalog.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 105 +--- drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_drm.c | 21 ++-- drivers/gpu/drm/msm/dp/dp_parser.c | 23 +--- drivers/gpu/drm/msm/dp/dp_parser.h | 13 - 9 files changed, 177 insertions(+), 40 deletions(-) -- 2.7.4
[Freedreno] [PATCH] drm: msm: fix error check return value of irq_of_parse_and_map()
From: Lv Ruyi
The irq_of_parse_and_map() function returns 0 on failure, and does not
return an negative value.
Reported-by: Zeal Robot
Signed-off-by: Lv Ruyi
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 3b92372e7bdf..1fb1ed9e95d9 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -570,7 +570,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
}
irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
- if (irq < 0) {
+ if (!irq) {
ret = irq;
DRM_DEV_ERROR(>dev, "failed to get irq: %d\n", ret);
goto fail;
--
2.25.1
[Freedreno] [PATCH] drm/msm/dp: fix error check return value of irq_of_parse_and_map()
From: Lv Ruyi
The irq_of_parse_and_map() function returns 0 on failure, and does not
return an negative value.
Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Reported-by: Zeal Robot
Signed-off-by: Lv Ruyi
---
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a42732b67349..3926d2ac107d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1239,7 +1239,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
- if (dp->irq < 0) {
+ if (!dp->irq) {
rc = dp->irq;
DRM_ERROR("failed to get irq: %d\n", rc);
return rc;
--
2.25.1
