[PATCH v3 5/8] drm/panel: panel-simple: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-simple.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index a34f4198a534..82041cdb5478 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -411,7 +411,12 @@ static int panel_simple_get_modes(struct drm_panel *panel, /* add hard-coded panel modes */ num += panel_simple_get_non_edid_modes(p, connector); - /* set up connector's "panel orientation" property */ + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, p->orientation); return num; @@ -434,6 +439,14 @@ static int panel_simple_get_timings(struct drm_panel *panel, return p->desc->num_timings; } +static enum drm_panel_orientation panel_simple_get_orientation(struct drm_panel *panel) +{ + struct panel_simple *p = to_panel_simple(panel); + + return p->orientation; +} + + static const struct drm_panel_funcs panel_simple_funcs = { .disable = panel_simple_disable, .unprepare = panel_simple_unprepare, @@ -441,6 +454,7 @@ static const struct drm_panel_funcs panel_simple_funcs = { .enable = panel_simple_enable, .get_modes = panel_simple_get_modes, .get_timings = panel_simple_get_timings, + .get_orientation = panel_simple_get_orientation, }; static struct panel_desc panel_dpi; -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 8/8] drm/mediatek: Config orientation property if panel provides it
Panel orientation property should be set before drm_dev_register(). Mediatek drm driver calls drm_dev_register() in .bind(). However, most panels sets orientation property relatively late, mostly in .get_modes() callback, since this is when they are able to get the connector and binds the orientation property to it, though the value should be known when the panel is probed. Let the drm driver check if the remote end point is a panel and if it contains the orientation property. If it does, set it before drm_dev_register() is called. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: no change. --- drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index bd3f5b485085..86613360d2d9 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -185,6 +185,7 @@ struct mtk_dsi { struct drm_encoder encoder; struct drm_bridge bridge; struct drm_bridge *next_bridge; + struct drm_panel *panel; struct drm_connector *connector; struct phy *phy; @@ -822,6 +823,12 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) ret = PTR_ERR(dsi->connector); goto err_cleanup_encoder; } + + /* Read panel orientation */ + if (dsi->panel) + drm_connector_set_panel_orientation(dsi->connector, + drm_panel_get_orientation(dsi->panel)); + drm_connector_attach_encoder(dsi->connector, >encoder); return 0; @@ -837,6 +844,9 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm = data; struct mtk_dsi *dsi = dev_get_drvdata(dev); + /* Get panel if existed */ + drm_of_find_panel_or_bridge(dev->of_node, 0, 0, >panel, NULL); + ret = mtk_dsi_encoder_init(drm, dsi); if (ret) return ret; -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 7/8] drm/panel: elida-kd35t133: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c b/drivers/gpu/drm/panel/panel-elida-kd35t133.c index 80227617a4d6..fa85a288afdc 100644 --- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c +++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c @@ -217,15 +217,29 @@ static int kd35t133_get_modes(struct drm_panel *panel, connector->display_info.width_mm = mode->width_mm; connector->display_info.height_mm = mode->height_mm; drm_mode_probed_add(connector, mode); + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, ctx->orientation); return 1; } +static enum drm_panel_orientation kd35t133_get_orientation(struct drm_panel *panel) +{ + struct kd35t133 *ctx = panel_to_kd35t133(panel); + + return ctx->orientation; +} + static const struct drm_panel_funcs kd35t133_funcs = { .unprepare = kd35t133_unprepare, .prepare= kd35t133_prepare, .get_modes = kd35t133_get_modes, + .get_orientation = kd35t133_get_orientation, }; static int kd35t133_probe(struct mipi_dsi_device *dsi) -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 6/8] drm/panel: ili9881c: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c index ba30d11547ad..c098a0ed6be7 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c @@ -853,17 +853,31 @@ static int ili9881c_get_modes(struct drm_panel *panel, connector->display_info.width_mm = mode->width_mm; connector->display_info.height_mm = mode->height_mm; + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, ctx->orientation); return 1; } +static enum drm_panel_orientation ili9881c_get_orientation(struct drm_panel *panel) +{ + struct ili9881c *ctx = panel_to_ili9881c(panel); + + return ctx->orientation; +} + static const struct drm_panel_funcs ili9881c_funcs = { .prepare= ili9881c_prepare, .unprepare = ili9881c_unprepare, .enable = ili9881c_enable, .disable= ili9881c_disable, .get_modes = ili9881c_get_modes, + .get_orientation = ili9881c_get_orientation, }; static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi) -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 4/8] drm/panel: lvds: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-lvds.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c index 27a1c9923b09..239527409b00 100644 --- a/drivers/gpu/drm/panel/panel-lvds.c +++ b/drivers/gpu/drm/panel/panel-lvds.c @@ -102,15 +102,29 @@ static int panel_lvds_get_modes(struct drm_panel *panel, connector->display_info.bus_flags = lvds->data_mirror ? DRM_BUS_FLAG_DATA_LSB_TO_MSB : DRM_BUS_FLAG_DATA_MSB_TO_LSB; + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, lvds->orientation); return 1; } +static enum drm_panel_orientation panel_lvds_get_orientation,(struct drm_panel *panel) +{ + struct panel_lvds *lvds = to_panel_lvds(panel); + + return lvds->orientation; +} + static const struct drm_panel_funcs panel_lvds_funcs = { .unprepare = panel_lvds_unprepare, .prepare = panel_lvds_prepare, .get_modes = panel_lvds_get_modes, + .get_orientation = panel_lvds_get_orientation, }; static int panel_lvds_parse_dt(struct panel_lvds *lvds) -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 3/8] drm/panel: panel-edp: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-edp.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 1732b4f56e38..5fa208005395 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -586,7 +586,12 @@ static int panel_edp_get_modes(struct drm_panel *panel, else if (!num) dev_warn(p->base.dev, "No display modes\n"); - /* set up connector's "panel orientation" property */ + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, p->orientation); return num; @@ -609,6 +614,13 @@ static int panel_edp_get_timings(struct drm_panel *panel, return p->desc->num_timings; } +static enum drm_panel_orientation panel_edp_get_orientation(struct drm_panel *panel) +{ + struct panel_edp *p = to_panel_edp(panel); + + return p->orientation; +} + static int detected_panel_show(struct seq_file *s, void *data) { struct drm_panel *panel = s->private; @@ -637,6 +649,7 @@ static const struct drm_panel_funcs panel_edp_funcs = { .prepare = panel_edp_prepare, .enable = panel_edp_enable, .get_modes = panel_edp_get_modes, + .get_orientation = panel_edp_get_orientation, .get_timings = panel_edp_get_timings, .debugfs_init = panel_edp_debugfs_init, }; -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 1/8] drm/panel: Add an API drm_panel_get_orientation() to return panel orientation
Panels usually call drm_connector_set_panel_orientation(), which is later than drm/kms driver calling drm_dev_register(). This leads to a WARN(). The orientation property is known earlier. For example, some panels parse the property through device tree during probe. Add an API to return the property from panel to drm/kms driver, so the drivers are able to call drm_connector_set_panel_orientation() before drm_dev_register(). Suggested-by: Hans de Goede Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: no change --- drivers/gpu/drm/drm_panel.c | 8 include/drm/drm_panel.h | 10 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index f634371c717a..4a512ca80673 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -223,6 +223,14 @@ int drm_panel_get_modes(struct drm_panel *panel, } EXPORT_SYMBOL(drm_panel_get_modes); +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel) +{ + if (panel && panel->funcs && panel->funcs->get_orientation) + return panel->funcs->get_orientation(panel); + + return DRM_MODE_PANEL_ORIENTATION_UNKNOWN; +} +EXPORT_SYMBOL(drm_panel_get_orientation); #ifdef CONFIG_OF /** * of_drm_find_panel - look up a panel using a device tree node diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 1ba2d424a53f..d1bd3be4bbdf 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -133,6 +133,15 @@ struct drm_panel_funcs { * Allows panels to create panels-specific debugfs files. */ void (*debugfs_init)(struct drm_panel *panel, struct dentry *root); + + /** +* @get_orientation: +* +* Return the panel orientation set by device tree or EDID. +* +* This function is optional. +*/ + enum drm_panel_orientation (*get_orientation)(struct drm_panel *panel); }; /** @@ -195,6 +204,7 @@ int drm_panel_enable(struct drm_panel *panel); int drm_panel_disable(struct drm_panel *panel); int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector *connector); +enum drm_panel_orientation drm_panel_get_orientation(struct drm_panel *panel); #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL) struct drm_panel *of_drm_find_panel(const struct device_node *np); -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 2/8] drm/panel: boe-tv101wum-nl6: Implement .get_orientation callback
To return the orientation property to drm/kms driver. Signed-off-by: Hsin-Yi Wang Reviewed-by: Hans de Goede --- v2->v3: add comments for notice. --- drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c index 1be150ac758f..a9cd07234179 100644 --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c @@ -1511,16 +1511,30 @@ static int boe_panel_get_modes(struct drm_panel *panel, connector->display_info.width_mm = boe->desc->size.width_mm; connector->display_info.height_mm = boe->desc->size.height_mm; connector->display_info.bpc = boe->desc->bpc; + /* +* drm drivers are expected to call drm_panel_get_orientation() to get +* panel's orientation then drm_connector_set_panel_orientation() to +* set the property before drm_dev_register(). Otherwise there will be +* a WARN_ON if orientation is set after drm is registered. +*/ drm_connector_set_panel_orientation(connector, boe->orientation); return 1; } +static enum drm_panel_orientation boe_panel_get_orientation(struct drm_panel *panel) +{ + struct boe_panel *boe = to_boe_panel(panel); + + return boe->orientation; +} + static const struct drm_panel_funcs boe_panel_funcs = { .unprepare = boe_panel_unprepare, .prepare = boe_panel_prepare, .enable = boe_panel_enable, .get_modes = boe_panel_get_modes, + .get_orientation = boe_panel_get_orientation, }; static int boe_panel_add(struct boe_panel *boe) -- 2.36.1.255.ge46751e96f-goog
[PATCH v3 0/8] Add a panel API to return panel orientation
Panels usually call drm_connector_set_panel_orientation(), which is later than drm/kms driver calling drm_dev_register(). This leads to a WARN()[1]. The orientation property is known earlier. For example, some panels parse the property through device tree during probe. The series add a panel API drm_panel_get_orientation() for drm/kms drivers. The drivers can use the API to get panel's orientation, so they can call drm_connector_set_panel_orientation() before drm_dev_register(). Panel needs to implement .get_orientation callback to return the property. [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20220530081910.3947168-2-hsi...@chromium.org/ Hsin-Yi Wang (8): drm/panel: Add an API drm_panel_get_orientation() to return panel orientation drm/panel: boe-tv101wum-nl6: Implement .get_orientation callback drm/panel: panel-edp: Implement .get_orientation callback drm/panel: lvds: Implement .get_orientation callback drm/panel: panel-simple: Implement .get_orientation callback drm/panel: ili9881c: Implement .get_orientation callback drm/panel: elida-kd35t133: Implement .get_orientation callback drm/mediatek: Config orientation property if panel provides it drivers/gpu/drm/drm_panel.c| 8 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++ drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 14 ++ drivers/gpu/drm/panel/panel-edp.c | 15 ++- drivers/gpu/drm/panel/panel-elida-kd35t133.c | 14 ++ drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++ drivers/gpu/drm/panel/panel-lvds.c | 14 ++ drivers/gpu/drm/panel/panel-simple.c | 16 +++- include/drm/drm_panel.h| 10 ++ 9 files changed, 113 insertions(+), 2 deletions(-) -- 2.36.1.255.ge46751e96f-goog
Re: [PATCH] dt-bindings: Fix properties without any type
Hello: This patch was applied to chrome-platform/linux.git (for-next) by Rob Herring : On Thu, 19 May 2022 16:14:11 -0500 you wrote: > Now that the schema tools can extract type information for all > properties (in order to decode dtb files), finding properties missing > any type definition is fairly trivial though not yet automated. > > Fix the various property schemas which are missing a type. Most of these > tend to be device specific properties which don't have a vendor prefix. > A vendor prefix is how we normally ensure a type is defined. > > [...] Here is the summary with links: - dt-bindings: Fix properties without any type https://git.kernel.org/chrome-platform/c/4e71ed985389 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH 1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection
Krzysztof Kozlowski 於 2022年6月6日 週一 上午12:11寫道: > > On 02/06/2022 17:31, ChiYuan Huang wrote: > > Krzysztof Kozlowski 於 2022年6月2日 週四 > > 下午9:58寫道: > >> > >> On 02/06/2022 15:56, Rob Herring wrote: > >>> On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: > On 26/05/2022 10:13, ChiYuan Huang wrote: > > Krzysztof Kozlowski 於 2022年5月26日 週四 > > 下午4:06寫道: > >> > >> On 26/05/2022 05:16, cy_huang wrote: > >>> From: ChiYuan Huang > >>> > >>> Add the new property for ocp level selection. > >>> > >>> Signed-off-by: ChiYuan Huang > >>> --- > >>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | > >>> 8 > >>> include/dt-bindings/leds/rt4831-backlight.h | > >>> 5 + > >>> 2 files changed, 13 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> > >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> index e0ac686..c1c59de 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> +++ > >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> @@ -47,6 +47,14 @@ properties: > >>> minimum: 0 > >>> maximum: 3 > >>> > >>> + richtek,bled-ocp-sel: > >> > >> Skip "sel" as it is a shortcut of selection. Name instead: > >> "richtek,backlight-ocp" > >> > > OK, if so, do I need to rename all properties from 'bled' to > > 'backlight' ? > > If only this property is naming as 'backlight'. it may conflict with > > the others like as "richtek,bled-ovp-sel". > > Ah, no, no need. > > >> > >>> +description: | > >>> + Backlight OCP level selection, currently support > >>> 0.9A/1.2A/1.5A/1.8A > >> > >> Could you explain here what is OCP (unfold the acronym)? > > Yes. And the full name is 'over current protection'. > > Thanks and this leads to second thing - you encode register value > instead of logical value. This must be a logical value in mA, so > "richtek,bled-ocp-microamp". > >>> > >>> We already have common properties for setting current of LEDs. We should > >>> use that here I think. > >> > >> It might not be exactly the same. We have "led-max-microamp" which is > >> the maximum allowed current. I guess over-current protection level is > >> slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is > >> something which still can be set and used by system/hardware. OCP should > >> not. > >> > > Yap, you're right. > > So I am right or Rob? > As I know, both are incorrect. > > From the modern backlight IC design, it uses the boost converter > > architecture. > > This OCP level is to limit the inductor current when the internal MOS > > switch turn on. > > Details can refer to the below wiki link > > https://en.wikipedia.org/wiki/Boost_converter > > > > And based on it, OVP is used to limit the inductor output voltage. > > Each channel maximum current is based on the IC affordable limit. > > It is more like as what you said 'led-max-microamp'. > > > > So boost voltage level may depend on the LED VF. > > The different series of LED may cause different boost voltage. > > > > RT4831's OVP/OCP is not just the protection, more like as the limit. > > This suggests Rob is right, so let's use led-max-microamp property? > No, the meaning is different. 'led-max-microamp' always means the channel output current. It already can be adjusted by backlight brightness node. For example low voltage side (3.3~4.4V) to generate the boost voltage to 16~17V, even 20V for BLED Vout. This OCP is to limit the input current of low voltage side. After the explanation, do you still think it's the same thing? > Best regards, > Krzysztof
drm/vc4: module dysfunctional on Raspberry Pi 3B as of 5.18.0
Hi Peter, i didn't subscribe to dri-devel, but i noticed your bug report. Could you please provide more information: Which kernel config do you use (is it a defconfig)? Do you use the mainline device tree blob or the Raspberry Pi DTB? Please provide the version/date of the GPU firmware? Do you have any KMS related setting in the config.txt? Can you provide a full dmesg for the bad case? Best regards
Re: [PATCH v1] dt-bindings: msm: update maintainers list with proper id
On Thu, 02 Jun 2022 16:19:58 +0530, Krishna wrote: > From: Krishna Manikandan > > Use quic id instead of codeaurora id in maintainers list > for display devicetree bindings. > > Signed-off-by: Krishna Manikandan > --- > Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml | 2 +- > Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml | 2 +- > Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml | 2 +- > Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +- > Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml| 2 +- > Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml| 2 +- > Documentation/devicetree/bindings/display/msm/dsi-phy-20nm.yaml| 2 +- > Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml| 2 +- > Documentation/devicetree/bindings/display/msm/dsi-phy-common.yaml | 2 +- > 9 files changed, 9 insertions(+), 9 deletions(-) > Applied, thanks!
Re: [PATCH v2 2/2] drm/meson: encoder_hdmi: Fix refcount leak in meson_encoder_hdmi_init
Hello, thank you for working on this! On Wed, Jun 1, 2022 at 5:40 AM Miaoqian Lin wrote: > > of_graph_get_remote_node() returns remote device nodepointer with > refcount incremented, we should use of_node_put() on it when done. > Add missing of_node_put() to avoid refcount leak. > > Fixes: e67f6037ae1b ("drm/meson: split out encoder from meson_dw_hdmi") > Signed-off-by: Miaoqian Lin Reviewed-by: Martin Blumenstingl Note to self: at first I thought the following code needs to be changed as well: notifier = cec_notifier_conn_register(>dev, NULL, _info); if (!notifier) return -ENOMEM; But a few lines before this we already have: of_node_put(remote); Meaning: this patch is fine as is. Best regards, Martin
Re: [PATCH v2 1/2] drm/meson: encoder_cvbs: Fix refcount leak in meson_encoder_cvbs_init
Hello, thank you for your patch! On Wed, Jun 1, 2022 at 5:39 AM Miaoqian Lin wrote: > > of_graph_get_remote_node() returns remote device nodepointer with > refcount incremented, we should use of_node_put() on it when done. > Add missing of_node_put() to avoid refcount leak. > > Fixes: 318ba02cd8a8 ("drm/meson: encoder_cvbs: switch to bridge with > ATTACH_NO_CONNECTOR") > Signed-off-by: Miaoqian Lin As far as I can tell this patch is identical to the one from v1. Please keep my Reviewed-by from the previous version in case nothing has changed for this specific patch: Reviewed-by: Martin Blumenstingl Best regards, Martin
Re: [PATCH 2/6] dt-bindings: drm/bridge: ti-sn65dsi83: add documentation for reverse lvds lanes
On Mon, May 30, 2022 at 05:05:45PM +0200, Marco Felsch wrote: > The TI converter chip can swap the LVDS data lanes in a pre-defined > manner. This can be useful to improve the layout characteristic. > > Signed-off-by: Marco Felsch > --- > .../bindings/display/bridge/ti,sn65dsi83.yaml | 58 ++- > 1 file changed, 56 insertions(+), 2 deletions(-) Reviewed-by: Rob Herring
Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a): > Allwinner DE2 and DE3 hardware support 3 pixel blend modes: > "None", "Pre-multiplied", "Coverage" > > Add the blend mode property and route it to the appropriate registers. > > Note: > "force_premulti" parameter was added to handle multi-overlay channel > cases in future changes. It must be set to true for cases when more > than 1 overlay layer is used within a channel and at least one of the > overlay layers within a group uses premultiplied blending mode. Please remove this parameter. It's nothing special, so it can be easily added once it's actually needed. For now, it only complicates code. > > Test: > Manually tested all the modes using kmsxx python wrapper with and > without 'force_premulti' flag enabled. > > Signed-off-by: Roman Stratiienko > --- > drivers/gpu/drm/sun4i/sun8i_mixer.h| 2 ++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 - > drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++ > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++ > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++ > 5 files changed, 94 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -65,6 +65,8 @@ > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n) (0xf << ((n) << 2)) > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2) > > +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe) > + > #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED BIT(1) > > #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch) > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 6ccbbca3176d..29c0d9cca19a > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > @@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer > *mixer, int channel, } > > static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int > channel, -int overlay, struct drm_plane *plane) > + int overlay, struct drm_plane *plane, > + unsigned int zpos, bool force_premulti) > { > - u32 mask, val, ch_base; > + u32 mask, val, ch_base, bld_base; > + bool in_premulti, out_premulti; > > + bld_base = sun8i_blender_base(mixer); > ch_base = sun8i_channel_base(mixer, channel); > > + in_premulti = plane->state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI; > + out_premulti = force_premulti || in_premulti; > + > mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK | > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK; > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK | > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK; > > val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8); > > - val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ? > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL : > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED; > + if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER; > + } else { > + val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ? > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL : > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED; > + > + if (in_premulti) > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI; > + } > + > + if (!in_premulti && out_premulti) > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM; > > regmap_update_bits(mixer->engine.regs, > SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay), > mask, val); > + > + regmap_update_bits( > + mixer->engine.regs, SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base), > + SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos), > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0); > } > > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int > channel, @@ -274,7 +296,7 @@ static void > sun8i_ui_layer_atomic_update(struct drm_plane *plane, > sun8i_ui_layer_update_coord(mixer, layer->channel, > layer->overlay, plane, zpos); > sun8i_ui_layer_update_alpha(mixer, layer->channel, > - layer->overlay, plane); > + layer->overlay, plane, zpos, false); > sun8i_ui_layer_update_formats(mixer, layer->channel, > layer->overlay, plane); >
Re: [PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3
Dne nedelja, 05. junij 2022 ob 11:40:18 CEST je Roman Stratiienko napisal(a): > Otherwise alpha value is discarded, resulting incorrect pixel > apperance on the display. > > This also fixes missing transparency for the most bottom layer. Can you explain that a bit more? Also, BSP driver never enables this bit. What are we doing differently? > > Test applications and videos w/ w/o this patch are available at [1]. > > [1]: https://github.com/GloDroid/glodroid_tests/issues/1 As stated in other emails, commit messages should not contain external links (per patch rules). Best regards, Jernej > Signed-off-by: Roman Stratiienko > > --- > Changelog: > > V2: Added code hunk missing in v1 > --- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 5 +++-- > drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 6b1711a9a71f..ba2932aaed08 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -320,8 +320,9 @@ static void sun8i_mixer_mode_set(struct sunxi_engine > *engine, else > val = 0; > > - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base), > -SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val); > + val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY; > + > + regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base), val); > > DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n", >interlaced ? "on" : "off"); > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..bc12c95af6f3 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -65,6 +65,7 @@ > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n) (0xf << ((n) << 2)) > #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2) > > +#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY BIT(0) > #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED BIT(1) > > #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
Re: [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Sun, 5 Jun 2022 at 20:32, Rob Clark wrote: > > On Sun, Jun 5, 2022 at 9:47 AM Daniel Vetter wrote: > > > > On Fri, 27 May 2022 at 01:55, Dmitry Osipenko > > wrote: > > > > > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > > > code duplication among DRM drivers by replacing theirs custom shrinker > > > implementations with the generic shrinker. > > > > > > In order to start using DRM SHMEM shrinker drivers should: > > > > > > 1. Implement new evict() shmem object callback. > > > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > > > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to > > >activate shrinking of shmem GEMs. > > > > > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > > > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > > > > > Signed-off-by: Daniel Almeida > > > Signed-off-by: Dmitry Osipenko > > > > So I guess I get a price for being blind since forever, because this > > thing existed since at least 2013. I just stumbled over > > llist_lru.[hc], a purpose built list helper for shrinkers. I think we > > should try to adopt that so that our gpu shrinkers look more like > > shrinkers for everything else. > > followup from a bit of irc discussion w/ danvet about list_lru: > > * It seems to be missing a way to bail out of iteration before > nr_to_scan is hit.. which is going to be inconvenient if you > want to allow active bos on the LRU but bail scanning once > you encounter the first one. > > * Not sure if the numa node awareness is super useful for GEM > bos > > First issue is perhaps not too hard to fix. But maybe a better > idea is a drm_gem_lru helper type thing which is more tailored > to GEM buffers? Yeah I guess reusing list_lru isn't that good idea. So just open-coding it for now, and then drm_gem_bo_lru or so if we need to share it separately from shmem helpers with other drivers. Maybe will be needed for ttm or so. -Daniel > > BR, > -R > > > Apologies for this, since I fear this might cause a bit of churn. > > Hopefully it's all contained to the list manipulation code in shmem > > helpers, I don't think this should leak any further. > > -Daniel > > > > > --- > > > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > > > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > > > include/drm/drm_device.h | 4 + > > > include/drm/drm_gem_shmem_helper.h| 87 ++- > > > 5 files changed, 594 insertions(+), 49 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > index 555fe212bd98..4cd0b5913492 100644 > > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object > > > *drm_gem_shmem_create(struct drm_device *dev, size_t > > > } > > > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > > > > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object > > > *shmem) > > > +{ > > > + return (shmem->madv >= 0) && shmem->evict && > > > + shmem->eviction_enabled && shmem->pages_use_count && > > > + !shmem->pages_pin_count && !shmem->base.dma_buf && > > > + !shmem->base.import_attach && shmem->sgt && > > > !shmem->evicted; > > > +} > > > + > > > +static void > > > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > > > +{ > > > + struct drm_gem_object *obj = >base; > > > + struct drm_gem_shmem_shrinker *gem_shrinker = > > > obj->dev->shmem_shrinker; > > > + > > > + dma_resv_assert_held(shmem->base.resv); > > > + > > > + if (!gem_shrinker || obj->import_attach) > > > + return; > > > + > > > + mutex_lock(_shrinker->lock); > > > + > > > + if (drm_gem_shmem_is_evictable(shmem) || > > > + drm_gem_shmem_is_purgeable(shmem)) > > > + list_move_tail(>madv_list, > > > _shrinker->lru_evictable); > > > + else if (shmem->madv < 0) > > > + list_del_init(>madv_list); > > > + else if (shmem->evicted) > > > + list_move_tail(>madv_list, > > > _shrinker->lru_evicted); > > > + else if (!shmem->pages) > > > + list_del_init(>madv_list); > > > + else > > > + list_move_tail(>madv_list, > > > _shrinker->lru_pinned); > > > + > > > + mutex_unlock(_shrinker->lock); > > > +} > > > + > > > /** > > > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > > > * @shmem: shmem GEM object to free > > > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > > *shmem) > > > } else { > > > dma_resv_lock(shmem->base.resv, NULL); > > > > > > + /* take out shmem GEM object from the memory
Re: [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Sun, Jun 5, 2022 at 9:47 AM Daniel Vetter wrote: > > On Fri, 27 May 2022 at 01:55, Dmitry Osipenko > wrote: > > > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > > code duplication among DRM drivers by replacing theirs custom shrinker > > implementations with the generic shrinker. > > > > In order to start using DRM SHMEM shrinker drivers should: > > > > 1. Implement new evict() shmem object callback. > > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to > >activate shrinking of shmem GEMs. > > > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > > > Signed-off-by: Daniel Almeida > > Signed-off-by: Dmitry Osipenko > > So I guess I get a price for being blind since forever, because this > thing existed since at least 2013. I just stumbled over > llist_lru.[hc], a purpose built list helper for shrinkers. I think we > should try to adopt that so that our gpu shrinkers look more like > shrinkers for everything else. followup from a bit of irc discussion w/ danvet about list_lru: * It seems to be missing a way to bail out of iteration before nr_to_scan is hit.. which is going to be inconvenient if you want to allow active bos on the LRU but bail scanning once you encounter the first one. * Not sure if the numa node awareness is super useful for GEM bos First issue is perhaps not too hard to fix. But maybe a better idea is a drm_gem_lru helper type thing which is more tailored to GEM buffers? BR, -R > Apologies for this, since I fear this might cause a bit of churn. > Hopefully it's all contained to the list manipulation code in shmem > helpers, I don't think this should leak any further. > -Daniel > > > --- > > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > > include/drm/drm_device.h | 4 + > > include/drm/drm_gem_shmem_helper.h| 87 ++- > > 5 files changed, 594 insertions(+), 49 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index 555fe212bd98..4cd0b5913492 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object > > *drm_gem_shmem_create(struct drm_device *dev, size_t > > } > > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem) > > +{ > > + return (shmem->madv >= 0) && shmem->evict && > > + shmem->eviction_enabled && shmem->pages_use_count && > > + !shmem->pages_pin_count && !shmem->base.dma_buf && > > + !shmem->base.import_attach && shmem->sgt && !shmem->evicted; > > +} > > + > > +static void > > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > > +{ > > + struct drm_gem_object *obj = >base; > > + struct drm_gem_shmem_shrinker *gem_shrinker = > > obj->dev->shmem_shrinker; > > + > > + dma_resv_assert_held(shmem->base.resv); > > + > > + if (!gem_shrinker || obj->import_attach) > > + return; > > + > > + mutex_lock(_shrinker->lock); > > + > > + if (drm_gem_shmem_is_evictable(shmem) || > > + drm_gem_shmem_is_purgeable(shmem)) > > + list_move_tail(>madv_list, > > _shrinker->lru_evictable); > > + else if (shmem->madv < 0) > > + list_del_init(>madv_list); > > + else if (shmem->evicted) > > + list_move_tail(>madv_list, > > _shrinker->lru_evicted); > > + else if (!shmem->pages) > > + list_del_init(>madv_list); > > + else > > + list_move_tail(>madv_list, > > _shrinker->lru_pinned); > > + > > + mutex_unlock(_shrinker->lock); > > +} > > + > > /** > > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > > * @shmem: shmem GEM object to free > > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > *shmem) > > } else { > > dma_resv_lock(shmem->base.resv, NULL); > > > > + /* take out shmem GEM object from the memory shrinker */ > > + drm_gem_shmem_madvise(shmem, -1); > > + > > WARN_ON(shmem->vmap_use_count); > > > > if (shmem->sgt) { > > @@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > *shmem) > > sg_free_table(shmem->sgt); > > kfree(shmem->sgt); > > } > > - if (shmem->pages) > > + if (shmem->pages_use_count) > > drm_gem_shmem_put_pages(shmem); > >
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
> On Jun 5, 2022, at 12:26 PM, Simon Ser wrote: > > --- Original Message --- > On Sunday, June 5th, 2022 at 17:47, Zack Rusin wrote: > Also, let me point this out because it also seems to be a fundamental misunderstanding, user space implementing software cursor doesn’t fix anything. Just leaves everything broken in different ways. The reason virtualized drivers went away from software cursors is because it makes it impossible to make it work with a bunch of interesting and desirable scenarios, e.g. unity mode where the guest might have a terminal and browser open and then the virtual machine software creates windows out of those, so you don’t have the entire desktop of the guest but instead native looking windows with the apps. In that case guest has no way of knowing when the cursor leaves the window, so to make seemless cursors work you’d need to implement irc or backdoors that send that information from the host to the guest, then have virtualized drivers create some sort of uevent, to send to the userspace to let it know to hide the cursor because it actually left the window. That’s a trivial scenario and there’s a lot more with mice that are remoted themselves, guests with singular or maybe many apps exported, possibly overlapping on the host but all within a desktop in the guest, etc. >>> >>> Are you saying that the current broken behavior is better than software >>> cursors? >> >> They’re broken in very different ways. You’re asking me which bugs do >> I prefer. Ultimately the current lack of hotspots leaves mouse unusable >> but I don’t see any reason to trade that for another set of bugs instead >> of just fixing it (either via fallback to legacy or using the new hotspot >> api). > > Software cursors aren't a bug. They're a fallback. They work well, or usually well enough, on native but not with virtual machines. They break a lot of features. The nature of virtualization is that the guest doesn’t explicitly get access to a lot of host side info. If you move to software cursor you automatically lose all that info (unless, as I mentioned before, you rewrite hypervisors to push all that info to guests again, either via backdoors, irq or register accesses). Software cursor just doesn’t work with modern hypervisors well enough to be a reasonable replacement. >>> New user-space supports setting the hotspot prop, and is aware that it >>> can't >>> set the cursor plane position, so the cursor plane can be exposed again >>> when >>> the cap is enabled. >> >> But we still use cursor plane position. Hotspots are offsets from >> cursor plane positions. Both are required. > > No, VM drivers don't need and disregard the cursor position AFAIK. They > replace it with the host cursor's position. Iirc they all use it. >>> >>> What do they use it for? >>> >>> The whole point of this discussion is to be able to display the guest's >>> cursor >>> image on the host's cursor, so that latency is reduced? >> >> >> Ah, I think I see now where the confusion is coming from. No, it’s >> definitely not. This has nothing to do with latency. By default >> position of a mouse cursor doesn’t tell us where the point that is >> actually used as an activation of a click is, e.g. a mouse cursor image >> with an arrow pointing to the top-left and a mouse cursor image pointing >> to the bottom right - both at the same position, as you can imagine it will >> become impossible to use the desktop if the click defaults to the top left, >> especially as the number of cursor images increases, you need information >> about which point within the cursor image activates the click, that’s the >> hotspot. You need to know the position of the image and you need to know >> which point within that image is responsible for actually activating the >> events. > > Yeah, that's what a hotspot is. > > By "the whole point of this discussion", I meant that the whole point > for VM drivers to expose a hardware cursor is to improve latency (and > provide other related features). > > At any rate, I consider broken any driver which exposes a cursor plane, > then doesn't display it exactly at the CRTC_X/CRTC_Y But we do… The cursor is at crtc_x, crtc_y. > or misbehaves if it's missing hotspot info. That doesn’t follow at all. How can they not behave incorrectly if the information is missing? Maybe we can refocus the discussion because it seems like we’re moving in circles. Your proposal for a feature cap and removal of the cursor plane doesn’t fix anything: - current user space which doesn’t fallback to legacy kms with virtualized driver is completely broken and obviously will always remain broken on all already released kernels - making the same software fallback to software cursor breaks a massive amount of interactions in VMs so what are you proposing? z
drm/vc4: module dysfunctional on Raspberry Pi 3B as of 5.18.0
Hello. As of Linux 5.18.0, module vc4 apparently isn't working on Raspberry Pi 3B any more. If a monitor is attached to the device, the boot messages show up as usual, but right when KMS starts, the screen turns black. Similarly, the screen also turns black when the module is blacklisted at boot time and loaded from the running system. The problem looks quite similar to the one posted some months ago in [1]. Unfortunately, looking through systemd's journal didn't seem to yield any real hint. Nevertheless, the results from grepping vc4 are → 5.17.1 > kernel: vc4-drm soc:gpu: bound 3f40.hvs (ops vc4_hvs_ops [vc4]) > kernel: rc rc0: vc4 as /devices/platform/soc/3f902000.hdmi/rc/rc0 > kernel: input: vc4 as /devices/platform/soc/3f902000.hdmi/rc/rc0/input0 > kernel: vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f004000.txp (ops vc4_txp_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3fc0.v3d (ops vc4_v3d_ops [vc4]) > kernel: fb0: switching to vc4 from simple > kernel: [drm] Initialized vc4 0.0.0 20140616 for soc:gpu on minor 0 > kernel: vc4-drm soc:gpu: [drm] fb0: vc4drmfb frame buffer device > systemd-logind[338]: Watching system buttons on /dev/input/event0 (vc4) → 5.18.0 > kernel: fb0: switching to vc4 from simple > kernel: vc4-drm soc:gpu: bound 3f40.hvs (ops vc4_hvs_ops [vc4]) > kernel: rc rc0: vc4 as /devices/platform/soc/3f902000.hdmi/rc/rc0 > kernel: input: vc4 as /devices/platform/soc/3f902000.hdmi/rc/rc0/input0 > kernel: vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f004000.txp (ops vc4_txp_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops vc4_crtc_ops [vc4]) > kernel: vc4-drm soc:gpu: bound 3fc0.v3d (ops vc4_v3d_ops [vc4]) > kernel: [drm] Initialized vc4 0.0.0 20140616 for soc:gpu on minor 0 > kernel: vc4-drm soc:gpu: [drm] fb0: vc4drmfb frame buffer device > systemd-logind[337]: Watching system buttons on /dev/input/event0 (vc4) Regards, Peter Mattern [1] https://lists.freedesktop.org/archives/dri-devel/2022-January/339458.html
Re: [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Fri, 27 May 2022 at 01:55, Dmitry Osipenko wrote: > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > code duplication among DRM drivers by replacing theirs custom shrinker > implementations with the generic shrinker. > > In order to start using DRM SHMEM shrinker drivers should: > > 1. Implement new evict() shmem object callback. > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to >activate shrinking of shmem GEMs. > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > Signed-off-by: Daniel Almeida > Signed-off-by: Dmitry Osipenko So I guess I get a price for being blind since forever, because this thing existed since at least 2013. I just stumbled over llist_lru.[hc], a purpose built list helper for shrinkers. I think we should try to adopt that so that our gpu shrinkers look more like shrinkers for everything else. Apologies for this, since I fear this might cause a bit of churn. Hopefully it's all contained to the list manipulation code in shmem helpers, I don't think this should leak any further. -Daniel > --- > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > include/drm/drm_device.h | 4 + > include/drm/drm_gem_shmem_helper.h| 87 ++- > 5 files changed, 594 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 555fe212bd98..4cd0b5913492 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct > drm_device *dev, size_t > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem) > +{ > + return (shmem->madv >= 0) && shmem->evict && > + shmem->eviction_enabled && shmem->pages_use_count && > + !shmem->pages_pin_count && !shmem->base.dma_buf && > + !shmem->base.import_attach && shmem->sgt && !shmem->evicted; > +} > + > +static void > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > +{ > + struct drm_gem_object *obj = >base; > + struct drm_gem_shmem_shrinker *gem_shrinker = > obj->dev->shmem_shrinker; > + > + dma_resv_assert_held(shmem->base.resv); > + > + if (!gem_shrinker || obj->import_attach) > + return; > + > + mutex_lock(_shrinker->lock); > + > + if (drm_gem_shmem_is_evictable(shmem) || > + drm_gem_shmem_is_purgeable(shmem)) > + list_move_tail(>madv_list, > _shrinker->lru_evictable); > + else if (shmem->madv < 0) > + list_del_init(>madv_list); > + else if (shmem->evicted) > + list_move_tail(>madv_list, _shrinker->lru_evicted); > + else if (!shmem->pages) > + list_del_init(>madv_list); > + else > + list_move_tail(>madv_list, _shrinker->lru_pinned); > + > + mutex_unlock(_shrinker->lock); > +} > + > /** > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > * @shmem: shmem GEM object to free > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > } else { > dma_resv_lock(shmem->base.resv, NULL); > > + /* take out shmem GEM object from the memory shrinker */ > + drm_gem_shmem_madvise(shmem, -1); > + > WARN_ON(shmem->vmap_use_count); > > if (shmem->sgt) { > @@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > sg_free_table(shmem->sgt); > kfree(shmem->sgt); > } > - if (shmem->pages) > + if (shmem->pages_use_count) > drm_gem_shmem_put_pages(shmem); > > WARN_ON(shmem->pages_use_count); > @@ -163,18 +202,82 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_free); > > -static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > +/** > + * drm_gem_shmem_set_evictable() - Make GEM evictable by memory shrinker > + * @shmem: shmem GEM object > + * > + * Tell memory shrinker that this GEM can be evicted. Initially eviction is > + * disabled for all GEMs. If GEM was purged, then -ENOMEM is returned. > + * > + * Returns: > + * 0 on success or a negative error code on failure. > + */ > +int drm_gem_shmem_set_evictable(struct drm_gem_shmem_object *shmem) > +{ > + dma_resv_lock(shmem->base.resv, NULL); > + > + if (shmem->madv < 0) > +
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
--- Original Message --- On Sunday, June 5th, 2022 at 17:47, Zack Rusin wrote: > > > Also, let me point this out because it also seems to be a fundamental > > > misunderstanding, user space implementing software cursor doesn’t fix > > > anything. Just leaves everything broken in different ways. The reason > > > virtualized drivers went away from software cursors is because it makes it > > > impossible to make it work with a bunch of interesting and desirable > > > scenarios, e.g. unity mode where the guest might have a terminal and > > > browser > > > open and then the virtual machine software creates windows out of those, > > > so > > > you don’t have the entire desktop of the guest but instead native looking > > > windows with the apps. In that case guest has no way of knowing when the > > > cursor leaves the window, so to make seemless cursors work you’d need to > > > implement irc or backdoors that send that information from the host to the > > > guest, then have virtualized drivers create some sort of uevent, to send > > > to > > > the userspace to let it know to hide the cursor because it actually left > > > the > > > window. That’s a trivial scenario and there’s a lot more with mice that > > > are > > > remoted themselves, guests with singular or maybe many apps exported, > > > possibly overlapping on the host but all within a desktop in the guest, > > > etc. > > > > Are you saying that the current broken behavior is better than software > > cursors? > > They’re broken in very different ways. You’re asking me which bugs do > I prefer. Ultimately the current lack of hotspots leaves mouse unusable > but I don’t see any reason to trade that for another set of bugs instead > of just fixing it (either via fallback to legacy or using the new hotspot > api). Software cursors aren't a bug. They're a fallback. > > > > > > New user-space supports setting the hotspot prop, and is aware that > > > > > > it can't > > > > > > set the cursor plane position, so the cursor plane can be exposed > > > > > > again when > > > > > > the cap is enabled. > > > > > > > > > > But we still use cursor plane position. Hotspots are offsets from > > > > > cursor plane positions. Both are required. > > > > > > > > No, VM drivers don't need and disregard the cursor position AFAIK. They > > > > replace it with the host cursor's position. > > > > > > Iirc they all use it. > > > > What do they use it for? > > > > The whole point of this discussion is to be able to display the guest's > > cursor > > image on the host's cursor, so that latency is reduced? > > > Ah, I think I see now where the confusion is coming from. No, it’s > definitely not. This has nothing to do with latency. By default > position of a mouse cursor doesn’t tell us where the point that is > actually used as an activation of a click is, e.g. a mouse cursor image > with an arrow pointing to the top-left and a mouse cursor image pointing > to the bottom right - both at the same position, as you can imagine it will > become impossible to use the desktop if the click defaults to the top left, > especially as the number of cursor images increases, you need information > about which point within the cursor image activates the click, that’s the > hotspot. You need to know the position of the image and you need to know > which point within that image is responsible for actually activating the > events. Yeah, that's what a hotspot is. By "the whole point of this discussion", I meant that the whole point for VM drivers to expose a hardware cursor is to improve latency (and provide other related features). At any rate, I consider broken any driver which exposes a cursor plane, then doesn't display it exactly at the CRTC_X/CRTC_Y or misbehaves if it's missing hotspot info.
Re: [PATCH 1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection
On 02/06/2022 17:31, ChiYuan Huang wrote: > Krzysztof Kozlowski 於 2022年6月2日 週四 下午9:58寫道: >> >> On 02/06/2022 15:56, Rob Herring wrote: >>> On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: On 26/05/2022 10:13, ChiYuan Huang wrote: > Krzysztof Kozlowski 於 2022年5月26日 週四 > 下午4:06寫道: >> >> On 26/05/2022 05:16, cy_huang wrote: >>> From: ChiYuan Huang >>> >>> Add the new property for ocp level selection. >>> >>> Signed-off-by: ChiYuan Huang >>> --- >>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 >>> >>> include/dt-bindings/leds/rt4831-backlight.h | 5 >>> + >>> 2 files changed, 13 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> index e0ac686..c1c59de 100644 >>> --- >>> a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> +++ >>> b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> @@ -47,6 +47,14 @@ properties: >>> minimum: 0 >>> maximum: 3 >>> >>> + richtek,bled-ocp-sel: >> >> Skip "sel" as it is a shortcut of selection. Name instead: >> "richtek,backlight-ocp" >> > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? > If only this property is naming as 'backlight'. it may conflict with > the others like as "richtek,bled-ovp-sel". Ah, no, no need. >> >>> +description: | >>> + Backlight OCP level selection, currently support >>> 0.9A/1.2A/1.5A/1.8A >> >> Could you explain here what is OCP (unfold the acronym)? > Yes. And the full name is 'over current protection'. Thanks and this leads to second thing - you encode register value instead of logical value. This must be a logical value in mA, so "richtek,bled-ocp-microamp". >>> >>> We already have common properties for setting current of LEDs. We should >>> use that here I think. >> >> It might not be exactly the same. We have "led-max-microamp" which is >> the maximum allowed current. I guess over-current protection level is >> slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is >> something which still can be set and used by system/hardware. OCP should >> not. >> > Yap, you're right. So I am right or Rob? > From the modern backlight IC design, it uses the boost converter architecture. > This OCP level is to limit the inductor current when the internal MOS > switch turn on. > Details can refer to the below wiki link > https://en.wikipedia.org/wiki/Boost_converter > > And based on it, OVP is used to limit the inductor output voltage. > Each channel maximum current is based on the IC affordable limit. > It is more like as what you said 'led-max-microamp'. > > So boost voltage level may depend on the LED VF. > The different series of LED may cause different boost voltage. > > RT4831's OVP/OCP is not just the protection, more like as the limit. This suggests Rob is right, so let's use led-max-microamp property? Best regards, Krzysztof
[PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property
Allwinner DE2 and DE3 hardware support 3 pixel blend modes: "None", "Pre-multiplied", "Coverage" Add the blend mode property and route it to the appropriate registers. Note: "force_premulti" parameter was added to handle multi-overlay channel cases in future changes. It must be set to true for cases when more than 1 overlay layer is used within a channel and at least one of the overlay layers within a group uses premultiplied blending mode. Test: Manually tested all the modes using kmsxx python wrapper with and without 'force_premulti' flag enabled. Signed-off-by: Roman Stratiienko --- drivers/gpu/drm/sun4i/sun8i_mixer.h| 2 ++ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 - drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++ drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++ drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++ 5 files changed, 94 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -65,6 +65,8 @@ #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2)) #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n) ((n) << 2) +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe) + #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1) #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch) BIT(ch) diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 6ccbbca3176d..29c0d9cca19a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, } static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel, - int overlay, struct drm_plane *plane) + int overlay, struct drm_plane *plane, + unsigned int zpos, bool force_premulti) { - u32 mask, val, ch_base; + u32 mask, val, ch_base, bld_base; + bool in_premulti, out_premulti; + bld_base = sun8i_blender_base(mixer); ch_base = sun8i_channel_base(mixer, channel); + in_premulti = plane->state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI; + out_premulti = force_premulti || in_premulti; + mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK | - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK; + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK | + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK; val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8); - val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ? - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL : - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED; + if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER; + } else { + val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ? + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL : + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED; + + if (in_premulti) + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI; + } + + if (!in_premulti && out_premulti) + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM; regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay), mask, val); + + regmap_update_bits( + mixer->engine.regs, SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base), + SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos), + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0); } static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, @@ -274,7 +296,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, sun8i_ui_layer_update_coord(mixer, layer->channel, layer->overlay, plane, zpos); sun8i_ui_layer_update_alpha(mixer, layer->channel, - layer->overlay, plane); + layer->overlay, plane, zpos, false); sun8i_ui_layer_update_formats(mixer, layer->channel, layer->overlay, plane); sun8i_ui_layer_update_buffer(mixer, layer->channel, @@ -332,8 +354,8 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct drm_device *drm, { enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY; int channel = mixer->cfg->vi_num + index; + unsigned int plane_cnt, blend_modes; struct sun8i_ui_layer *layer; -
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
> On Jun 5, 2022, at 3:30 AM, Simon Ser wrote: > > ⚠ External Email > > On Friday, June 3rd, 2022 at 20:31, Zack Rusin wrote: > >>> My proposal was: >>> >>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). >>> Only >>> user-space which supports the hotspot props will enable it. >>> - By default, don't expose a cursor plane, because current user-space >>> doesn't >>> support it (Weston might put a window in the cursor plane, and nobody >>> can >>> report hotspot). >>> - If the KMS client enables the cap, advertise the cursor >>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y >>> properties >>> since the driver will pick the position. >>> >>> That way both old and new user-space are fixed. >> >> I don’t think I see how that fixes anything. In particular I don’t see a >> way >> of fixing the old user space at all. We require hotspot info, old user >> space >> doesn’t set it because there’s no way of setting it on atomic kms. > > Old atomic user-space is fixed by removing the cursor plane. Then old > atomic user-space will fallback to drawing the cursor itself, e.g. via > OpenGL. But it’s not fixed, because the driver is still on a deny-list and nothing implements this. You’re saying you could potentially “fix” by implementing client side cursor handling in all kms clients? That’s a hard sell if the user space can just put the virtualized driver on deny-lists and fallback to use old kms which supports hotspots. >>> >>> What deny-list are you referring to? >>> >>> All compositors I know of implement a fallback when no cursor plane is >>> usable. >> >> I put the links in the first email in the >> series: >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.gnome.org%2FGNOME%2Fmutter%2F-%2Fblob%2Fmain%2Fsrc%2Fbackends%2Fnative%2Fmeta-kms-impl-device-atomic.c%23L1188data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=wggJaNScF0ziIG%2BfSdSUKBVZGoNjtm4Q8amS7SbJa%2FY%3Dreserved=0 >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Finvent.kde.org%2Fplasma%2Fkwin%2F-%2Fblob%2Fmaster%2Fsrc%2Fbackends%2Fdrm%2Fdrm_gpu.cpp%23L156data=05%7C01%7Czackr%40vmware.com%7C2b0ab6c67e7d4bfb618a08da46c5394f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637900110157712044%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=%2BCTEJ9XtlI9kuKJJZVMNodtkZSkRIv8RN9jSRAM1pqM%3Dreserved=0 > > I was not aware of these lists. Having to work-around drivers in user-space is > really not great. > > But regardless, that doesn't really change anything. With my proposal: > > - Old user-space with hacky deny-lists (Mutter, KWin) still goes through the > legacy uAPI. > - Old user-space without the hacky deny-lists (Weston, wlroots) uses software > cursors and is not broken anymore. > - New user-space can report hotspot and is not broken. Yea, that doesn’t work, but I think below I stopped where the issue is. >> Also, let me point this out because it also seems to be a fundamental >> misunderstanding, user space implementing software cursor doesn’t fix >> anything. Just leaves everything broken in different ways. The reason >> virtualized drivers went away from software cursors is because it makes it >> impossible to make it work with a bunch of interesting and desirable >> scenarios, e.g. unity mode where the guest might have a terminal and browser >> open and then the virtual machine software creates windows out of those, so >> you don’t have the entire desktop of the guest but instead native looking >> windows with the apps. In that case guest has no way of knowing when the >> cursor leaves the window, so to make seemless cursors work you’d need to >> implement irc or backdoors that send that information from the host to the >> guest, then have virtualized drivers create some sort of uevent, to send to >> the userspace to let it know to hide the cursor because it actually left the >> window. That’s a trivial scenario and there’s a lot more with mice that are >> remoted themselves, guests with singular or maybe many apps exported, >> possibly overlapping on the host but all within a desktop in the guest, etc. > > Are you saying that the current broken behavior is better than software > cursors? They’re broken in very different ways. You’re asking me which bugs do I prefer. Ultimately the current lack of hotspots leaves mouse unusable but I don’t see any reason to trade that for another set of bugs instead of just fixing it (either via fallback to legacy or using the new hotspot api). > New user-space supports setting the hotspot prop, and is aware that it
[PATCH 3/3] mm/mempool: use might_alloc()
mempool are generally used for GFP_NOIO, so this wont benefit all that much because might_alloc currently only checks GFP_NOFS. But it does validate against mmu notifier pte zapping, some might catch some drivers doing really silly things, plus it's a bit more meaningful in what we're checking for here. Signed-off-by: Daniel Vetter Cc: Andrew Morton Cc: linux...@kvack.org --- mm/mempool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/mempool.c b/mm/mempool.c index b933d0fc21b8..96488b13a1ef 100644 --- a/mm/mempool.c +++ b/mm/mempool.c @@ -379,7 +379,7 @@ void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask) gfp_t gfp_temp; VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO); - might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM); + might_alloc(gfp_mask); gfp_mask |= __GFP_NOMEMALLOC; /* don't allocate emergency reserves */ gfp_mask |= __GFP_NORETRY; /* don't loop in __alloc_pages */ -- 2.36.0
[PATCH 2/3] mm/slab: delete cache_alloc_debugcheck_before()
It only does a might_sleep_if(GFP_RECLAIM) check, which is already covered by the might_alloc() in slab_pre_alloc_hook(). And all callers of cache_alloc_debugcheck_before() call that beforehand already. Signed-off-by: Daniel Vetter Cc: Christoph Lameter Cc: Pekka Enberg Cc: David Rientjes Cc: Joonsoo Kim Cc: Andrew Morton Cc: Vlastimil Babka Cc: Roman Gushchin Cc: linux...@kvack.org --- mm/slab.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/mm/slab.c b/mm/slab.c index b04e40078bdf..75779ac5f5ba 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -2973,12 +2973,6 @@ static void *cache_alloc_refill(struct kmem_cache *cachep, gfp_t flags) return ac->entry[--ac->avail]; } -static inline void cache_alloc_debugcheck_before(struct kmem_cache *cachep, - gfp_t flags) -{ - might_sleep_if(gfpflags_allow_blocking(flags)); -} - #if DEBUG static void *cache_alloc_debugcheck_after(struct kmem_cache *cachep, gfp_t flags, void *objp, unsigned long caller) @@ -3219,7 +3213,6 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid, size_t orig_ if (unlikely(ptr)) goto out_hooks; - cache_alloc_debugcheck_before(cachep, flags); local_irq_save(save_flags); if (nodeid == NUMA_NO_NODE) @@ -3304,7 +3297,6 @@ slab_alloc(struct kmem_cache *cachep, struct list_lru *lru, gfp_t flags, if (unlikely(objp)) goto out; - cache_alloc_debugcheck_before(cachep, flags); local_irq_save(save_flags); objp = __do_cache_alloc(cachep, flags); local_irq_restore(save_flags); @@ -3541,8 +3533,6 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, if (!s) return 0; - cache_alloc_debugcheck_before(s, flags); - local_irq_disable(); for (i = 0; i < size; i++) { void *objp = kfence_alloc(s, s->object_size, flags) ?: __do_cache_alloc(s, flags); -- 2.36.0
[PATCH 1/3] mm/page_alloc: use might_alloc()
... instead of open codding it. Completely equivalent code, just a notch more meaningful when reading. Signed-off-by: Daniel Vetter Cc: Andrew Morton Cc: linux...@kvack.org --- mm/page_alloc.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2db95780e003..24d170cb 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5177,10 +5177,7 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order, *alloc_flags |= ALLOC_CPUSET; } - fs_reclaim_acquire(gfp_mask); - fs_reclaim_release(gfp_mask); - - might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM); + might_alloc(gfp_mask); if (should_fail_alloc_page(gfp_mask, order)) return false; -- 2.36.0
Re: [PATCH 1/2] dt-bindings: display: simple: add EDT ETML0700Y5DHA panel
On Mon, 30 May 2022 14:24:06 +0200, Marco Felsch wrote: > Add binding for the Emerging Display Technology ETML0700Y5DHA panel. > It is a 7" WSVGA (1024x600) TFT LCD panel with: > - LVDS data interface, > - backlight and > - capacitive touch. > > Signed-off-by: Marco Felsch > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
Re: [PATCH AUTOSEL 4.19 16/38] drm/sun4i: Add support for D1 TCONs
On Mon, May 30, 2022 at 09:41:59AM -0500, Samuel Holland wrote: Hi Sasha, On 5/30/22 8:49 AM, Sasha Levin wrote: From: Samuel Holland [ Upstream commit b9b52d2f4aafa2bd637ace0f24615bdad8e49f01 ] D1 has a TCON TOP, so its quirks are similar to those for the R40 TCONs. While there are some register changes, the part of the TCON TV supported by the driver matches the R40 quirks, so that quirks structure can be reused. D1 has the first supported TCON LCD with a TCON TOP, so the TCON LCD needs a new quirks structure. D1's TCON LCD hardware supports LVDS; in fact it provides dual-link LVDS from a single TCON. However, it comes with a brand new LVDS PHY. Since this PHY has not been tested, leave out LVDS driver support for now. Signed-off-by: Samuel Holland Reviewed-by: Jernej Skrabec Signed-off-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20220424162633.12369-14-sam...@sholland.org Signed-off-by: Sasha Levin This patch adds support for hardware in a SoC that will not boot on earlier kernel releases, so there is no benefit to backporting the patch (to any previous release). Dropped, thanks! -- Thanks, Sasha
[PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3
Otherwise alpha value is discarded, resulting incorrect pixel apperance on the display. This also fixes missing transparency for the most bottom layer. Test applications and videos w/ w/o this patch are available at [1]. [1]: https://github.com/GloDroid/glodroid_tests/issues/1 Signed-off-by: Roman Stratiienko --- Changelog: V2: Added code hunk missing in v1 --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 5 +++-- drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 6b1711a9a71f..ba2932aaed08 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -320,8 +320,9 @@ static void sun8i_mixer_mode_set(struct sunxi_engine *engine, else val = 0; - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base), - SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val); + val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY; + + regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base), val); DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n", interlaced ? "on" : "off"); diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..bc12c95af6f3 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -65,6 +65,7 @@ #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2)) #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n) ((n) << 2) +#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY BIT(0) #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1) #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch) BIT(ch) -- 2.30.2
[PATCH] fbdev: Fix syntax errors in comments
Delete the redundant word 'its'. Signed-off-by: Xiang wangx --- drivers/video/fbdev/skeletonfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/skeletonfb.c b/drivers/video/fbdev/skeletonfb.c index bcacfb6934fa..3d4d78362ede 100644 --- a/drivers/video/fbdev/skeletonfb.c +++ b/drivers/video/fbdev/skeletonfb.c @@ -96,7 +96,7 @@ static const struct fb_fix_screeninfo xxxfb_fix = { /* * Modern graphical hardware not only supports pipelines but some - * also support multiple monitors where each display can have its + * also support multiple monitors where each display can have * its own unique data. In this case each display could be * represented by a separate framebuffer device thus a separate * struct fb_info. Now the struct xxx_par represents the graphics -- 2.36.1
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Saturday, June 4th, 2022 at 23:19, Hans de Goede wrote: > >>> My proposal was: > >>> > >>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). > >>> Only > >>> user-space which supports the hotspot props will enable it. > >>> - By default, don't expose a cursor plane, because current user-space > >>> doesn't > >>> support it (Weston might put a window in the cursor plane, and nobody can > >>> report hotspot). > >>> - If the KMS client enables the cap, advertise the cursor > >>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties > >>> since the driver will pick the position. > >>> > >>> That way both old and new user-space are fixed. > >> > >> I don’t think I see how that fixes anything. In particular I don’t see a > >> way > >> of fixing the old user space at all. We require hotspot info, old user > >> space > >> doesn’t set it because there’s no way of setting it on atomic kms. > > > > Old atomic user-space is fixed by removing the cursor plane. Then old > > atomic user-space will fallback to drawing the cursor itself, e.g. via > > OpenGL. > > That is just a terrible idea, the current situation is that userspace has a > hardcoded list of drivers which need hotspots, so it uses the old non-atomic > APIs on that "hw" since the atomic APIs don't support hotspots. *Some* user-space does this (Mutter, KWin). There is also user-space which doesn't (Weston, wlroots). > The downsides I see to your proposal are: > > 1. Falling back to sw cursor rendering instead of using the old APIs would > be a pretty significant regression in user experience. I know that in theory > sw rendering can be made to not flicker, but in practice it tends to be a > flickery mess. Old Mutter/KWin with the deny-lists cannot be updated and will still use the legacy uAPI. New Muter/KWin with support for atomic hotspot will not need a deny-list anymore. No breakage here. > 2. It does not help anything since userspace is still hardcoded to use > the old, hotspot aware non-atomic API anyways. So it would only make things > worse since hiding the cursorplane for userspace which does not set the CAP > would mean the the old non-atomic API also stops working. Or this would add > extra complications in the kernel to still keep the old APIs working. The cursor plane would only be hidden when user-space has enabled DRM_CAP_UNIVERSAL_PLANES. IOW, user-space only supporting the legacy uAPI will still work as it does today.
Re: [PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS
On Friday, June 3rd, 2022 at 20:31, Zack Rusin wrote: > > > > > > My proposal was: > > > > > > > > > > > > - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better > > > > > > name). Only > > > > > > user-space which supports the hotspot props will enable it. > > > > > > - By default, don't expose a cursor plane, because current > > > > > > user-space doesn't > > > > > > support it (Weston might put a window in the cursor plane, and > > > > > > nobody can > > > > > > report hotspot). > > > > > > - If the KMS client enables the cap, advertise the cursor > > > > > > plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y > > > > > > properties > > > > > > since the driver will pick the position. > > > > > > > > > > > > That way both old and new user-space are fixed. > > > > > > > > > > I don’t think I see how that fixes anything. In particular I don’t > > > > > see a way > > > > > of fixing the old user space at all. We require hotspot info, old > > > > > user space > > > > > doesn’t set it because there’s no way of setting it on atomic kms. > > > > > > > > Old atomic user-space is fixed by removing the cursor plane. Then old > > > > atomic user-space will fallback to drawing the cursor itself, e.g. via > > > > OpenGL. > > > > > > But it’s not fixed, because the driver is still on a deny-list and > > > nothing implements this. You’re saying you could potentially “fix” by > > > implementing client side cursor handling in all kms clients? That’s a > > > hard sell if the user space can just put the virtualized driver on > > > deny-lists and fallback to use old kms which supports hotspots. > > > > What deny-list are you referring to? > > > > All compositors I know of implement a fallback when no cursor plane is > > usable. > > I put the links in the first email in the > series: > https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/backends/native/meta-kms-impl-device-atomic.c#L1188 > https://invent.kde.org/plasma/kwin/-/blob/master/src/backends/drm/drm_gpu.cpp#L156 I was not aware of these lists. Having to work-around drivers in user-space is really not great. But regardless, that doesn't really change anything. With my proposal: - Old user-space with hacky deny-lists (Mutter, KWin) still goes through the legacy uAPI. - Old user-space without the hacky deny-lists (Weston, wlroots) uses software cursors and is not broken anymore. - New user-space can report hotspot and is not broken. > Also, let me point this out because it also seems to be a fundamental > misunderstanding, user space implementing software cursor doesn’t fix > anything. Just leaves everything broken in different ways. The reason > virtualized drivers went away from software cursors is because it makes it > impossible to make it work with a bunch of interesting and desirable > scenarios, e.g. unity mode where the guest might have a terminal and browser > open and then the virtual machine software creates windows out of those, so > you don’t have the entire desktop of the guest but instead native looking > windows with the apps. In that case guest has no way of knowing when the > cursor leaves the window, so to make seemless cursors work you’d need to > implement irc or backdoors that send that information from the host to the > guest, then have virtualized drivers create some sort of uevent, to send to > the userspace to let it know to hide the cursor because it actually left the > window. That’s a trivial scenario and there’s a lot more with mice that are > remoted themselves, guests with singular or maybe many apps exported, > possibly overlapping on the host but all within a desktop in the guest, etc. Are you saying that the current broken behavior is better than software cursors? > > > > New user-space supports setting the hotspot prop, and is aware that it > > > > can't > > > > set the cursor plane position, so the cursor plane can be exposed again > > > > when > > > > the cap is enabled. > > > > > > But we still use cursor plane position. Hotspots are offsets from > > > cursor plane positions. Both are required. > > > > No, VM drivers don't need and disregard the cursor position AFAIK. They > > replace it with the host cursor's position. > > Iirc they all use it. What do they use it for? The whole point of this discussion is to be able to display the guest's cursor image on the host's cursor, so that latency is reduced? When the VM software does this, why does it need to use CRTC_X/CRTC_Y, since these are thrown away by the host? > I guess we could repurpose src_x|y to mean mouse hotspots and write patches > for userspace to use that instead of explicit properties That sounds like a terrible idea.