Re: [RFC 0/6] Regressions for "imply" behavior change
On Thu, Apr 16, 2020 at 4:52 PM Jason Gunthorpe wrote: > On Thu, Apr 16, 2020 at 02:38:50PM +0200, Arnd Bergmann wrote: > > On Thu, Apr 16, 2020 at 12:17 PM Jani Nikula > > wrote: > > > Of course, this is all just talk until someone(tm) posts a patch > > > actually making the change. I've looked at the kconfig tool sources > > > before; not going to make the same mistake again. > > > > Right. OTOH whoever implements it gets to pick the color of the > > bikeshed. ;-) > > I hope someone takes it up, especially now that imply, which > apparently used to do this, doesn't any more :) The old 'imply' was something completely different, it was more of a 'try to select if you can so we can assume it's there, but give up if it can only be a module and we need it to be built-in". Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 4/5] soc / drm: mediatek: Move routing control to mmsys device
Hi, Matthias: Matthias Brugger 於 2020年3月26日 週四 下午11:45寫道： > > > > On 26/03/2020 15:51, CK Hu wrote: > > Hi, Matthias: > > > > On Thu, 2020-03-26 at 12:54 +0100, Matthias Brugger wrote: > >> Hi CK, > >> > >> On 26/03/2020 00:05, CK Hu wrote: > >>> Hi, Matthias: > >>> > >>> On Wed, 2020-03-25 at 17:16 +0100, Matthias Brugger wrote: > > On 11/03/2020 17:53, Enric Balletbo i Serra wrote: > > Provide a mtk_mmsys_ddp_connect() and mtk_mmsys_disconnect() functions > > to > > replace mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path(). > > Those functions will allow DRM driver and others to control the data > > path routing. > > > > Signed-off-by: Enric Balletbo i Serra > > Reviewed-by: Matthias Brugger > > Reviewed-by: CK Hu > > Acked-by: CK Hu > > This patch does not apply against v5.6-rc1. > Please rebase as this is a quite big patch and it won't be easy to do > that by hand. > >>> > >>> I think this patch depends on [1] which has been acked by me and I have > >>> not picked it. The simple way is that you pick [1] first and then pick > >>> this series. > >>> > >>> [1] > >>> https://patchwork.kernel.org/patch/11406227/ > >>> > >> > >> You would need to provide a stable tag for me that I can merge into my > >> tree. You > >> can also try to merge my for-next [1] which has the newest version from > >> Enric. > >> If you see any merge conflict, then we have to do something about it :) > >> > >> Regards, > >> Matthias > >> > >> [1] > >> https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/log/?h=for-next > >> > > > > You have applied this series, so I would not apply other patches which > > would conflict with this series. After this series land on main stream > > (wish it happen in this merge window), I would rebase other patch on > > main stream. > > > > I haven't (yet) send the pull request. If you want to bring in your patches in > v5.7 as well we can find a solution to that. Shall I provide you with a stable > branch which you can merge? This way you can add all your patches in the pull > request as well and we don't have to wait for v5.8 to get things into > mainline. > > Let me know and I'll provide you with a stable branch. This series is in linux-next but does not in main stream. So would you please provide a stable branch so I could pull this series? Regards, Chun-Kuang. > > Regards, > Matthias > > > Regards, > > CK > > > >>> Regards, > >>> CK > >>> > > Regards, > Matthias > > > --- > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: display: dw_mipi_dsi.txt: convert to yaml
Hi Rob,
Thank you for the review, I will address all your points in v2,
however I have a one neclarity below.
On Thu, 16 Apr 2020, Rob Herring wrote:
On Thu, Apr 16, 2020 at 7:51 AM Adrian Ratiu
wrote:
This converts the Synopsis MIPI DSI binding documentation to
yaml and should be quite straightforward. I've added a missing
ref clk and also added Mark and Rob as maintainers based on
'get_maintainer.pl' results.
Cc: Rob Herring Cc: Mark Rutland
Cc: devicet...@vger.kernel.org
Suggested-by: Laurent Pinchart
Signed-off-by: Adrian Ratiu
---
.../bindings/display/bridge/dw_mipi_dsi.txt | 32 -
.../display/bridge/snps,dw-mipi-dsi.yaml | 66
+++ 2 files changed, 66 insertions(+), 32
deletions(-) delete mode 100644
Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
create mode 100644
Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
diff --git
a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
deleted file mode 100644 index b13adf30b8d3.. ---
a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt
+++ /dev/null @@ -1,32 +0,0 @@ -Synopsys DesignWare MIPI DSI
host controller - -
-This document defines device tree properties for the Synopsys
DesignWare MIPI -DSI host controller. It doesn't constitue a
device tree binding specification -by itself but is meant to be
referenced by platform-specific device tree -bindings. - -When
referenced from platform device tree bindings the properties
defined in -this document are defined as follows. The platform
device tree bindings are -responsible for defining whether each
optional property is used or not. - -- reg: Memory mapped base
address and length of the DesignWare MIPI DSI - host
controller registers. (mandatory) - -- clocks: References to
all the clocks specified in the clock-names property - as
specified in [1]. (mandatory) - -- clock-names: - - "pclk" is
the peripheral clock for either AHB and APB. (mandatory) - -
"px_clk" is the pixel clock for the DPI/RGB input. (optional) -
-- resets: References to all the resets specified in the
reset-names property - as specified in [2]. (optional) - --
reset-names: string reset name, must be "apb" if
used. (optional) - -- panel or bridge node: see
[3]. (mandatory) - -[1]
Documentation/devicetree/bindings/clock/clock-bindings.txt -[2]
Documentation/devicetree/bindings/reset/reset.txt -[3]
Documentation/devicetree/bindings/display/mipi-dsi-bus.txt diff
--git
a/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
b/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
new file mode 100644 index ..0ab4125eee30 ---
/dev/null +++
b/Documentation/devicetree/bindings/display/bridge/snps,dw-mipi-dsi.yaml
@@ -0,0 +1,66 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR
BSD-2-Clause) +%YAML 1.2 +--- +$id:
http://devicetree.org/schemas/display/bridge/snps,dw-mipi-dsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml# +
+title: Synopsys DesignWare MIPI DSI host controller +
+maintainers: + - Rob Herring
No thanks, I don't know anything about this hardware. It should
be the owner for this binding, not who applies patches.
Sorry about that, I just followed what get_maintainer.pl -f
returned.
I'm not sure I understand the owner vs maintainers difference for
the "maintainers" entry. How do I find out who is the owner?
Looking at the git log, this file was touched only once when added
in 88dd1e6f9ad8 ("dt-bindings: display: Add Synopsys DW MIPI DSI
host controller") by Philippe CORNU .
Is the person who added the file automatically owner?
(cc'd Philippe)
+ - Mark Rutland
Check current maintainers. Mark is not one anymore.
Yes, I just noticed he got removed in my latest next-20200416
tree, thanks.
+
+description: |
+ This document defines device tree properties for the Synopsys DesignWare MIPI
+ DSI host controller. It doesn't constitue a device tree binding specification
+ by itself but is meant to be referenced by platform-specific device tree
+ bindings.
+
+ When referenced from platform device tree bindings the properties defined in
+ this document are defined as follows. The platform device tree bindings are
+ responsible for defining whether each property is required or optional.
+
Need to reference ($ref) dsi-controller.yaml here.
+properties:
+ reg:
+description: |
+ Memory mapped base address and length of the DesignWare MIPI DSI host
+ controller registers.
Drop the description. That's every 'reg'. You need to say how many
regions (maxItems: 1?).
+
+ clocks:
+description: |
+ References to all the clocks s
Re: [PATCH 1/2] drm: panel: Set connector type for LP120UP1
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 06:44:03PM +0200, Enric Balletbo i Serra wrote:
> The LP120UP1 is a eDP panel, set the connector type accordingly.
>
> Signed-off-by: Enric Balletbo i Serra
Reviewed-by: Laurent Pinchart
> ---
>
> drivers/gpu/drm/panel/panel-simple.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 3ad828eaefe1..6253635601bb 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2168,6 +2168,7 @@ static const struct panel_desc lg_lp120up1 = {
> .width = 267,
> .height = 183,
> },
> + .connector_type = DRM_MODE_CONNECTOR_eDP,
> };
>
> static const struct drm_display_mode lg_lp129qe_mode = {
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Fix page flip ioctl format check
From: Ville Syrjälä
Revert back to comparing fb->format->format instead fb->format for the
page flip ioctl. This check was originally only here to disallow pixel
format changes, but when we changed it to do the pointer comparison
we potentially started to reject some (but definitely not all) modifier
changes as well. In fact the current behaviour depends on whether the
driver overrides the format info for a specific format+modifier combo.
Eg. on i915 this now rejects compression vs. no compression changes but
does not reject any other tiling changes. That's just inconsistent
nonsense.
The main reason we have to go back to the old behaviour is to fix page
flipping with Xorg. At some point Xorg got its atomic rights taken away
and since then we can't page flip between compressed and non-compressed
fbs on i915. Currently we get no page flipping for any games pretty much
since Mesa likes to use compressed buffers. Not sure how compositors are
working around this (don't use one myself). I guess they must be doing
something to get non-compressed buffers instead. Either that or
somehow no one noticed the tearing from the blit fallback.
Looking back at the original discussion on this change we pretty much
just did it in the name of skipping a few extra pointer dereferences.
However, I've decided not to revert the whole thing in case someone
has since started to depend on these changes. None of the other checks
are relevant for i915 anyways.
Cc: sta...@vger.kernel.org
Cc: Laurent Pinchart
Fixes: dbd4d5761e1f ("drm: Replace 'format->format' comparisons to just
'format' comparisons")
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_plane.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index d6ad60ab0d38..f2ca5315f23b 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1153,7 +1153,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
if (ret)
goto out;
- if (old_fb->format != fb->format) {
+ if (old_fb->format->format != fb->format->format) {
DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer
format.\n");
ret = -EINVAL;
goto out;
--
2.24.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/bridge: ps8640: Let panel to set the connector type
Hi Enric, Thank you for the patch. On Thu, Apr 16, 2020 at 06:44:04PM +0200, Enric Balletbo i Serra wrote: > The panel connector type should be set by the panel not the bridge, so > remove the connector_type assignment. > > Signed-off-by: Enric Balletbo i Serra Reviewed-by: Laurent Pinchart > --- > > drivers/gpu/drm/bridge/parade-ps8640.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c > b/drivers/gpu/drm/bridge/parade-ps8640.c > index 956b76e0a44d..13755d278db6 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > @@ -278,8 +278,6 @@ static int ps8640_probe(struct i2c_client *client) > if (!panel) > return -ENODEV; > > - panel->connector_type = DRM_MODE_CONNECTOR_eDP; > - > ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel); > if (IS_ERR(ps_bridge->panel_bridge)) > return PTR_ERR(ps_bridge->panel_bridge); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix page flip ioctl format check
Hi Ville,
Thank you for the patch.
On Thu, Apr 16, 2020 at 08:04:20PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Revert back to comparing fb->format->format instead fb->format for the
> page flip ioctl. This check was originally only here to disallow pixel
> format changes, but when we changed it to do the pointer comparison
> we potentially started to reject some (but definitely not all) modifier
> changes as well. In fact the current behaviour depends on whether the
> driver overrides the format info for a specific format+modifier combo.
> Eg. on i915 this now rejects compression vs. no compression changes but
> does not reject any other tiling changes. That's just inconsistent
> nonsense.
>
> The main reason we have to go back to the old behaviour is to fix page
> flipping with Xorg. At some point Xorg got its atomic rights taken away
> and since then we can't page flip between compressed and non-compressed
> fbs on i915. Currently we get no page flipping for any games pretty much
> since Mesa likes to use compressed buffers. Not sure how compositors are
> working around this (don't use one myself). I guess they must be doing
> something to get non-compressed buffers instead. Either that or
> somehow no one noticed the tearing from the blit fallback.
>
> Looking back at the original discussion on this change we pretty much
> just did it in the name of skipping a few extra pointer dereferences.
> However, I've decided not to revert the whole thing in case someone
> has since started to depend on these changes. None of the other checks
> are relevant for i915 anyways.
Do display controller usually support changing modifiers for page flips
? I understand from the information about that i915 does, but is that
usual ? Could there be drivers that really on this check to reject
modifier changes, and that aren't prepared to handle them if they are
not rejected by the core ? I'm not opposed to this change, but I'd like
to carefully consider the fallout.
> Cc: sta...@vger.kernel.org
> Cc: Laurent Pinchart
> Fixes: dbd4d5761e1f ("drm: Replace 'format->format' comparisons to just
> 'format' comparisons")
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_plane.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index d6ad60ab0d38..f2ca5315f23b 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1153,7 +1153,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> if (ret)
> goto out;
>
> - if (old_fb->format != fb->format) {
> + if (old_fb->format->format != fb->format->format) {
> DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer
> format.\n");
> ret = -EINVAL;
> goto out;
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/7] drm/bridge_connector: Set default status connected for eDP connectors
Hi Enric, Thank you for the patch. On Thu, Apr 16, 2020 at 05:57:14PM +0200, Enric Balletbo i Serra wrote: > In an eDP application, HPD is not required and on most bridge chips > useless. If HPD is not used, we need to set initial status as connected, > otherwise the connector created by the drm_bridge_connector API remains > in an unknown state. > > Signed-off-by: Enric Balletbo i Serra Reviewed-by: Laurent Pinchart > --- > > Changes in v2: None > > drivers/gpu/drm/drm_bridge_connector.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > b/drivers/gpu/drm/drm_bridge_connector.c > index c6994fe673f3..a58cbde59c34 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -187,6 +187,7 @@ drm_bridge_connector_detect(struct drm_connector > *connector, bool force) > case DRM_MODE_CONNECTOR_DPI: > case DRM_MODE_CONNECTOR_LVDS: > case DRM_MODE_CONNECTOR_DSI: > + case DRM_MODE_CONNECTOR_eDP: > status = connector_status_connected; > break; > default: -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/7] drm/bridge: ps8640: Get the EDID from eDP control
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 05:57:13PM +0200, Enric Balletbo i Serra wrote:
> The PS8640 DSI-to-eDP bridge can retrieve the EDID, so implement the
> .get_edid callback and set the flag to indicate the core to use it.
>
> Signed-off-by: Enric Balletbo i Serra
> ---
>
> Changes in v2: None
>
> drivers/gpu/drm/bridge/parade-ps8640.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
> b/drivers/gpu/drm/bridge/parade-ps8640.c
> index d3a53442d449..956b76e0a44d 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -242,8 +242,18 @@ static int ps8640_bridge_attach(struct drm_bridge
> *bridge,
> return ret;
> }
>
> +static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
> +struct drm_connector *connector)
> +{
> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> +
> + return drm_get_edid(connector,
> + ps_bridge->page[PAGE0_DP_CNTL]->adapter);
This will only work if the DDC signals are connected to the PS8640
(quite obviously). Is that guaranteed, or could some systems connect
them directory to an SoC I2C controller ? In the latter case we would
have to report this in the DT bindings of the PS8640. That's not
blocking for this patch, I am just wondering, as I would have expected
the driver to already expose EDID one way or another if this was
available and used.
Reviewed-by: Laurent Pinchart
> +}
> +
> static const struct drm_bridge_funcs ps8640_bridge_funcs = {
> .attach = ps8640_bridge_attach,
> + .get_edid = ps8640_bridge_get_edid,
> .post_disable = ps8640_post_disable,
> .pre_enable = ps8640_pre_enable,
> };
> @@ -296,6 +306,8 @@ static int ps8640_probe(struct i2c_client *client)
>
> ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
> ps_bridge->bridge.of_node = dev->of_node;
> + ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
> + ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP;
>
> ps_bridge->page[PAGE0_DP_CNTL] = client;
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/7] drm/mediatek: mtk_dsi: Rename bridge to next_bridge
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 05:57:15PM +0200, Enric Balletbo i Serra wrote:
> This is really a cosmetic change just to make a bit more readable the
> code after convert the driver to drm_bridge. The bridge variable name
> will be used by the encoder drm_bridge, and the chained bridge will be
> named next_bridge.
>
> Signed-off-by: Enric Balletbo i Serra
> ---
>
> Changes in v2: None
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index cfa45d6abd74..44ee884cc31c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -182,7 +182,7 @@ struct mtk_dsi {
> struct drm_encoder encoder;
> struct drm_connector conn;
> struct drm_panel *panel;
> - struct drm_bridge *bridge;
> + struct drm_bridge *next_bridge;
> struct phy *phy;
>
> void __iomem *regs;
> @@ -903,8 +903,9 @@ static int mtk_dsi_create_conn_enc(struct drm_device
> *drm, struct mtk_dsi *dsi)
> dsi->encoder.possible_crtcs = 1;
>
> /* If there's a bridge, attach to it and let it create the connector */
Maybe s/bridge/next bridge/ here ? I expect this comment to go away
though, as there will always be a next bridge when the driver switches
to the DRM panel bridge helper.
Reviewed-by: Laurent Pinchart
> - if (dsi->bridge) {
> - ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL, 0);
> + if (dsi->next_bridge) {
> + ret = drm_bridge_attach(&dsi->encoder, dsi->next_bridge, NULL,
> + 0);
> if (ret) {
> DRM_ERROR("Failed to attach bridge to drm\n");
> goto err_encoder_cleanup;
> @@ -1185,7 +1186,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> }
>
> ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> - &dsi->panel, &dsi->bridge);
> + &dsi->panel, &dsi->next_bridge);
> if (ret)
> goto err_unregister_host;
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/2] drm/tegra: output: Don't leak OF node on error
Hi Dmitry,
Thank you for the patch.
On Thu, Apr 16, 2020 at 08:24:04PM +0300, Dmitry Osipenko wrote:
> The OF node should be put before returning error in tegra_output_probe(),
> otherwise node's refcount will be leaked.
>
> Signed-off-by: Dmitry Osipenko
Reviewed-by: Laurent Pinchart
> ---
> drivers/gpu/drm/tegra/output.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index e36e5e7c2f69..a6a711d54e88 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -102,10 +102,10 @@ int tegra_output_probe(struct tegra_output *output)
> panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
> if (panel) {
> output->panel = of_drm_find_panel(panel);
> + of_node_put(panel);
> +
> if (IS_ERR(output->panel))
> return PTR_ERR(output->panel);
> -
> - of_node_put(panel);
> }
>
> output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
> @@ -113,13 +113,12 @@ int tegra_output_probe(struct tegra_output *output)
> ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
> if (ddc) {
> output->ddc = of_find_i2c_adapter_by_node(ddc);
> + of_node_put(ddc);
> +
> if (!output->ddc) {
> err = -EPROBE_DEFER;
> - of_node_put(ddc);
> return err;
> }
> -
> - of_node_put(ddc);
> }
>
> output->hpd_gpio = devm_gpiod_get_from_of_node(output->dev,
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/7] drm/mediatek: mtk_dsi: Use simple encoder
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 05:57:17PM +0200, Enric Balletbo i Serra wrote:
> The mtk_dsi driver uses an empty implementation for its encoder. Replace
> the code with the generic simple encoder.
>
> Signed-off-by: Enric Balletbo i Serra
Reviewed-by: Laurent Pinchart
> ---
>
> Changes in v2: None
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 3400d6686c85..48361c1e9f34 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> #include "mtk_drm_ddp_comp.h"
>
> @@ -788,15 +789,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> dsi->enabled = false;
> }
>
> -static void mtk_dsi_encoder_destroy(struct drm_encoder *encoder)
> -{
> - drm_encoder_cleanup(encoder);
> -}
> -
> -static const struct drm_encoder_funcs mtk_dsi_encoder_funcs = {
> - .destroy = mtk_dsi_encoder_destroy,
> -};
> -
> static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi
> *dsi);
> static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi);
>
> @@ -1126,8 +1118,8 @@ static int mtk_dsi_encoder_init(struct drm_device *drm,
> struct mtk_dsi *dsi)
> {
> int ret;
>
> - ret = drm_encoder_init(drm, &dsi->encoder, &mtk_dsi_encoder_funcs,
> -DRM_MODE_ENCODER_DSI, NULL);
> + ret = drm_simple_encoder_init(drm, &dsi->encoder,
> + DRM_MODE_ENCODER_DSI);
> if (ret) {
> DRM_ERROR("Failed to encoder init to drm\n");
> return ret;
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/7] drm/mediatek: mtk_dsi: Use the drm_panel_bridge API
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 05:57:18PM +0200, Enric Balletbo i Serra wrote:
> Replace the manual panel handling code by a drm_panel_bridge. This
> simplifies the driver and allows all components in the display pipeline
> to be treated as bridges, paving the way to generic connector handling.
>
> Signed-off-by: Enric Balletbo i Serra
> ---
>
> Changes in v2:
> - Do not set connector_type for panel here. (Sam Ravnborg)
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 177 -
> 1 file changed, 19 insertions(+), 158 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 48361c1e9f34..44718fa3d1ca 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -182,8 +182,7 @@ struct mtk_dsi {
> struct mipi_dsi_host host;
> struct drm_encoder encoder;
> struct drm_bridge bridge;
> - struct drm_connector conn;
> - struct drm_panel *panel;
> + struct drm_bridge *panel_bridge;
I think you can use the next_bridge field to store the panel bridge, it
will simplify the code a bit in mtk_dsi_bridge_attach().
> struct drm_bridge *next_bridge;
> struct phy *phy;
>
> @@ -212,11 +211,6 @@ static inline struct mtk_dsi *bridge_to_dsi(struct
> drm_bridge *b)
> return container_of(b, struct mtk_dsi, bridge);
> }
>
> -static inline struct mtk_dsi *connector_to_dsi(struct drm_connector *c)
> -{
> - return container_of(c, struct mtk_dsi, conn);
> -}
> -
> static inline struct mtk_dsi *host_to_dsi(struct mipi_dsi_host *h)
> {
> return container_of(h, struct mtk_dsi, host);
> @@ -682,16 +676,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> mtk_dsi_lane0_ulp_mode_leave(dsi);
> mtk_dsi_clk_hs_mode(dsi, 0);
>
> - if (dsi->panel) {
> - if (drm_panel_prepare(dsi->panel)) {
> - DRM_ERROR("failed to prepare the panel\n");
> - goto err_disable_digital_clk;
> - }
> - }
> -
> return 0;
> -err_disable_digital_clk:
> - clk_disable_unprepare(dsi->digital_clk);
> err_disable_engine_clk:
> clk_disable_unprepare(dsi->engine_clk);
> err_phy_power_off:
> @@ -718,15 +703,7 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
>*/
> mtk_dsi_stop(dsi);
>
> - if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
> - if (dsi->panel) {
> - if (drm_panel_unprepare(dsi->panel)) {
> - DRM_ERROR("failed to unprepare the panel\n");
> - return;
> - }
> - }
> - }
> -
> + mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
> mtk_dsi_reset_engine(dsi);
> mtk_dsi_lane0_ulp_mode_enter(dsi);
> mtk_dsi_clk_ulp_mode_enter(dsi);
> @@ -757,19 +734,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
>
> mtk_dsi_start(dsi);
>
> - if (dsi->panel) {
> - if (drm_panel_enable(dsi->panel)) {
> - DRM_ERROR("failed to enable the panel\n");
> - goto err_dsi_power_off;
> - }
> - }
> -
> dsi->enabled = true;
> -
> - return;
> -err_dsi_power_off:
> - mtk_dsi_stop(dsi);
> - mtk_dsi_poweroff(dsi);
> }
>
> static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> @@ -777,34 +742,24 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> if (!dsi->enabled)
> return;
>
> - if (dsi->panel) {
> - if (drm_panel_disable(dsi->panel)) {
> - DRM_ERROR("failed to disable the panel\n");
> - return;
> - }
> - }
> -
> mtk_dsi_poweroff(dsi);
>
> dsi->enabled = false;
> }
>
> -static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi
> *dsi);
> -static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi);
> -
> static int mtk_dsi_bridge_attach(struct drm_bridge *bridge,
>enum drm_bridge_attach_flags flags)
> {
> struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> + struct drm_bridge *next;
>
> - return mtk_dsi_create_conn_enc(bridge->dev, dsi);
> -}
> -
> -static void mtk_dsi_bridge_detach(struct drm_bridge *bridge)
> -{
> - struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> + if (dsi->next_bridge)
> + next = dsi->next_bridge;
> + else
> + next = dsi->panel_bridge;
>
> - mtk_dsi_destroy_conn_enc(dsi);
> + /* Attach the panel or bridge to the dsi bridge */
> + return drm_bridge_attach(bridge->encoder, next, &dsi->bridge, flags);
> }
>
> static void mtk_dsi_bridge_mode_set(struct drm_bridge *bridge,
> @@ -830,101 +785,13 @@ static void mtk_dsi_bridge_enable(struct drm_bridge
> *bridge)
> mtk_output_dsi_enable(dsi);
> }
>
> -static int mtk_dsi_connector_get_modes(str
Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 05:57:19PM +0200, Enric Balletbo i Serra wrote:
> Use the drm_bridge_connector helper to create a connector for pipelines
> that use drm_bridge. This allows splitting connector operations across
> multiple bridges when necessary, instead of having the last bridge in
> the chain creating the connector and handling all connector operations
> internally.
That's the right direction, but this should be done in the mtk display
controller driver core, not in here. I'm OK with the code being here as
an interim measure if needed to move forward, but that should then be
temporary only.
> Signed-off-by: Enric Balletbo i Serra
> ---
>
> Changes in v2: None
>
> drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 44718fa3d1ca..2f8876c32864 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -17,6 +17,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -184,6 +185,7 @@ struct mtk_dsi {
> struct drm_bridge bridge;
> struct drm_bridge *panel_bridge;
> struct drm_bridge *next_bridge;
> + struct drm_connector *connector;
> struct phy *phy;
>
> void __iomem *regs;
> @@ -983,10 +985,19 @@ static int mtk_dsi_encoder_init(struct drm_device *drm,
> struct mtk_dsi *dsi)
>*/
> dsi->encoder.possible_crtcs = 1;
>
> - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
> + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> if (ret)
> goto err_cleanup_encoder;
>
> + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
> + if (IS_ERR(dsi->connector)) {
> + DRM_ERROR("Unable to create bridge connector\n");
> + ret = PTR_ERR(dsi->connector);
> + goto err_cleanup_encoder;
> + }
> + drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
> +
> return 0;
>
> err_cleanup_encoder:
> @@ -1144,6 +1155,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>
> dsi->bridge.funcs = &mtk_dsi_bridge_funcs;
> dsi->bridge.of_node = dev->of_node;
> + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
I think this line belongs to the patch that adds drm_bridge support to
this driver.
>
> drm_bridge_add(&dsi->bridge);
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
Hi Enric,
On Thu, Apr 16, 2020 at 08:35:26PM +0300, Laurent Pinchart wrote:
> On Thu, Apr 16, 2020 at 05:57:19PM +0200, Enric Balletbo i Serra wrote:
> > Use the drm_bridge_connector helper to create a connector for pipelines
> > that use drm_bridge. This allows splitting connector operations across
> > multiple bridges when necessary, instead of having the last bridge in
> > the chain creating the connector and handling all connector operations
> > internally.
>
> That's the right direction, but this should be done in the mtk display
> controller driver core, not in here. I'm OK with the code being here as
> an interim measure if needed to move forward, but that should then be
> temporary only.
I forgot to mention that the drm_encoder should also move out of the
bridge driver to the display controller driver.
> > Signed-off-by: Enric Balletbo i Serra
> > ---
> >
> > Changes in v2: None
> >
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 44718fa3d1ca..2f8876c32864 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -17,6 +17,7 @@
> >
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -184,6 +185,7 @@ struct mtk_dsi {
> > struct drm_bridge bridge;
> > struct drm_bridge *panel_bridge;
> > struct drm_bridge *next_bridge;
> > + struct drm_connector *connector;
> > struct phy *phy;
> >
> > void __iomem *regs;
> > @@ -983,10 +985,19 @@ static int mtk_dsi_encoder_init(struct drm_device
> > *drm, struct mtk_dsi *dsi)
> > */
> > dsi->encoder.possible_crtcs = 1;
> >
> > - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
> > + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> > + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > if (ret)
> > goto err_cleanup_encoder;
> >
> > + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
> > + if (IS_ERR(dsi->connector)) {
> > + DRM_ERROR("Unable to create bridge connector\n");
> > + ret = PTR_ERR(dsi->connector);
> > + goto err_cleanup_encoder;
> > + }
> > + drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
> > +
> > return 0;
> >
> > err_cleanup_encoder:
> > @@ -1144,6 +1155,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> >
> > dsi->bridge.funcs = &mtk_dsi_bridge_funcs;
> > dsi->bridge.of_node = dev->of_node;
> > + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>
> I think this line belongs to the patch that adds drm_bridge support to
> this driver.
>
> >
> > drm_bridge_add(&dsi->bridge);
> >
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Fix page flip ioctl format check
On Thu, Apr 16, 2020 at 08:08:14PM +0300, Laurent Pinchart wrote:
> Hi Ville,
>
> Thank you for the patch.
>
> On Thu, Apr 16, 2020 at 08:04:20PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Revert back to comparing fb->format->format instead fb->format for the
> > page flip ioctl. This check was originally only here to disallow pixel
> > format changes, but when we changed it to do the pointer comparison
> > we potentially started to reject some (but definitely not all) modifier
> > changes as well. In fact the current behaviour depends on whether the
> > driver overrides the format info for a specific format+modifier combo.
> > Eg. on i915 this now rejects compression vs. no compression changes but
> > does not reject any other tiling changes. That's just inconsistent
> > nonsense.
> >
> > The main reason we have to go back to the old behaviour is to fix page
> > flipping with Xorg. At some point Xorg got its atomic rights taken away
> > and since then we can't page flip between compressed and non-compressed
> > fbs on i915. Currently we get no page flipping for any games pretty much
> > since Mesa likes to use compressed buffers. Not sure how compositors are
> > working around this (don't use one myself). I guess they must be doing
> > something to get non-compressed buffers instead. Either that or
> > somehow no one noticed the tearing from the blit fallback.
> >
> > Looking back at the original discussion on this change we pretty much
> > just did it in the name of skipping a few extra pointer dereferences.
> > However, I've decided not to revert the whole thing in case someone
> > has since started to depend on these changes. None of the other checks
> > are relevant for i915 anyways.
>
> Do display controller usually support changing modifiers for page flips
> ? I understand from the information about that i915 does, but is that
> usual ? Could there be drivers that really on this check to reject
> modifier changes, and that aren't prepared to handle them if they are
> not rejected by the core ? I'm not opposed to this change, but I'd like
> to carefully consider the fallout.
After a bit of grepping I can't actually see any other driver providing
a .get_format_info() hook. So looks like there is no change in behaviour
for any other driver. Based on that we could even do a full revert, but
meh.
>
> > Cc: sta...@vger.kernel.org
> > Cc: Laurent Pinchart
> > Fixes: dbd4d5761e1f ("drm: Replace 'format->format' comparisons to just
> > 'format' comparisons")
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_plane.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index d6ad60ab0d38..f2ca5315f23b 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -1153,7 +1153,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> > if (ret)
> > goto out;
> >
> > - if (old_fb->format != fb->format) {
> > + if (old_fb->format->format != fb->format->format) {
> > DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer
> > format.\n");
> > ret = -EINVAL;
> > goto out;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: android: ion: Skip sync if not mapped
Great! Thanks! regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry,
Thank you for the patch.
On Thu, Apr 16, 2020 at 08:24:05PM +0300, Dmitry Osipenko wrote:
> Newer Tegra device-trees will specify a video output graph that involves
> LVDS encoder bridge, This patch adds support for the LVDS encoder bridge
> to the RGB output, allowing us to model display hardware properly.
>
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/tegra/drm.h| 2 ++
> drivers/gpu/drm/tegra/output.c | 10 ++
> drivers/gpu/drm/tegra/rgb.c| 34 ++
> 3 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 804869799305..cccd368b6752 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -12,6 +12,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -116,6 +117,7 @@ struct tegra_output {
> struct device_node *of_node;
> struct device *dev;
>
> + struct drm_bridge *bridge;
> struct drm_panel *panel;
> struct i2c_adapter *ddc;
> const struct edid *edid;
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index a6a711d54e88..37fc6b8c173f 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -180,6 +180,16 @@ int tegra_output_init(struct drm_device *drm, struct
> tegra_output *output)
> int connector_type;
> int err;
>
> + if (output->bridge) {
> + err = drm_bridge_attach(&output->encoder, output->bridge,
> + NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
Using DRM_BRIDGE_ATTACH_NO_CONNECTOR is definitely the way to go, but
please be aware that it will require creating a connector and an encoder
manually (which I think this driver already does), and using the bridge
operations to implement the connector operations. I highly recommend
using the DRM bridge connector helper for this purpose.
> + if (err) {
> + dev_err(output->dev, "cannot connect bridge: %d\n",
> + err);
> + return err;
> + }
> + }
> +
> if (output->panel) {
May I also recommend switching to the DRM panel bridge helper ? It will
simplify the code.
> err = drm_panel_attach(output->panel, &output->connector);
> if (err < 0)
> diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
> index 0562a7eb793f..0df213e92664 100644
> --- a/drivers/gpu/drm/tegra/rgb.c
> +++ b/drivers/gpu/drm/tegra/rgb.c
> @@ -5,6 +5,7 @@
> */
>
> #include
> +#include
>
> #include
> #include
> @@ -210,6 +211,7 @@ static const struct drm_encoder_helper_funcs
> tegra_rgb_encoder_helper_funcs = {
>
> int tegra_dc_rgb_probe(struct tegra_dc *dc)
> {
> + const unsigned int encoder_port = 0, panel_port = 1;
> struct device_node *np;
> struct tegra_rgb *rgb;
> int err;
> @@ -226,6 +228,38 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
> rgb->output.of_node = np;
> rgb->dc = dc;
>
> + /*
> + * Tegra devices that have LVDS panel utilize LVDS-encoder bridge
> + * for converting 24/18 RGB data-lanes into 8 lanes. Encoder usually
> + * have a powerdown control which needs to be enabled in order to
> + * transfer data to the panel. Historically devices that use an older
> + * device-tree version didn't model the bridge, assuming that encoder
> + * is turned ON by default, while today's DRM allows us to model LVDS
> + * encoder properly.
> + *
> + * Newer device-trees may utilize output->encoder->panel graph.
> + *
> + * For older device-trees we fall back to use nvidia,panel phandle.
> + */
> + np = of_graph_get_remote_node(rgb->output.of_node, encoder_port, -1);
> + if (np) {
> + rgb->output.bridge = of_drm_find_bridge(np);
> + of_node_put(np);
> +
> + if (!rgb->output.bridge)
> + return -EPROBE_DEFER;
> +
> + np = of_graph_get_remote_node(rgb->output.bridge->of_node,
> + panel_port, -1);
This shouldn't be needed, the LVDS codec bridge driver should already
get the panel and handle it internally. You only need to handle panels
in this driver when they're connected directly to the RGB input.
> + if (np) {
> + rgb->output.panel = of_drm_find_panel(np);
> + of_node_put(np);
> +
> + if (IS_ERR(rgb->output.panel))
> + return PTR_ERR(rgb->output.panel);
> + }
> + }
> +
> err = tegra_output_probe(&rgb->output);
> if (err < 0)
> return err;
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo
Re: [PATCH v3 1/2] drm/tegra: output: Don't leak OF node on error
Hi Dmitry, On Thu, Apr 16, 2020 at 08:30:24PM +0300, Dmitry Osipenko wrote: > 16.04.2020 20:27, Laurent Pinchart пишет: > > On Thu, Apr 16, 2020 at 08:24:04PM +0300, Dmitry Osipenko wrote: > >> The OF node should be put before returning error in tegra_output_probe(), > >> otherwise node's refcount will be leaked. > >> > >> Signed-off-by: Dmitry Osipenko > > > > Reviewed-by: Laurent Pinchart > > Hello Laurent, > > That is the fastest kernel review I ever got, thank you :) I think I'm also guilty for the slowest kernel reviews ever, so maybe in the end it will even out :-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/8] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
On Wed, 15 Apr 2020, Enric Balletbo Serra
wrote:
Hi Adrian,
Some few comments/nits below,
Hi Enric,
Thank you for your review, all your suggested changes will be part
of v7. I have only one minor comment below.
Missatge de Adrian Ratiu del dia
dt., 14 d’abr. 2020 a les 17:19:
In order to support multiple versions of the Synopsis MIPI DSI
host controller, which have different register layouts but
almost identical HW protocols, we add a regmap infrastructure
which can abstract away register accesses for platform drivers
using the bridge.
The controller HW revision is detected during bridge probe
which will be used in future commits to load the relevant
register layout which the bridge will use transparently to the
platform drivers.
Suggested-by: Ezequiel Garcia
Tested-by: Adrian Pop Tested-by:
Arnaud Ferraris Signed-off-by:
Adrian Ratiu --- New in v5. ---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208
++ 1 file changed, 117 insertions(+), 91
deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
5ef0f154aa7b..6d9e2f21c9cc 100644 ---
a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -15,6 +15,7
@@
#include #include
#include
+#include
Should Kconfig select REGMAP for this driver?
#include
#include
@@ -227,6 +228,7 @@ struct dw_mipi_dsi {
struct drm_bridge *panel_bridge; struct device *dev;
void __iomem *base;
+ struct regmap *regs;
struct clk *pclk;
@@ -235,6 +237,7 @@ struct dw_mipi_dsi {
u32 lanes; u32 format; unsigned long mode_flags;
+ u32 hw_version;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
@@ -249,6 +252,13 @@ struct dw_mipi_dsi {
const struct dw_mipi_dsi_plat_data *plat_data;
};
+static const struct regmap_config dw_mipi_dsi_regmap_cfg = { +
.reg_bits = 32, + .val_bits = 32, + .reg_stride =
4, + .name = "dw-mipi-dsi", +}; +
/*
* Check if either a link to a master or slave is present */
@@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi
*bridge_to_dsi(struct drm_bridge *bridge)
return container_of(bridge, struct dw_mipi_dsi,
bridge);
}
-static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg,
u32 val) -{ - writel(val, dsi->base + reg); -} - -static
inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg) -{ -
return readl(dsi->base + reg); -} -
static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device
*device)
{
@@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct
dw_mipi_dsi *dsi,
if (lpm)
val |= CMD_MODE_ALL_LP;
- dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); - dsi_write(dsi, DSI_CMD_MODE_CFG,
val); + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); + regmap_write(dsi->regs,
DSI_CMD_MODE_CFG, val);
}
static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi
*dsi, u32 hdr_val) {
int ret;
- u32 val, mask; + u32 val = 0, mask;
I think that this change is not needed, `val` is an input
variable that will be overwritten inside the
regmap_read_poll_timeout. Initialize here is not needed.
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, !(val
& GEN_CMD_FULL), 1000, -
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, !(val & GEN_CMD_FULL), 1000, +
CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to get available
command FIFO\n"); return ret;
}
- dsi_write(dsi, DSI_GEN_HDR, hdr_val); +
regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, (val
& mask) == mask, -1000,
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, (val & mask) == mask, +
1000, CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to write command
FIFO\n"); return ret;
@@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct
dw_mipi_dsi *dsi,
const u8 *tx_buf = packet->payload; int len =
packet->payload_length, pld_data_bytes = sizeof(u32),
ret; __le32 word;
- u32 val; + u32 val = 0;
The same here, 'val' is used for the regmap_read_poll_timeout
and will be overwritten, no need to initialize.
while (len) {
if (len < pld_data_bytes) {
word = 0; memcpy(&word, tx_buf, le
Re: [PATCH v6 1/8] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
On Wed, 15 Apr 2020, Enric Balletbo Serra
wrote:
Hi Adrian,
Some few comments/nits below,
Missatge de Adrian Ratiu del dia
dt., 14 d’abr. 2020 a les 17:19:
In order to support multiple versions of the Synopsis MIPI DSI
host controller, which have different register layouts but
almost identical HW protocols, we add a regmap infrastructure
which can abstract away register accesses for platform drivers
using the bridge.
The controller HW revision is detected during bridge probe
which will be used in future commits to load the relevant
register layout which the bridge will use transparently to the
platform drivers.
Suggested-by: Ezequiel Garcia
Tested-by: Adrian Pop Tested-by:
Arnaud Ferraris Signed-off-by:
Adrian Ratiu --- New in v5. ---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208
++ 1 file changed, 117 insertions(+), 91
deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
5ef0f154aa7b..6d9e2f21c9cc 100644 ---
a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -15,6 +15,7
@@
#include #include
#include
+#include
Should Kconfig select REGMAP for this driver?
#include
#include
@@ -227,6 +228,7 @@ struct dw_mipi_dsi {
struct drm_bridge *panel_bridge; struct device *dev;
void __iomem *base;
+ struct regmap *regs;
struct clk *pclk;
@@ -235,6 +237,7 @@ struct dw_mipi_dsi {
u32 lanes; u32 format; unsigned long mode_flags;
+ u32 hw_version;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
@@ -249,6 +252,13 @@ struct dw_mipi_dsi {
const struct dw_mipi_dsi_plat_data *plat_data;
};
+static const struct regmap_config dw_mipi_dsi_regmap_cfg = { +
.reg_bits = 32, + .val_bits = 32, + .reg_stride =
4, + .name = "dw-mipi-dsi", +}; +
/*
* Check if either a link to a master or slave is present */
@@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi
*bridge_to_dsi(struct drm_bridge *bridge)
return container_of(bridge, struct dw_mipi_dsi,
bridge);
}
-static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg,
u32 val) -{ - writel(val, dsi->base + reg); -} - -static
inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg) -{ -
return readl(dsi->base + reg); -} -
static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device
*device)
{
@@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct
dw_mipi_dsi *dsi,
if (lpm)
val |= CMD_MODE_ALL_LP;
- dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); - dsi_write(dsi, DSI_CMD_MODE_CFG,
val); + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 :
PHY_TXREQUESTCLKHS); + regmap_write(dsi->regs,
DSI_CMD_MODE_CFG, val);
}
static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi
*dsi, u32 hdr_val) {
int ret;
- u32 val, mask; + u32 val = 0, mask;
I think that this change is not needed, `val` is an input
variable that will be overwritten inside the
regmap_read_poll_timeout. Initialize here is not needed.
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, !(val
& GEN_CMD_FULL), 1000, -
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, !(val & GEN_CMD_FULL), 1000, +
CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to get available
command FIFO\n"); return ret;
}
- dsi_write(dsi, DSI_GEN_HDR, hdr_val); +
regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
- ret = readl_poll_timeout(dsi->base +
DSI_CMD_PKT_STATUS, -val, (val
& mask) == mask, -1000,
CMD_PKT_STATUS_TIMEOUT_US); + ret =
regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS, +
val, (val & mask) == mask, +
1000, CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to write command
FIFO\n"); return ret;
@@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct
dw_mipi_dsi *dsi,
const u8 *tx_buf = packet->payload; int len =
packet->payload_length, pld_data_bytes = sizeof(u32),
ret; __le32 word;
- u32 val; + u32 val = 0;
The same here, 'val' is used for the regmap_read_poll_timeout
and will be overwritten, no need to initialize.
while (len) {
if (len < pld_data_bytes) {
word = 0; memcpy(&word, tx_buf, len);
- dsi_write(dsi, DSI_GEN_PLD_DATA,
le32_to_cpu(word)); +
regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
Re: [Intel-gfx] [PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios
On Thu, 16 Apr 2020, Alex Deucher wrote:
> On Wed, Apr 15, 2020 at 6:05 AM Jani Nikula wrote:
>>
>>
>> Alex, Harry, Christian, can you please eyeball this series and see if it
>> makes sense for you?
>>
>
> Patches 4, 5 are:
> Acked-by: Alex Deucher
> Feel free to take them through whichever tree you want.
Thanks a bunch! I'll let you know.
BR,
Jani.
>
> Alex
>
>
>> Thanks,
>> Jani.
>>
>>
>> On Tue, 07 Apr 2020, Jeevan B wrote:
>> > From: Oleg Vasilev
>> >
>> > Since DP-specific information is stored in driver's structures, every
>> > driver needs to implement subconnector property by itself.
>> >
>> > v2: rebase
>> >
>> > Cc: Alex Deucher
>> > Cc: Christian König
>> > Cc: David (ChunMing) Zhou
>> > Cc: amd-...@lists.freedesktop.org
>> > Signed-off-by: Jeevan B
>> > Signed-off-by: Oleg Vasilev
>> > Reviewed-by: Emil Velikov
>> > Link:
>> > https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasi...@intel.com
>> > ---
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 +
>> > drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 18 +-
>> > 3 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > index f355d9a..71aade0 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > @@ -26,6 +26,7 @@
>> >
>> > #include
>> > #include
>> > +#include
>> > #include
>> > #include
>> > #include "amdgpu.h"
>> > @@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector
>> > *connector, bool force)
>> > pm_runtime_put_autosuspend(connector->dev->dev);
>> > }
>> >
>> > + drm_dp_set_subconnector_property(&amdgpu_connector->base,
>> > + ret,
>> > + amdgpu_dig_connector->dpcd,
>> > +
>> > amdgpu_dig_connector->downstream_ports);
>> > return ret;
>> > }
>> >
>> > @@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>> > if (has_aux)
>> > amdgpu_atombios_dp_aux_init(amdgpu_connector);
>> >
>> > + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> > + connector_type == DRM_MODE_CONNECTOR_eDP) {
>> > +
>> > drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
>> > + }
>> > +
>> > return;
>> >
>> > failed:
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > index 37ba07e..04a430e 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > @@ -469,6 +469,7 @@ struct amdgpu_encoder {
>> > struct amdgpu_connector_atom_dig {
>> > /* displayport */
>> > u8 dpcd[DP_RECEIVER_CAP_SIZE];
>> > + u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>> > u8 dp_sink_type;
>> > int dp_clock;
>> > int dp_lane_count;
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > index 9b74cfd..900b272 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > @@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct
>> > amdgpu_connector *amdgpu_connect
>> > buf[0], buf[1], buf[2]);
>> > }
>> >
>> > +static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector
>> > *amdgpu_connector)
>> > +{
>> > + struct amdgpu_connector_atom_dig *dig_connector =
>> > amdgpu_connector->con_priv;
>> > + int ret;
>> > +
>> > + if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
>> > + ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
>> > +DP_DOWNSTREAM_PORT_0,
>> > +dig_connector->downstream_ports,
>> > +DP_MAX_DOWNSTREAM_PORTS);
>> > + if (ret)
>> > + memset(dig_connector->downstream_ports, 0,
>> > +DP_MAX_DOWNSTREAM_PORTS);
>> > + }
>> > +}
>> > +
>> > int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
>> > {
>> > struct amdgpu_connector_atom_dig *dig_connector =
>> > amdgpu_connector->con_priv;
>> > @@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct
>> > amdgpu_connector *amdgpu_connector)
>> > dig_connector->dpcd);
>> >
>> > amdgpu_atombios_dp_probe_oui(amdgpu_connector);
>> > -
>> > + amdgpu_atombios_dp_ds_ports(amdgpu_connector);
>> > return 0;
>> > }
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
>> ___
>> dri-devel mailing list
>> dri-de
Re: [PATCH trivial 1/6] dt-bindings: Fix misspellings of "Analog Devices"
On Thu, 16 Apr 2020 12:30:53 +0200, Geert Uytterhoeven wrote: > According to https://www.analog.com/, the company name is spelled > "Analog Devices". > > Signed-off-by: Geert Uytterhoeven > --- > .../devicetree/bindings/display/bridge/adi,adv7123.txt| 4 ++-- > .../devicetree/bindings/display/bridge/adi,adv7511.txt| 4 ++-- > Documentation/devicetree/bindings/dma/adi,axi-dmac.txt| 2 +- > Documentation/devicetree/bindings/iio/dac/ad5755.txt | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > Applied, thanks. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/6] Regressions for "imply" behavior change
On 16.04.2020 20:21, Jason Gunthorpe wrote: > On Thu, Apr 16, 2020 at 11:12:56AM -0400, Nicolas Pitre wrote: >> On Thu, 16 Apr 2020, Arnd Bergmann wrote: >> >>> On Thu, Apr 16, 2020 at 12:17 PM Jani Nikula >>> wrote: On Thu, 16 Apr 2020, Arnd Bergmann wrote: > On Thu, Apr 16, 2020 at 5:25 AM Saeed Mahameed > wrote: >> BTW how about adding a new Kconfig option to hide the details of >> ( BAR || !BAR) ? as Jason already explained and suggested, this will >> make it easier for the users and developers to understand the actual >> meaning behind this tristate weird condition. >> >> e.g have a new keyword: >> reach VXLAN >> which will be equivalent to: >> depends on VXLAN && !VXLAN > I'd love to see that, but I'm not sure what keyword is best. For your > suggestion of "reach", that would probably do the job, but I'm not > sure if this ends up being more or less confusing than what we have > today. Ah, perfect bikeshedding topic! Perhaps "uses"? If the dependency is enabled it gets used as a dependency. >>> That seems to be the best naming suggestion so far >> What I don't like about "uses" is that it doesn't convey the conditional >> dependency. It could be mistaken as being synonymous to "select". >> >> What about "depends_if" ? The rationale is that this is actually a >> dependency, but only if the related symbol is set (i.e. not n or empty). > I think that stretches the common understanding of 'depends' a bit too > far.. A depends where the target can be N is just too strange. > > Somthing incorporating 'optional' seems like a better choice > 'optionally uses' seems particularly clear and doesn't overload > existing works like depends or select I think the whole misunderstanding with imply is that ppl try to use it as weak 'depend on' but it is weak 'select' - ie it enforces value of implied symbol in contrast to 'depends' which enforces value of current symbol. So if we want to add new symbol 'weak depend' it would be good to stress out that difference. Moreover name imply is already cryptic, adding another keyword which for sure will be cryptic (as there are no natural candidates) will complicate things more. Maybe adding some decorator would be better, like optionally or weak, for example: optionally depends on X optionally select Y Even more readable: depends on X if on depends on Y if enabled Regards Andrzej > > Jason > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] AMDGPU: Correctly initialize thermal controller for GPUs with Powerplay table v0 (e.g Hawaii)
Initialize thermal controller fields in the PowerPlay table for Hawaii
GPUs, so that fan speeds are reported.
Signed-off-by: Sandeep Raghuraman
---
.../drm/amd/powerplay/hwmgr/processpptables.c | 28 +++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
index 77c14671866c..bb58cfab1033 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
@@ -984,6 +984,34 @@ static int init_thermal_controller(
struct pp_hwmgr *hwmgr,
const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
{
+ hwmgr->thermal_controller.ucType =
+ powerplay_table->sThermalController.ucType;
+ hwmgr->thermal_controller.ucI2cLine =
+ powerplay_table->sThermalController.ucI2cLine;
+ hwmgr->thermal_controller.ucI2cAddress =
+ powerplay_table->sThermalController.ucI2cAddress;
+
+ hwmgr->thermal_controller.fanInfo.bNoFan =
+ (0 != (powerplay_table->sThermalController.ucFanParameters &
+ ATOM_PP_FANPARAMETERS_NOFAN));
+
+ hwmgr->thermal_controller.fanInfo.ucTachometerPulsesPerRevolution =
+ powerplay_table->sThermalController.ucFanParameters &
+ ATOM_PP_FANPARAMETERS_TACHOMETER_PULSES_PER_REVOLUTION_MASK;
+
+ hwmgr->thermal_controller.fanInfo.ulMinRPM
+ = powerplay_table->sThermalController.ucFanMinRPM * 100UL;
+ hwmgr->thermal_controller.fanInfo.ulMaxRPM
+ = powerplay_table->sThermalController.ucFanMaxRPM * 100UL;
+
+ set_hw_cap(
+ hwmgr,
+ ATOM_PP_THERMALCONTROLLER_NONE !=
hwmgr->thermal_controller.ucType,
+ PHM_PlatformCaps_ThermalController
+ );
+
+ hwmgr->thermal_controller.use_hw_fan_control = 1;
+
return 0;
}
--
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 1/8] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
Hi Adrian,
[snip]
> >>
> >> +static void dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi
> >> *dsi) +{ + regmap_read(dsi->regs, DSI_VERSION,
> >> &dsi->hw_version); + dsi->hw_version &= VERSION; +
> >> if (!dsi->hw_version) + dev_err(dsi->dev, "Failed
> >> to read DSI hw version register\n");
> >
> > Is this an error that should be ignored? If you can't get the HW
> > version, probably, there is something wrong with your hardware
> > so, don't you need to return an error?
> >
>
> After thinking a bit more about it, that error should be a
> warning.
>
> I added it because in some cases (for eg. if the peripheral clock
> is disabled) the reads can return 0 which is obviously an invalid
> version and the bridge will error in the next step when not
> finding a layout.
>
If you'll error anyway, why wait? IIUC at this point the clock *must*
be enabled, and if not, something is wrong with the driver, I don't
see any advantage on delay the error. do you have a use case where
this is called and peripheral clock disabled?
> So I'll make this a warning in v7 and explicitely mention that
> reads version == 0 can be caused by a disabled pclk.
>
-- Enric
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] AMDGPU: Correctly initialize thermal controller for GPUs with Powerplay table v0 (e.g Hawaii)
On Thu, Apr 16, 2020 at 4:07 PM Sandeep Raghuraman wrote:
>
> Initialize thermal controller fields in the PowerPlay table for Hawaii
> GPUs, so that fan speeds are reported.
>
> Signed-off-by: Sandeep Raghuraman
Applied. Thanks!
Alex
>
> ---
> .../drm/amd/powerplay/hwmgr/processpptables.c | 28 +++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
> index 77c14671866c..bb58cfab1033 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c
> @@ -984,6 +984,34 @@ static int init_thermal_controller(
> struct pp_hwmgr *hwmgr,
> const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
> {
> + hwmgr->thermal_controller.ucType =
> + powerplay_table->sThermalController.ucType;
> + hwmgr->thermal_controller.ucI2cLine =
> + powerplay_table->sThermalController.ucI2cLine;
> + hwmgr->thermal_controller.ucI2cAddress =
> + powerplay_table->sThermalController.ucI2cAddress;
> +
> + hwmgr->thermal_controller.fanInfo.bNoFan =
> + (0 != (powerplay_table->sThermalController.ucFanParameters &
> + ATOM_PP_FANPARAMETERS_NOFAN));
> +
> + hwmgr->thermal_controller.fanInfo.ucTachometerPulsesPerRevolution =
> + powerplay_table->sThermalController.ucFanParameters &
> + ATOM_PP_FANPARAMETERS_TACHOMETER_PULSES_PER_REVOLUTION_MASK;
> +
> + hwmgr->thermal_controller.fanInfo.ulMinRPM
> + = powerplay_table->sThermalController.ucFanMinRPM * 100UL;
> + hwmgr->thermal_controller.fanInfo.ulMaxRPM
> + = powerplay_table->sThermalController.ucFanMaxRPM * 100UL;
> +
> + set_hw_cap(
> + hwmgr,
> + ATOM_PP_THERMALCONTROLLER_NONE !=
> hwmgr->thermal_controller.ucType,
> + PHM_PlatformCaps_ThermalController
> + );
> +
> + hwmgr->thermal_controller.use_hw_fan_control = 1;
> +
> return 0;
> }
>
> --
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dimitry. On Thu, Apr 16, 2020 at 11:21:40PM +0300, Dmitry Osipenko wrote: > 16.04.2020 21:52, Dmitry Osipenko пишет: > ... > >> May I also recommend switching to the DRM panel bridge helper ? It will > >> simplify the code. > > > > Could you please clarify what is the "DRM panel bridge helper"? > > > > I think we won't need any additional helpers after switching to the > > bridge connector helper, no? > > Actually, I now see that the panel needs to be manually attached to the > connector. > > Still it's not apparent to me how to get panel out of the bridge. It > looks like there is no such "panel helper" for the bridge API or I just > can't find it. Take a look in bridge/panel.c I think devm_drm_panel_bridge_add() is what you are after. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
Hi Christoph, Sorry for the late. On Sat, Apr 11, 2020 at 09:20:52AM +0200, Christoph Hellwig wrote: > Hi Minchan, > > On Fri, Apr 10, 2020 at 04:11:36PM -0700, Minchan Kim wrote: > > It doesn't mean we couldn't use zsmalloc as module any longer. It means > > we couldn't use zsmalloc as module with pgtable mapping whcih was little > > bit faster on microbenchmark in some architecutre(However, I usually temped > > to remove it since it had several problems). However, we could still use > > zsmalloc as module as copy way instead of pgtable mapping. Thus, if someone > > really want to rollback the feature, they should provide reasonable reason > > why it doesn't work for them. "A little fast" wouldn't be enough to exports > > deep internal to the module. > > do you have any data how much faster it is on arm (and does that include > arm64 as well)? Besides the exports which were my prime concern, https://github.com/sjenning/zsmapbench I need to recall the memory. IIRC, it was almost 30% faster at that time in ARM so was not trivial at that time. However, it was story from several years ago. > zsmalloc with pgtable mappings also is the only user of map_kernel_range > outside of vmalloc.c, if it really is another code base for tiny > improvements we could mark map_kernel_range or in fact remove it entirely > and open code it in the remaining callers. I alsh have temped to remove it. Let me have time to revist it in this chance. Thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
On Wed, 15 Apr 2020 10:35:08 +0200, "H. Nikolaus Schaller" wrote: > The Imagination PVR/SGX GPU is part of several SoC from > multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo, > Allwinner A83 and others. > > With this binding, we describe how the SGX processor is > interfaced to the SoC (registers, interrupt etc.). > > In most cases, Clock, Reset and power management is handled > by a parent node or elsewhere (e.g. code in the driver). > > Tested by make dt_binding_check dtbs_check > > Signed-off-by: H. Nikolaus Schaller > --- > .../devicetree/bindings/gpu/img,pvrsgx.yaml | 122 ++ > 1 file changed, 122 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml: while parsing a block mapping in "", line 74, column 13 did not find expected key in "", line 117, column 21 Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/gpu/img,pvrsgx.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/gpu/img,pvrsgx.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs Makefile:1264: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1270997 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/29] mm: only allow page table mappings for built-in zsmalloc
On Tue, Apr 14, 2020 at 03:13:30PM +0200, Christoph Hellwig wrote: > This allows to unexport map_vm_area and unmap_kernel_range, which are > rather deep internal and should not be available to modules, as they for > example allow fine grained control of mapping permissions, and also > allow splitting the setup of a vmalloc area and the actual mapping and > thus expose vmalloc internals. > > zsmalloc is typically built-in and continues to work (just like the > percpu-vm code using a similar patter), while modular zsmalloc also > continues to work, but must use copies. > > Signed-off-by: Christoph Hellwig > Acked-by: Peter Zijlstra (Intel) Acked-by: Minchan Kim Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry, On Thu, Apr 16, 2020 at 11:21:40PM +0300, Dmitry Osipenko wrote: > 16.04.2020 21:52, Dmitry Osipenko пишет: > ... > >> May I also recommend switching to the DRM panel bridge helper ? It will > >> simplify the code. > > > > Could you please clarify what is the "DRM panel bridge helper"? > > > > I think we won't need any additional helpers after switching to the > > bridge connector helper, no? > > Actually, I now see that the panel needs to be manually attached to the > connector. The DRM panel bridge helper creates a bridge from a panel (with devm_drm_panel_bridge_add()). You can then attach that bridge to the chain, like any other bridge, and the enable/disable operations will be called automatically without any need to call the panel enable/disable manually as done currently. > Still it's not apparent to me how to get panel out of the bridge. It > looks like there is no such "panel helper" for the bridge API or I just > can't find it. You don't need to get a panel out of the bridge. You should get the bridge as done today, and wrap it in a bridge with devm_drm_panel_bridge_add(). -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/tegra: output: rgb: Support LVDS encoder bridge
Hi Dmitry, On Fri, Apr 17, 2020 at 12:15:33AM +0300, Dmitry Osipenko wrote: > 16.04.2020 23:50, Laurent Pinchart пишет: > > On Thu, Apr 16, 2020 at 11:21:40PM +0300, Dmitry Osipenko wrote: > >> 16.04.2020 21:52, Dmitry Osipenko пишет: > >> ... > May I also recommend switching to the DRM panel bridge helper ? It will > simplify the code. > >>> > >>> Could you please clarify what is the "DRM panel bridge helper"? > >>> > >>> I think we won't need any additional helpers after switching to the > >>> bridge connector helper, no? > >> > >> Actually, I now see that the panel needs to be manually attached to the > >> connector. > > > > The DRM panel bridge helper creates a bridge from a panel (with > > devm_drm_panel_bridge_add()). You can then attach that bridge to the > > chain, like any other bridge, and the enable/disable operations will be > > called automatically without any need to call the panel enable/disable > > manually as done currently. > > > >> Still it's not apparent to me how to get panel out of the bridge. It > >> looks like there is no such "panel helper" for the bridge API or I just > >> can't find it. > > > > You don't need to get a panel out of the bridge. You should get the > > bridge as done today, > > You mean "get the panel", correct? Yes, sorry. > > and wrap it in a bridge with > > devm_drm_panel_bridge_add(). > > > > The lvds-codec already wraps panel into the bridge using > devm_drm_panel_bridge_add() and chains it for us, please see > lvds_codec_probe() / lvds_codec_attach(). > > Does it mean that lvds-codec is not supporting the new model? No, that *is* the new model :-) As I think I've commented during review, when you have an LVDS encoder and a panel, you don't need to handle the panel at all. When you have an RGB panel connected directly to the RGB output, you need to wrap it in a bridge, exactly the same way as lvds-codec does for its panel. > Everything works nicely using the old model, where bridge creates > connector and attaches panel to it for us. > > [I'm still not sure what is the point to use the new model in a case of > a simple chain of bridges] > > Using the new model, the connector isn't created by the bridge, so I > need to duplicate that creation effort in the driver. Once the bridge > connector is manually created, I need to attach panel to this connector, > but panel is reachable only via the remote bridge (which wraps the panel). > > driver connector -> LVDS bridge -> panel bridge -> panel With the new model, 1. The display driver and the bridge drivers need to get hold of the bridge directly connected to their output (for instance with of_drm_find_panel()). If the output is connected to a panel, they need to wrap that panel in a bridge (with devm_drm_panel_bridge_add()). I plan, in the future, to make creation of panel bridges automatic, so drivers won't have to care. 2. The display driver needs to create a dummy drm_encoder for each of its outputs (for instance with drm_simple_encoder_init()). 3. The display driver needs to create a drm_connector for each of its outputs, and implement connector operations by delegating them to the bridges in the pipeline. Unless there's a good reason not to do so, this should be done with drm_bridge_connector_init(). That's it. Every driver then focusses on its own needs, bridge drivers handle only the device they're associated with, and the DRM core and DRM bridge connector helper will handle all the rest. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
Hi Enric,
On Thu, Apr 16, 2020 at 11:33:24PM +0200, Enric Balletbo i Serra wrote:
> On 16/4/20 19:36, Laurent Pinchart wrote:
> > On Thu, Apr 16, 2020 at 08:35:26PM +0300, Laurent Pinchart wrote:
> >> On Thu, Apr 16, 2020 at 05:57:19PM +0200, Enric Balletbo i Serra wrote:
> >>> Use the drm_bridge_connector helper to create a connector for pipelines
> >>> that use drm_bridge. This allows splitting connector operations across
> >>> multiple bridges when necessary, instead of having the last bridge in
> >>> the chain creating the connector and handling all connector operations
> >>> internally.
> >>
> >> That's the right direction, but this should be done in the mtk display
> >> controller driver core, not in here. I'm OK with the code being here as
> >> an interim measure if needed to move forward, but that should then be
> >> temporary only.
>
> It'd be nice if we can do this as an interim measure for now, so at least we
> have the embedded display working. IIUC to move that to the display controller
> driver core I should also convert/rework the mtk_dpi and mtk_hdmi drivers.
> This
> is used for the external display on my device but to fully support this I'll
> also need to rework the bridge chain logic to handle the
> multi-sink/multi-source
> use case. This is something I plan to work on but I suspect won't be easy and
> will trigger lots of discussions, and, of course, some time.
>
> So, if is fine I won't move this for now.
That's totally fine with me, I just wanted to make sure you were aware
that more work was needed :-)
Thanks for all your efforts !
> > I forgot to mention that the drm_encoder should also move out of the
> > bridge driver to the display controller driver.
> >
> >>> Signed-off-by: Enric Balletbo i Serra
> >>> ---
> >>>
> >>> Changes in v2: None
> >>>
> >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +-
> >>> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> >>> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> >>> index 44718fa3d1ca..2f8876c32864 100644
> >>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> >>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> >>> @@ -17,6 +17,7 @@
> >>>
> >>> #include
> >>> #include
> >>> +#include
> >>> #include
> >>> #include
> >>> #include
> >>> @@ -184,6 +185,7 @@ struct mtk_dsi {
> >>> struct drm_bridge bridge;
> >>> struct drm_bridge *panel_bridge;
> >>> struct drm_bridge *next_bridge;
> >>> + struct drm_connector *connector;
> >>> struct phy *phy;
> >>>
> >>> void __iomem *regs;
> >>> @@ -983,10 +985,19 @@ static int mtk_dsi_encoder_init(struct drm_device
> >>> *drm, struct mtk_dsi *dsi)
> >>>*/
> >>> dsi->encoder.possible_crtcs = 1;
> >>>
> >>> - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
> >>> + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> >>> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >>> if (ret)
> >>> goto err_cleanup_encoder;
> >>>
> >>> + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
> >>> + if (IS_ERR(dsi->connector)) {
> >>> + DRM_ERROR("Unable to create bridge connector\n");
> >>> + ret = PTR_ERR(dsi->connector);
> >>> + goto err_cleanup_encoder;
> >>> + }
> >>> + drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
> >>> +
> >>> return 0;
> >>>
> >>> err_cleanup_encoder:
> >>> @@ -1144,6 +1155,7 @@ static int mtk_dsi_probe(struct platform_device
> >>> *pdev)
> >>>
> >>> dsi->bridge.funcs = &mtk_dsi_bridge_funcs;
> >>> dsi->bridge.of_node = dev->of_node;
> >>> + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> >>
> >> I think this line belongs to the patch that adds drm_bridge support to
> >> this driver.
> >>
> >>>
> >>> drm_bridge_add(&dsi->bridge);
> >>>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: panel: Return always an error pointer in drm_panel_bridge_add()
Hi Enric,
Thank you for the patch.
On Thu, Apr 16, 2020 at 11:06:54PM +0200, Enric Balletbo i Serra wrote:
> Since commit 89958b7cd955 ("drm/bridge: panel: Infer connector type from
> panel by default"), drm_panel_bridge_add() and their variants can return
> NULL and an error pointer. This is fine but none of the actual users of
> the API are checking for the NULL value. Instead of change all the
> users, seems reasonable to return an error pointer instead. So change
> the returned value for those functions when the connector type is unknown.
>
> Suggested-by: Laurent Pinchart
> Signed-off-by: Enric Balletbo i Serra
Reviewed-by: Laurent Pinchart
> ---
>
> drivers/gpu/drm/bridge/panel.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 8461ee8304ba..7a3df0f319f3 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -166,7 +166,7 @@ static const struct drm_bridge_funcs
> panel_bridge_bridge_funcs = {
> *
> * The connector type is set to @panel->connector_type, which must be set to
> a
> * known type. Calling this function with a panel whose connector type is
> - * DRM_MODE_CONNECTOR_Unknown will return NULL.
> + * DRM_MODE_CONNECTOR_Unknown will return ERR_PTR(-EINVAL).
> *
> * See devm_drm_panel_bridge_add() for an automatically managed version of
> this
> * function.
> @@ -174,7 +174,7 @@ static const struct drm_bridge_funcs
> panel_bridge_bridge_funcs = {
> struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel)
> {
> if (WARN_ON(panel->connector_type == DRM_MODE_CONNECTOR_Unknown))
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> return drm_panel_bridge_add_typed(panel, panel->connector_type);
> }
> @@ -265,7 +265,7 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct
> device *dev,
>struct drm_panel *panel)
> {
> if (WARN_ON(panel->connector_type == DRM_MODE_CONNECTOR_Unknown))
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> return devm_drm_panel_bridge_add_typed(dev, panel,
> panel->connector_type);
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] dt-bindings: drm/bridge: ti-sn65dsi86: Add hpd-gpios to the bindings
Hi, On Wed, Apr 15, 2020 at 5:54 PM Laurent Pinchart wrote: > > Hi Doug, > > On Wed, Apr 15, 2020 at 04:49:00PM -0700, Doug Anderson wrote: > > On Wed, Apr 15, 2020 at 1:33 PM Laurent Pinchart wrote: > > > On Wed, Apr 15, 2020 at 12:53:02PM -0700, Stephen Boyd wrote: > > > > Quoting Douglas Anderson (2020-04-15 08:48:40) > > > > > Allow people to specify to use a GPIO for hot-plug-detect. Add an > > > > > example. > > > > > > > > > > NOTE: The current patch adding support for hpd-gpios to the Linux > > > > > driver for hpd-gpios only adds enough support to the driver so that > > > > > the bridge can use one of its own GPIOs. The bindings, however, are > > > > > written generically. > > > > > > > > > > Signed-off-by: Douglas Anderson > > > > > --- > > > > > > > > > > .../bindings/display/bridge/ti,sn65dsi86.yaml | 10 > > > > > +- > > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > > > > > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > > > > > index 8cacc6db33a9..554bfd003000 100644 > > > > > --- > > > > > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > > > > > +++ > > > > > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml > > > > > @@ -60,6 +60,10 @@ properties: > > > > > const: 1 > > > > > description: See ../../pwm/pwm.yaml for description of the cell > > > > > formats. > > > > > > > > > > + hpd-gpios: > > > > > +maxItems: 1 > > > > > +description: If present use the given GPIO for hot-plug-detect. > > > > > > > > Shouldn't this go in the panel node? And the panel driver should get the > > > > gpio and poll it after powering up the panel? Presumably that's why we > > > > have the no-hpd property in the simple panel binding vs. putting it here > > > > in the bridge. > > > > > > Same question really, I think this belongs to the panel (or connector) > > > node indeed. > > > > Hrm. > > > > To me "no-hpd" feels OK in the panel because the lack of a connection > > is somewhat symmetric. Thus it's OK to say either "HPD isn't hooked > > up to the panel in this system" or "HPD isn't hooked up to the bridge > > in this system" and both express the same thing (AKA that there is no > > HPD connection between the bridge and the panel). In the case of > > "no-hpd" it's more convenient to express it on the panel side because > > the panel driver is the one whose behavior has to change if HPD isn't > > hooked up. The panel datasheet is the one that says how long of a > > delay we need if HPD isn't hooked up. > > > > ...but when you're talking about where the bridge driver should look > > to find the HPD signal that it needs, that really feels like it should > > be described as part of the bridge. Specifically imagine we were > > using our bridge for DP, not for eDP. In that case simple-panel > > wouldn't be involved because we could get any type of display plugged > > in. Thus it couldn't go in the panel node. Here it feels clearer > > that hpd-gpio needs to be a property of the bridge driver. > > If you were using it for DP, you would need a DT node for the DP > connector (with bindings to be added to > Documentation/devicetree/bindings/display/connector/, similar to the > ones we already have for other connectors). That DT node should > reference the HPD pin GPIO. The bridge driver for the connector > (drivers/gpu/drm/bridge/display-connector.c) would then handle HPD. The > good news is that it already does :-) I'm having a really hard time following, but maybe it's because my knowledge of the DRM terminology is feeble at best? Looking at it from a DRM driver perspective and thus looking in 'drm/bridge/ti-sn65dsi86.c' I see that the driver for this bridge chip effectively is both the bridge and the connector. The struct encapsulating the driver data has both: struct drm_bridge bridge; struct drm_connector connector; ...in ti_sn_bridge_attach() the code calls drm_connector_init() for the connector. Looking at it from a device tree point of view, there is no separate node representing an eDP connector for one mainline user of 'ti,sn65dsi86' (sdm845-cheza). The device tree node has one input port (from "dsi0_out") and one output port (to "panel_in_edp"). There is no separate connector node as I can see with "hdmi-connector". ...and, as far as I can tell, sdm845-cheza is using the bindings as documented. The bindings say that the 'ti,sn65dsi86' node needs two ports: - Video port 0 for DSI input - Video port 1 for eDP output So, though I'm probably terribly confused, I would tentatively say that: - I'd guess that the 'ti,sn65dsi86' bindings were written / approved back before people were encouraged to model the connector as a separate node. - In the case of 'ti,sn65dsi86' the current dts node is both the node for the bridge and the connector. - If we want to try to deprec
[drm-intel:topic/core-for-CI 2/20] kernel/locking/lockdep.c:2744:49: sparse: sparse: cast truncates bits from constant value (a0000 becomes 0)
tree: git://anongit.freedesktop.org/drm-intel topic/core-for-CI
head: d0435a9b45070b945578c093dcd363b6b73a502c
commit: cbc1ad45be43de36150fd98dae644fc89a69a5a0 [2/20] lockdep: Up
MAX_LOCKDEP_CHAINS
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-191-gc51a0382-dirty
git checkout cbc1ad45be43de36150fd98dae644fc89a69a5a0
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
sparse warnings: (new ones prefixed by >>)
>> kernel/locking/lockdep.c:2744:49: sparse: sparse: cast truncates bits from
>> constant value (a becomes 0)
>> kernel/locking/lockdep.c:2744:49: sparse: sparse: cast truncates bits from
>> constant value (a becomes 0)
vim +2744 kernel/locking/lockdep.c
810507fe6fd5ff Waiman Long 2020-02-06 2736
810507fe6fd5ff Waiman Long 2020-02-06 2737 static inline void
init_chain_block(int offset, int next, int bucket, int size)
810507fe6fd5ff Waiman Long 2020-02-06 2738 {
810507fe6fd5ff Waiman Long 2020-02-06 2739 chain_hlocks[offset] = (next >>
16) | CHAIN_BLK_FLAG;
810507fe6fd5ff Waiman Long 2020-02-06 2740 chain_hlocks[offset + 1] =
(u16)next;
810507fe6fd5ff Waiman Long 2020-02-06 2741
810507fe6fd5ff Waiman Long 2020-02-06 2742 if (size && !bucket) {
810507fe6fd5ff Waiman Long 2020-02-06 2743 chain_hlocks[offset +
2] = size >> 16;
810507fe6fd5ff Waiman Long 2020-02-06 @2744 chain_hlocks[offset +
3] = (u16)size;
810507fe6fd5ff Waiman Long 2020-02-06 2745 }
810507fe6fd5ff Waiman Long 2020-02-06 2746 }
810507fe6fd5ff Waiman Long 2020-02-06 2747
:: The code at line 2744 was first introduced by commit
:: 810507fe6fd5ff3de429121adff49523fabb643a locking/lockdep: Reuse freed
chain_hlocks entries
:: TO: Waiman Long
:: CC: Ingo Molnar
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: panel: Document some missing compatible strings
From: Thierry Reding Add missing compatible strings for the Panasonic and Chunghwa panels found on NVIDIA Dalmore and Cardhu boards. Signed-off-by: Thierry Reding --- .../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++ .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml index b2e8742fd6af..88ac75333a5e 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml @@ -29,6 +29,8 @@ properties: # compatible must be listed in alphabetical order, ordered by compatible. # The description in the comment is mandatory for each compatible. +# Panasonic 10" WUXGA TFT LCD panel + - panasonic,vvx10f004b00 # Panasonic 10" WUXGA TFT LCD panel - panasonic,vvx10f034n00 diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 393ffc6acbba..6c21650664e2 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -82,6 +82,8 @@ properties: # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel - chunghwa,claa101wa01a # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel + - chunghwa,claa101wb01 +# Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel - chunghwa,claa101wb03 # DataImage, Inc. 7" WVGA (800x480) TFT LCD panel with 24-bit parallel interface. - dataimage,scf0700c48ggu18 -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
