Re: [PATCH v3 12/12] drm/bridge: tc358768: Attempt to fix DSI horizontal timings

2023-09-04 Thread Marcel Ziswiler
Hi Tomi

Looks good. Thanks! Tested both on Verdin AM62 as well as on Verdin iMX8M Mini.

Just a minor nit-pick in your code comment further below.

On Tue, 2023-08-22 at 19:19 +0300, Tomi Valkeinen wrote:
> The DSI horizontal timing calculations done by the driver seem to often
> lead to underflows or overflows, depending on the videomode.
> 
> There are two main things the current driver doesn't seem to get right:
> DSI HSW and HFP, and VSDly. However, even following Toshiba's
> documentation it seems we don't always get a working display.
> 
> This patch attempts to fix the horizontal timings for DSI event mode, and
> on a system with a DSI->HDMI encoder, a lot of standard HDMI modes now
> seem to work. The work relies on Toshiba's documentation, but also quite
> a bit on empirical testing.
> 
> This also adds timing related debug prints to make it easier to improve
> on this later.
> 
> The DSI pulse mode has only been tested with a fixed-resolution panel,
> which limits the testing of different modes on DSI pulse mode. However,
> as the VSDly calculation also affects pulse mode, so this might cause a
> regression.
> 
> Reviewed-by: Peter Ujfalusi 
> Signed-off-by: Tomi Valkeinen 

For the whole series:

Tested-by: Marcel Ziswiler 

> ---
>  drivers/gpu/drm/bridge/tc358768.c | 211 
> +-
>  1 file changed, 183 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358768.c 
> b/drivers/gpu/drm/bridge/tc358768.c
> index f41bf56b7d6b..b465e0a31d09 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -157,6 +158,7 @@ struct tc358768_priv {
> u32 frs;/* PLL Freqency range for HSCK (post divider) */
>  
> u32 dsiclk; /* pll_clk / 2 */
> +   u32 pclk;   /* incoming pclk rate */
>  };
>  
>  static inline struct tc358768_priv *dsi_host_to_tc358768(struct mipi_dsi_host
> @@ -380,6 +382,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv,
> priv->prd = best_prd;
> priv->frs = frs;
> priv->dsiclk = best_pll / 2;
> +   priv->pclk = mode->clock * 1000;
>  
> return 0;
>  }
> @@ -638,6 +641,28 @@ static u32 tc358768_ps_to_ns(u32 ps)
> return ps / 1000;
>  }
>  
> +static u32 tc358768_dpi_to_ns(u32 val, u32 pclk)
> +{
> +   return (u32)div_u64((u64)val * NANO, pclk);
> +}
> +
> +/* Convert value in DPI pixel clock units to DSI byte count */
> +static u32 tc358768_dpi_to_dsi_bytes(struct tc358768_priv *priv, u32 val)
> +{
> +   u64 m = (u64)val * priv->dsiclk / 4 * priv->dsi_lanes;
> +   u64 n = priv->pclk;
> +
> +   return (u32)div_u64(m + n - 1, n);
> +}
> +
> +static u32 tc358768_dsi_bytes_to_ns(struct tc358768_priv *priv, u32 val)
> +{
> +   u64 m = (u64)val * NANO;
> +   u64 n = priv->dsiclk / 4 * priv->dsi_lanes;
> +
> +   return (u32)div_u64(m, n);
> +}
> +
>  static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>  {
> struct tc358768_priv *priv = bridge_to_tc358768(bridge);
> @@ -647,11 +672,19 @@ static void tc358768_bridge_pre_enable(struct 
> drm_bridge *bridge)
> s32 raw_val;
> const struct drm_display_mode *mode;
> u32 hsbyteclk_ps, dsiclk_ps, ui_ps;
> -   u32 dsiclk, hsbyteclk, video_start;
> -   const u32 internal_delay = 40;
> +   u32 dsiclk, hsbyteclk;
> int ret, i;
> struct videomode vm;
> struct device *dev = priv->dev;
> +   /* In pixelclock units */
> +   u32 dpi_htot, dpi_data_start;
> +   /* In byte units */
> +   u32 dsi_dpi_htot, dsi_dpi_data_start;
> +   u32 dsi_hsw, dsi_hbp, dsi_hact, dsi_hfp;
> +   const u32 dsi_hss = 4; /* HSS is a short packet (4 bytes) */
> +   /* In hsbyteclk units */
> +   u32 dsi_vsdly;
> +   const u32 internal_dly = 40;
>  
> if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> dev_warn_once(dev, "Non-continuous mode unimplemented, 
> falling back to continuous\n");
> @@ -686,27 +719,23 @@ static void tc358768_bridge_pre_enable(struct 
> drm_bridge *bridge)
> case MIPI_DSI_FMT_RGB888:
> val |= (0x3 << 4);
> hact = vm.hactive * 3;
> -   video_start = (vm.hsync_len + vm.hback_porch) * 3;
> data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> break;
> case MIPI_DSI_FMT_RGB666:
>    

Re: [PATCH 3/4] drm/bridge: lt8912b: Manually disable HPD only if it was enabled

2023-09-04 Thread Marcel Ziswiler
Hi Tomi

Looks good. Thanks! Tested both on Verdin AM62 as well as on Verdin iMX8M Mini.

Just a minor nit-pick in your commit message.

On Fri, 2023-08-04 at 13:48 +0300, Tomi Valkeinen wrote:
> lt8912b only calls drm_bridge_hpd_enable() if it creates a connector and
> the next bridge has DRM_BRIDGE_OP_HPD set. However, when calling
> drm_bridge_hpd_disable() it misses checking if a connector was created,
> calling drm_bridge_hpd_disable() even if HPD was nenver enabled. I don't

was never enabled

> see any issues causing by this wrong call, though.

any issues caused by this wrong call

> Add the check to avoid wrongly calling drm_bridge_hpd_disable().
> 
> Fixes: 3b0a01a6a522 ("drm/bridge: lt8912b: Add hot plug detection")
> Signed-off-by: Tomi Valkeinen 

For the whole series:

Tested-by: Marcel Ziswiler 

> ---
>  drivers/gpu/drm/bridge/lontium-lt8912b.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c 
> b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 2d752e083433..9ee639e75a1c 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -587,7 +587,7 @@ static void lt8912_bridge_detach(struct drm_bridge 
> *bridge)
>  
> lt8912_hard_power_off(lt);
>  
> -   if (lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
> +   if (lt->connector.dev && lt->hdmi_port->ops & DRM_BRIDGE_OP_HPD)
> drm_bridge_hpd_disable(lt->hdmi_port);
>  }

Cheers

Marcel


Re: [PATCH v14 0/6] drm/imx: Introduce i.MX8qm/qxp DPU DRM

2023-08-22 Thread Marcel Ziswiler
Hi Liu Ying

On Tue, 2023-08-22 at 05:36 +, Ying Liu wrote:
> Hi,
> 
> > On Friday, January 6, 2023 1:50 PM Ying Liu wrote:
> > 
> > Hi,
> > 
> > 
> > This is the v14 series to introduce i.MX8qm/qxp Display Processing Unit(DPU)
> > DRM support.

[snip]

> This patch series has been submitted for a quite long period of time.
> 
> Anything I can do to have it landed ?

Well, may it be tested? Are all the missing pieces there now?

Thanks!

> Regards,
> Liu Ying

Cheers

Marcel


Re: [PATCH v13 5/6] drm/imx: Introduce i.MX8qm/qxp DPU DRM

2023-01-04 Thread Marcel Ziswiler
Hi Liu

Thank you very much!

On Wed, 2022-10-19 at 10:02 +0800, Liu Ying wrote:
> This patch introduces i.MX8qm/qxp Display Processing Unit(DPU) DRM support.
> 
> DPU is comprised of two main components that include a blit engine for
> 2D graphics accelerations(with composition support) and a display
> controller for display output processing, as well as a command sequencer.
> Outside of DPU, optional prefetch engines, a.k.a, Prefetch Resolve
> Gasket(PRG) and Display Prefetch Resolve(DPR), can fetch data from memory
> prior to some DPU fetchunits of blit engine and display controller.  The
> prefetch engines support reading linear formats and resolving Vivante GPU
> tile formats.
> 
> This patch adds kernel modesetting support for the display controller part.
> The driver supports two CRTCs per display controller, planes backed by
> four fetchunits(decode0/1, fetchlayer, fetchwarp), fetchunit allocation
> logic for the two CRTCs, prefetch engines(with tile resolving supported),
> plane upscaling/deinterlacing/yuv2rgb CSC/alpha blending and CRTC gamma
> correction.  The registers of the controller is accessed without command
> sequencer involved, instead just by using CPU.
> 
> Reference manual can be found at:
> https://www.nxp.com/webapp/Download?colCode=IMX8DQXPRM
> 
> Reviewed-by: Laurentiu Palcu 
> Signed-off-by: Liu Ying 
> ---
> v12->v13:
> * Drop 'drm->irq_enabled = true;' to fix a potential build break
>   reported by 'kernel test robot '. drm->irq_enabled
>   should not be used by imx-dpu drm as it is only used by legacy
>   drivers with userspace modesetting.
> 
> v11->v12:
> * Rebase upon v6.1-rc1.
> * Minor update on Kconfigs, struct names and macro names due to the rebase.
> 
> v10->v11:
> * Rebase upon v6.0-rc1.
> * Include drm_blend.h and drm_framebuffer.h in dpu-kms.c and dpu-plane.c
>   to fix build errors due to the rebase.
> * Fix a checkpatch warning for dpu-crtc.c.
> * Properly use dev_err_probe() to return it's return value directly where
>   possible.
> 
> v9->v10:
> * Make 'checkpatch.pl --strict' happier.
> * Add Laurentiu's R-b tag.
> 
> v8->v9:
> * Use drm_atomic_get_new_plane_state() in dpu_plane_atomic_update(). 
> (Laurentiu)
> * Drop getting DPU DT alias ID, as it is unused.
> * Get the DPR interrupt(dpr_wrap) by name.
> 
> v7->v8:
> * Update dpu_plane_atomic_check() and dpu_plane_atomic_update(), due to DRM
>   plane helper functions API change(atomic_check and atomic_update) from DRM
>   atomic core.  Also, rename plane->state variables and relevant DPU plane
>   state variables in those two functions to reflect they are new states, like
>   the patch 'drm: Rename plane->state variables in atomic update and disable'
>   recently landed in drm-misc-next.
> * Replace drm_gem_fb_prepare_fb() with drm_gem_plane_helper_prepare_fb(),
>   due to DRM core API change.
> * Use 256byte DPR burst length for GPU standard tile and 128byte DPR burst
>   length for 32bpp GPU super tile to align with the latest version of internal
>   HW documention.
> 
> v6->v7:
> * Fix return value of dpu_get_irqs() if platform_get_irq() fails. (Laurentiu)
> * Use the function array dpu_irq_handler[] to store individual DPU irq 
> handlers.
>   (Laurentiu)
> * Call get/put() hooks directly to get/put DPU fetchunits for DPU plane 
> groups.
>   (Laurentiu)
> * Shorten the names of individual DPU irq handlers by using DPU unit abbrev
>   names to make writing dpu_irq_handler[] easier.
> 
> v5->v6:
> * Do not use macros where possible. (Laurentiu)
> * Break dpu_plane_atomic_check() into some smaller functions. (Laurentiu)
> * Address some minor comments from Laurentiu.
> * Add dpu_crtc_err() helper marco to tell dmesg which CRTC generates error.
> * Drop calling dev_set_drvdata() from dpu_drm_bind/unbind() as it is done
>   in dpu_drm_probe().
> * Some trivial tweaks.
> 
> v4->v5:
> * Rebase up onto the latest drm-misc-next branch and remove the hook to
>   drm_atomic_helper_legacy_gamma_set(), because it was dropped by the newly
>   landed commit 'drm: automatic legacy gamma support'.
> * Remove a redundant blank line from dpu_plane_atomic_update().
> 
> v3->v4:
> * No change.
> 
> v2->v3:
> * Fix build warnings Reported-by: kernel test robot .
> * Drop build dependency on IMX_SCU, as dummy SCU functions have been added in
>   header files by the patch 'firmware: imx: add dummy functions' which has
>   landed in linux-next/master branch.
> 
> v1->v2:
> * Add compatible for i.MX8qm DPU, as this is tested with i.MX8qm LVDS 
> displays.
>   (Laurentiu)
> * Fix PRG burst size and stride. (Laurentiu)
> * Put 'ports' OF node to fix the bail-out logic in dpu_drm_probe(). 
> (Laurentiu)
> 
>  drivers/gpu/drm/imx/Kconfig   |    1 +
>  drivers/gpu/drm/imx/Makefile  |    1 +
>  drivers/gpu/drm/imx/dpu/Kconfig   |    9 +
>  drivers/gpu/drm/imx/dpu/Makefile  |   10 +
>  drivers/gpu/drm/imx/dpu/dpu-constframe.c  |  171 
>  drivers/gpu/drm/imx/dpu/dpu-core.c    | 1044 

Re: [PATCH] drm: bridge: samsung-dsim: Add i.MX8M Plus support

2022-11-01 Thread Marcel Ziswiler
Hi Marek

On Sun, 2022-10-16 at 00:07 +0200, Marek Vasut wrote:
> Add extras to support i.MX8M Plus. The main change is the removal of
> HS/VS/DE signal inversion in the LCDIFv3-DSIM glue logic, otherwise
> the implementation of this IP in i.MX8M Plus is very much compatible
> with the i.MX8M Mini/Nano one.
> 
> Signed-off-by: Marek Vasut 

Reviewed-by: Marcel Ziswiler 

I also tried to test it on Verdin iMX8M Plus but so far I was unable to make it 
work. Either I am still missing
some other pieces or I got the imx8mp.dtsi parts wrong. Do you have them 
available somewhere by any chance?

Thanks!

Cheers

Marcel

> ---
> Cc: Adam Ford 
> Cc: Andrzej Hajda 
> Cc: Frieder Schrempf 
> Cc: Inki Dae 
> Cc: Jagan Teki 
> Cc: Kyungmin Park 
> Cc: Laurent Pinchart 
> Cc: Marek Szyprowski 
> Cc: Matteo Lisi 
> Cc: Michael Nazzareno Trimarchi 
> Cc: NXP Linux Team 
> Cc: Robert Foss 
> Cc: Seung-Woo Kim 
> Cc: Tim Harvey 
> Cc: Tommaso Merciai 
> Cc: linux-amarula 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 55 +--
>  include/drm/bridge/samsung-dsim.h |  1 +
>  2 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 012d8b6463ad6..cf40fd8813ff2 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -465,6 +465,7 @@ samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
> [SAMSUNG_DSIM_TYPE_EXYNOS5422] = &exynos5422_dsi_driver_data,
> [SAMSUNG_DSIM_TYPE_EXYNOS5433] = &exynos5433_dsi_driver_data,
> [SAMSUNG_DSIM_TYPE_IMX8MM] = &imx8mm_dsi_driver_data,
> +   [SAMSUNG_DSIM_TYPE_IMX8MP] = &imx8mm_dsi_driver_data,
>  };
>  
>  static inline struct samsung_dsim *host_to_dsi(struct mipi_dsi_host *h)
> @@ -1404,18 +1405,26 @@ static int samsung_dsim_atomic_check(struct 
> drm_bridge *bridge,
> struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
>  
> +   /*
> +    * The i.MX8M Mini/Nano glue logic between LCDIF and DSIM
> +    * inverts HS/VS/DE sync signals polarity, therefore, while
> +    * i.MX 8M Mini Applications Processor Reference Manual Rev. 3, 
> 11/2020
> +    * 13.6.3.5.2 RGB interface
> +    * i.MX 8M Nano Applications Processor Reference Manual Rev. 2, 
> 07/2022
> +    * 13.6.2.7.2 RGB interface
> +    * both claim "Vsync, Hsync, and VDEN are active high signals.", the
> +    * LCDIF must generate inverted HS/VS/DE signals, i.e. active LOW.
> +    *
> +    * The i.MX8M Plus glue logic between LCDIFv3 and DSIM does not
> +    * implement the same behavior, therefore LCDIFv3 must generate
> +    * HS/VS/DE signals active HIGH.
> +    */
> if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) {
> -   /**
> -    * FIXME:
> -    * At least LCDIF + DSIM needs active low sync,
> -    * but i.MX 8M Mini Applications Processor Reference Manual,
> -    * Rev. 3, 11/2020 says
> -    *
> -    * 13.6.3.5.2 RGB interface
> -    * Vsync, Hsync, and VDEN are active high signals.
> -    */
> adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | 
> DRM_MODE_FLAG_NVSYNC);
> adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | 
> DRM_MODE_FLAG_PVSYNC);
> +   } else if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MP) {
> +   adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | 
> DRM_MODE_FLAG_NVSYNC);
> +   adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | 
> DRM_MODE_FLAG_PVSYNC);
> }
>  
> return 0;
> @@ -1448,7 +1457,8 @@ static int samsung_dsim_attach(struct drm_bridge 
> *bridge,
>  * Passing NULL to the previous bridge makes Exynos DSI drivers
>  * work which is exactly done before.
>  */
> -   if (!(dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM))
> +   if (dsi->plat_data->hw_type != SAMSUNG_DSIM_TYPE_IMX8MM &&
> +   dsi->plat_data->hw_type != SAMSUNG_DSIM_TYPE_IMX8MP)
> previous = NULL;
>  
> return drm_bridge_attach(bridge->encoder, dsi->out_bridge, previous,
> @@ -1649,7 +1659,11 @@ static const struct samsung_dsim_host_ops 
> samsung_dsim_generic_host_ops = {
> .unregister_host = samsung_dsim_unregister_host,

Re: [PATCH v7 00/10] drm: bridge: Add Samsung MIPI DSIM bridge

2022-10-19 Thread Marcel Ziswiler
Hi Jagan

On Wed, 2022-10-05 at 20:42 +0530, Jagan Teki wrote:
> This series supports common bridge support for Samsung MIPI DSIM
> which is used in Exynos and i.MX8MM SoC's.
> 
> The final bridge supports both the Exynos and i.MX8MM DSI devices.
> 
> Changes for v7:
> * fix the drm bridge attach chain for exynos drm dsi driver
> * fix the hw_type checking logic
> 
> Changes for v6:
> * handle previous bridge for exynos dsi while attaching bridge 
> 
> Changes for v5:
> * bridge changes to support multi-arch
> * updated and clear commit messages
> * add hw_type via plat data
> * removed unneeded quirk
> * rebased on linux-next
> 
> Changes for v4:
> * include Inki Dae in MAINTAINERS
> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build
> * update init handling to ensure host init done on first cmd transfer
> 
> Changes for v3:
> * fix the mult-arch build
> * fix dsi host init
> * updated commit messages
> 
> Changes for v2:
> * fix bridge handling
> * fix dsi host init
> * correct the commit messages
> 
> Patch 0001: Samsung DSIM bridge
> 
> Patch 0002: PHY optional
> 
> Patch 0003: OF-graph or Child node lookup
> 
> Patch 0004: DSI host initialization 
> 
> Patch 0005: atomic check
> 
> Patch 0006: PMS_P offset via plat data
> 
> Patch 0007: atomic_get_input_bus_fmts
> 
> Patch 0008: input_bus_flags
> 
> Patch 0009: document fsl,imx8mm-mipi-dsim
> 
> Patch 0010: add i.MX8MM DSIM support
> 
> Tested in Engicam i.Core MX8M Mini SoM.

Thanks for this work!

This also works great on Verdin iMX8M Mini together with the SN65DSI84-based 
Verdin DSI to LVDS Adapter.

Tested-by: Marcel Ziswiler 

> Repo:
> https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v7
> 
> Any inputs?
> Jagan.
> 
> Jagan Teki (10):
>   drm: bridge: Add Samsung DSIM bridge driver
>   drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices
>   drm: bridge: samsung-dsim: Mark PHY as optional
>   drm: bridge: samsung-dsim: Handle proper DSI host initialization
>   drm: bridge: samsung-dsim: Add atomic_check
>   drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset
>   drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
>   drm: bridge: samsung-dsim: Add input_bus_flags
>   dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support
>   drm: bridge: samsung-dsim: Add i.MX8MM support
> 
>  .../bindings/display/exynos/exynos_dsim.txt   |    1 +
>  MAINTAINERS   |    9 +
>  drivers/gpu/drm/bridge/Kconfig    |   12 +
>  drivers/gpu/drm/bridge/Makefile   |    1 +
>  drivers/gpu/drm/bridge/samsung-dsim.c | 1856 +
>  drivers/gpu/drm/exynos/Kconfig    |    1 +
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c   | 1766 +---
>  include/drm/bridge/samsung-dsim.h |  115 +
>  8 files changed, 2108 insertions(+), 1653 deletions(-)
>  create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
>  create mode 100644 include/drm/bridge/samsung-dsim.h

Cheers

Marcel


Re: [PATCH] drm/bridge: dw-hdmi: use safe format when first in bridge chain

2022-01-17 Thread Marcel Ziswiler
Hi Neil

Sorry, just some trivial spelling fixes.

On Mon, 2022-01-17 at 15:17 +0100, Neil Armstrong wrote:
> When the dw-hdmi bridge is in first place of the bridge chain, this
> means there is now way

no way

> to select an input format of the dw-hdmi HW
> component.
> 
> Since introduction of display-connector, negociation

:%s/negociation/negotiation/g

> was broken since
> the dw-hdmi negociation code only worked when the dw-hdmi bridge was
> in last position of the bridge chain or behind another bridge also
> supporting input & output format negociation.
> 
> Commit 0656d1285b79 ("drm/bridge: display-connector: implement bus fmts 
> callbacks")
> was introduced to make negociation work again by making display-connector
> act as a pass-through concerning input & output format negociation.
> 
> But in the case were

where

> the dw-hdmi was single in the bridge chain, for
> example on Renesas SoCs, with the disply-connector

display-connector

> bridge the dw-hdmi
> is no more single, breaking output format.
> 
> Reported-by: Biju Das 
> Bisected-by: Kieran Bingham 
> Tested-by: Kieran Bingham 
> Fixes: 0656d1285b79 ("drm/bridge: display-connector: implement bus fmts 
> callbacks").
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 54d8fdad395f..9f2e1cac0ae2 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2551,8 +2551,9 @@ static u32 
> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
> if (!output_fmts)
> return NULL;
>  
> -   /* If dw-hdmi is the only bridge, avoid negociating with ourselves */
> -   if (list_is_singular(&bridge->encoder->bridge_chain)) {
> +   /* If dw-hdmi is the first or only bridge, avoid negociating with 
> ourselves */
> +   if (list_is_singular(&bridge->encoder->bridge_chain) ||
> +   list_is_first(&bridge->chain_node, 
> &bridge->encoder->bridge_chain)) {
> *num_output_fmts = 1;
> output_fmts[0] = MEDIA_BUS_FMT_FIXED;
>  
> @@ -2673,6 +2674,10 @@ static u32 
> *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> if (!input_fmts)
> return NULL;
>  
> +   /* If dw-hdmi is the first bridge fall-back to safe output format */
> +   if (list_is_first(&bridge->chain_node, 
> &bridge->encoder->bridge_chain))
> +   output_fmt = MEDIA_BUS_FMT_FIXED;
> +
> switch (output_fmt) {
> /* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */
> case MEDIA_BUS_FMT_FIXED:

Cheers

Marcel


Re: [PATCH v1] drm: import DMA_BUF module namespace

2021-10-29 Thread Marcel Ziswiler
On Thu, 2021-10-28 at 20:45 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 27.10.21 um 22:54 schrieb Marcel Ziswiler:
> > Sali Thomas
> > 
> > On Wed, 2021-10-27 at 20:30 +0200, Thomas Zimmermann wrote:
> > > Hi,
> > > 
> > > thanks for the patch.
> > 
> > You are very welcome.
> > 
> > > Am 27.10.21 um 17:25 schrieb Marcel Ziswiler:
> > > > From: Marcel Ziswiler 
> > > > 
> > > > Today's -next fails building arm64 defconfig as follows:
> > > > 
> > > > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from
> > > >    namespace DMA_BUF, but does not import it.
> > > > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vmap from
> > > >    namespace DMA_BUF, but does not import it.
> > > > 
> > > > Reported-by: Linux Kernel Functional Testing 
> > > > Fixes: commit 4b2b5e142ff4 ("drm: Move GEM memory managers into 
> > > > modules")
> > > > Signed-off-by: Marcel Ziswiler 
> > > > 
> > > > ---
> > > > 
> > > >    drivers/gpu/drm/drm_gem_cma_helper.c | 2 ++
> > > >    1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
> > > > b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > index 6f7b3f8ec04d3..69f8564ad11cd 100644
> > > > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > @@ -23,6 +23,8 @@
> > > >    #include 
> > > >    #include 
> > > >    
> > > > +MODULE_IMPORT_NS(DMA_BUF);
> > > 
> > > Could this line be moved to the bottom of the file, where the other
> > > MODULE statements are?
> > 
> > Hehe, good question. I was actually asking myself the same but quickly 
> > looking at a few files and they all
> > had
> > it after their includes towards the top. Turns out that was rather short 
> > sighted...
> > 
> > Let me look more closely. Current -next has exactly 200 files with a 
> > MODULE_IMPORT_NS statement. Okay, some
> > of
> > which are documentation. Anyway, 132 of which do have it with their other 
> > MODULE macros towards the end as
> > you
> > suggest. 20 of which and mainly DRM stuff has it towards the top after them 
> > includes. Funny.
> > 
> > What does the documentation suggest?
> > 
> > Documentation/core-api/symbol-namespaces.rst
> > 
> > "It is advisable to add the MODULE_IMPORT_NS() statement close to other 
> > module
> > metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE(). Refer to 
> > section
> > 5. for a way to create missing import statements automatically."
> > 
> > There you go. Plus there is even some fancy automation (;-p).
> > 
> > So let me move it down there then.
> 
> Will you send out another revision of the patch?

I already did.

https://marc.info/?l=linux-arm-kernel&m=163537686316807

> > > In the fixed commit 4b2b5e142ff4, there's a similar change for
> > > drm_gem_shmem_helper.c. It uses dma-buf_vmap as well. Does that module
> > > require the same fix?
> > 
> > Likely. Let me just run ze automation and see what we get...
> > 
> > > Do you have any idea why I don't see these errors in my builds?
> > 
> > Well, I guess, there are various KCONFIG symbols influencing that whole 
> > story. How about e.g.
> > 
> > init/Kconfig:config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
> 
> Thanks for the hint.

Welcome.

> Best regards
> Thomas
> 
> > 
> > > Best regards
> > > Thomas
> > 
> > Cheers
> > 
> > Marcel
> > 
> > > > +
> > > >    /**
> > > >     * DOC: cma helpers
> > > >     *
> > > > 
> > > 
> > > -- 
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > (HRB 36809, AG Nürnberg)
> > > Geschäftsführer: Felix Imendörffer
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer


Re: [PATCH v1] drm: import DMA_BUF module namespace

2021-10-27 Thread Marcel Ziswiler
Sali Thomas

On Wed, 2021-10-27 at 20:30 +0200, Thomas Zimmermann wrote:
> Hi,
> 
> thanks for the patch.

You are very welcome.

> Am 27.10.21 um 17:25 schrieb Marcel Ziswiler:
> > From: Marcel Ziswiler 
> > 
> > Today's -next fails building arm64 defconfig as follows:
> > 
> > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from
> >   namespace DMA_BUF, but does not import it.
> > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vmap from
> >   namespace DMA_BUF, but does not import it.
> > 
> > Reported-by: Linux Kernel Functional Testing 
> > Fixes: commit 4b2b5e142ff4 ("drm: Move GEM memory managers into modules")
> > Signed-off-by: Marcel Ziswiler 
> > 
> > ---
> > 
> >   drivers/gpu/drm/drm_gem_cma_helper.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
> > b/drivers/gpu/drm/drm_gem_cma_helper.c
> > index 6f7b3f8ec04d3..69f8564ad11cd 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -23,6 +23,8 @@
> >   #include 
> >   #include 
> >   
> > +MODULE_IMPORT_NS(DMA_BUF);
> 
> Could this line be moved to the bottom of the file, where the other 
> MODULE statements are?

Hehe, good question. I was actually asking myself the same but quickly looking 
at a few files and they all had
it after their includes towards the top. Turns out that was rather short 
sighted...

Let me look more closely. Current -next has exactly 200 files with a 
MODULE_IMPORT_NS statement. Okay, some of
which are documentation. Anyway, 132 of which do have it with their other 
MODULE macros towards the end as you
suggest. 20 of which and mainly DRM stuff has it towards the top after them 
includes. Funny.

What does the documentation suggest?

Documentation/core-api/symbol-namespaces.rst

"It is advisable to add the MODULE_IMPORT_NS() statement close to other module
metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE(). Refer to section
5. for a way to create missing import statements automatically."

There you go. Plus there is even some fancy automation (;-p).

So let me move it down there then.

> In the fixed commit 4b2b5e142ff4, there's a similar change for 
> drm_gem_shmem_helper.c. It uses dma-buf_vmap as well. Does that module 
> require the same fix?

Likely. Let me just run ze automation and see what we get...

> Do you have any idea why I don't see these errors in my builds?

Well, I guess, there are various KCONFIG symbols influencing that whole story. 
How about e.g.

init/Kconfig:config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS

> Best regards
> Thomas

Cheers

Marcel

> > +
> >   /**
> >    * DOC: cma helpers
> >    *
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer

-- 
Best regards - Mit freundlichen Grüssen - Meilleures salutations

Marcel Ziswiler
Software Team Lead - Embedded Linux BSP

Toradex AG
Ebenaustrasse 10 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800


Re: ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from namespace DMA_BUF, but does not import it.

2021-10-27 Thread Marcel Ziswiler
On Wed, 2021-10-27 at 18:14 +0530, Naresh Kamboju wrote:
> Regression found on arm64 gcc-10 and gcc-11 built with defconfig
> Following build warnings / errors reported on linux next 20211027.
> 
> metadata:
>     git_describe: next-20211027
>     git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
>     git_short_log: ae5179317e79 (\"Add linux-next specific files for 
> 20211027\")
>     target_arch: arm64
>     toolchain: gcc-11
> 
> build error :
> --
> ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from
> namespace DMA_BUF, but does not import it.
> ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vmap from
> namespace DMA_BUF, but does not import it.
> make[2]: *** [/builds/linux/scripts/Makefile.modpost:134:
> modules-only.symvers] Error 1
> make[2]: *** Deleting file 'modules-only.symvers'
> make[2]: Target '__modpost' not remade because of errors.
> 
> Reported-by: Linux Kernel Functional Testing 
> 
> build link:
> ---
> https://builds.tuxbuild.com/205SAU159J0g6lSlRRS11o5hHyY/build.log
> 
> build config:
> -
> https://builds.tuxbuild.com/205SAU159J0g6lSlRRS11o5hHyY/config
> 
> # To install tuxmake on your system globally
> # sudo pip3 install -U tuxmake
> tuxmake --runtime podman --target-arch arm64 --toolchain gcc-11
> --kconfig defconfig
> 
> --
> Linaro LKFT
> https://lkft.linaro.org

I also just experienced this one and will send a fix shortly.


Re: [PATCH v6 00/14] Add some DRM bridge drivers support for i.MX8qm/qxp SoCs

2021-03-28 Thread Marcel Ziswiler
Hi Liu

On Tue, 2021-03-23 at 17:09 +0800, Liu Ying wrote:
> On Tue, 2021-03-23 at 01:03 +0000, Marcel Ziswiler wrote:
> > Hi Liu
> > 
> > Some further discrepancy with them binding examples:
> > 
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi:335.9-36: Warning (reg_format): 
> > /dpu@5618:reg: property has
> > invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi:508.9-35: Warning (reg_format): 
> > /syscon@56221000:reg: property has
> > invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi:601.9-34: Warning (reg_format): 
> > /phy@56228300:reg: property has
> > invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
> > arch/arm64/boot/dts/freescale/imx8qxp.dtsi:613.9-36: Warning (reg_format): 
> > /pixel-combiner@5602:reg:
> > property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 
> > 2)
> > 
> > And with that I am unable to bring it up:
> > 
> > [    1.714498] imx8qxp-ldb 562210001000.syscon:ldb: 
> > [drm:ldb_init_helper] *ERROR* failed to get regmap: -
> > 12
> > [    1.724441] imx8qxp-ldb: probe of 562210001000.syscon:ldb failed 
> > with error -12
> > [    1.734983] imx8qxp-pixel-combiner 56020001.pixel-combiner: 
> > invalid resource
> > [    1.742830] imx8qxp-pixel-combiner: probe of 
> > 56020001.pixel-combiner failed with error -22
> > [    1.754040] imx8qxp-display-pixel-link dc0-pixel-link0: 
> > [drm:imx8qxp_pixel_link_bridge_probe] *ERROR*
> > failed
> > to get pixel link node alias id: -19
> > [    1.769626] imx8qxp-pxl2dpi 562210001000.syscon:pxl2dpi: 
> > [drm:imx8qxp_pxl2dpi_bridge_probe] *ERROR*
> > failed to get regmap: -12
> > [    1.781397] imx8qxp-pxl2dpi: probe of 562210001000.syscon:pxl2dpi 
> > failed with error -12
> > [    1.840547] imx8qxp-lpcg-clk 5958.clock-controller: deferred probe 
> > timeout, ignoring dependency
> > [    1.840571] imx8qxp-lpcg-clk: probe of 5958.clock-controller failed 
> > with error -110
> > 
> > Any suggestions welcome. Thanks!
> 
> Please reference the patch set I shared in my last reply and see how it
> goes.  Thanks.

Thank you very much. After a little bit of fiddling I can confirm that this 
also works fine on a Toradex
Colibri iMX8X [1] with either a Capacitive Touch Display 10.1" LVDS which has a 
Logic Technologies LT170410-
2WHC [2] single-channel panel inside or a dual-channel LG LP156WF1 full HD 
panel.

During boot I noticed quite some clocking/power domain related messages:

[0.537965] gpt0_clk: failed to attached the power domain -2

[0.562372] dc1_disp0_clk: failed to attached the power domain -2
[0.562800] dc1_disp0_clk: failed to get clock parent -22
[0.562858] dc1_disp0_clk: failed to get clock rate -22

[0.563059] dc1_disp1_clk: failed to attached the power domain -2
[0.563463] dc1_disp1_clk: failed to get clock parent -22
[0.563514] dc1_disp1_clk: failed to get clock rate -22

[0.563773] dc1_pll0_clk: failed to attached the power domain -2
[0.564174] dc1_pll0_clk: failed to get clock rate -22

[0.564413] dc1_pll1_clk: failed to attached the power domain -2
[0.564838] dc1_pll1_clk: failed to get clock rate -22

[0.565099] dc1_bypass0_clk: failed to attached the power domain -2
[0.565516] dc1_bypass0_clk: failed to get clock rate -22

[0.565755] dc1_bypass1_clk: failed to attached the power domain -2
[0.566159] dc1_bypass1_clk: failed to get clock rate -22

[0.574493] lvds0_i2c0_clk: failed to attached the power domain -2
[0.574894] lvds0_i2c0_clk: failed to get clock rate -22

[0.575134] lvds0_i2c1_clk: failed to attached the power domain -2
[0.575526] lvds0_i2c1_clk: failed to get clock rate -22

[0.575785] lvds0_pwm0_clk: failed to attached the power domain -2
[0.576189] lvds0_pwm0_clk: failed to get clock rate -22

[0.576417] lvds1_i2c0_clk: failed to attached the power domain -2
[0.576854] lvds1_i2c0_clk: failed to get clock rate -22

[0.577129] lvds1_i2c1_clk: failed to attached the power domain -2
[0.577554] lvds1_i2c1_clk: failed to get clock rate -22

[0.577787] lvds1_pwm0_clk: failed to attached the power domain -2
[0.578198] lvds1_pwm0_clk: failed to get clock rate -22

[0.578464] mipi_csi0_core_clk: failed to attached the power domain -2

[0.579104] mipi_csi0_esc_clk: failed to attached the power domain -2

[0.579738] mipi_csi0_i2c0_clk: failed to attached the power domain -2

[0.580368] mipi_csi0_pwm0_clk: failed to attached the power domain -2

And the following repeats a couple dozens of times:

[4.391495] dc1_disp0_clk: fai

Re: [PATCH v6 00/14] Add some DRM bridge drivers support for i.MX8qm/qxp SoCs

2021-03-23 Thread Marcel Ziswiler
Hi Liu

I gave this a try however I believe I am still missing some piece as it throws 
the following during compilation
of the device tree:

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_irqsteer"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpu_lpcg"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpu_lpcg"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_disp_lpcg"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_disp_lpcg"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr1_channel1"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr1_channel2"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr1_channel3"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr2_channel1"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr2_channel2"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:333.18-439.7: ERROR 
(phandle_references): /dpu@5618: Reference
to non-existent node or label "dc0_dpr2_channel3"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:501.38-591.3: ERROR 
(phandle_references): /syscon@56221000:
Reference to non-existent node or label "mipi_lvds_0_di_mipi_lvds_regs_lpcg"

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:603.29-656.7: ERROR 
(phandle_references): /pixel-combiner@5602:
Reference to non-existent node or label "dc0_pixel_combiner_lpcg"

For now I just put all the examples from the various 
Documentation/devicetree/bindings/*/imx8qxp-*.yaml files
directly into arch/arm64/boot/dts/freescale/imx8qxp.dtsi. Maybe you do have the 
various device tree parts
available somewhere as well?

Any suggestions? Do you by any chance have a git tree available anywhere which 
includes all dependencies and
everything which one could try?

Thanks!

Cheers

Marcel

On Wed, 2021-03-17 at 11:42 +0800, Liu Ying wrote:
> Hi,
> 
> This is the v6 series to add some DRM bridge drivers support
> for i.MX8qm/qxp SoCs.
> 
> The bridges may chain one by one to form display pipes to support
> LVDS displays.  The relevant display controller is DPU embedded in
> i.MX8qm/qxp SoCs.
> 
> The DPU KMS driver can be found at:
> https://www.spinics.net/lists/arm-kernel/msg878542.html
> 
> This series supports the following display pipes:
> 1) i.MX8qxp:
> prefetch eng -> DPU -> pixel combiner -> pixel link ->
> pixel link to DPI(PXL2DPI) -> LVDS display bridge(LDB)
> 
> 2) i.MX8qm:
> prefetch eng -> DPU -> pixel combiner -> pixel link -> LVDS display 
> bridge(LDB)
> 
> 
> This series dropped the patch 'phy: Add LVDS configuration options', as
> suggested by Robert Foss, because it has already been sent with the following
> series to add Mixel combo PHY found in i.MX8qxp:
> https://www.spinics.net/lists/arm-kernel/msg879957.html
> 
> So, this version depends on that series.
> 
> 
> Patch 1/14 and 2/14 add bus formats used by pixel combiner.
> 
> Patch 7/14 adds dt-binding for Control and Status Registers module(a syscon
> used by PXL2DPI and LDB), which references the PXL2DPI and LDB schemas.
> 
> Patch 10/14 adds a helper for LDB bridge drivers.
> 
> Patch 3/14 ~ 6/14, 8/14, 9/14 and 11/14 ~ 13/14 add drm bridge drivers and
> dt-bindings support for the bridges.
> 
> Patch 14/14 updates MAINTAINERS.
> 
> 
> I've tested this series with a koe,tx26d202vm0bwa dual link LVDS panel and
> a LVDS to HDMI bridge(with a downstream drm bridge driver).
> 
> 
> Welcome comments, thanks.
> 
> v5->v6:
> * Fix data organizations in documentation(patch 2/14) for
>   MEDIA_BUS_FMT_RGB{666,888}_1X30-CPADLO. (Laurent)
> * Add Laurent's R-b tags on patch 1/14 and 2/14.
> * Drop 'select' schema from the CSR dt-binding documentation(patch 7/14). 
> (Rob)
> * Add Rob's R-b tag on patch 8/14.
> 
> v4->v5:
> * Drop the patch 'phy: Add LVDS configuration options'. (Robert)
> * Add Robert's R-b tags on patch 1/14, 2/14, 4/14 and 6/14.
> * Drop the 'PC_BUF_PARA_REG' register definition from the pixel combiner 
> bridge
>   driver(patch 4/14). (Robert)
> * Make a comment occupy a line in the pixel link bridge driver(patch 6/14).
>   (Robert)
> * Introduce a new patch(patch 7/14) to add dt-b

Re: [PATCH v6 01/14] media: uapi: Add some RGB bus formats for i.MX8qm/qxp pixel combiner

2021-03-23 Thread Marcel Ziswiler
On Wed, 2021-03-17 at 11:42 +0800, Liu Ying wrote:
> This patch adds RGB666_1X30_CPADLO, RGB888_1X30_CPADLO, RGB666_1X36_CPADLO
> and RGB888_1X36_CPADLO bus formats used by i.MX8qm/qxp pixel combiner.
> The RGB pixels with padding low per component are transmitted on a 30-bit
> input bus(10-bit per component) from a display controller or a 36-bit
> output bus(12-bit per component) to a pixel link.
> 
> Reviewed-by: Robert Foss 
> Reviewed-by: Laurent Pinchart 
> 
> Signed-off-by: Liu Ying 
> ---
> v5->v6:
> * Add Laurent's R-b tag.
> 
> v4->v5:
> * Add Robert's R-b tag.
> 
> v3->v4:
> * No change.
> 
> v2->v3:
> * No change.
> 
> v1->v2:
> * No change.
> 
>  include/uapi/linux/media-bus-format.h | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/media-bus-format.h 
> b/include/uapi/linux/media-bus-format.h
> index 0dfc11e..ec3323d 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -34,7 +34,7 @@
>  
>  #define MEDIA_BUS_FMT_FIXED0x0001
>  
> -/* RGB - next is   0x101e */
> +/* RGB - next is   0x1022 */
>  #define MEDIA_BUS_FMT_RGB444_1X12  0x1016
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE  0x1001
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE  0x1002
> @@ -59,9 +59,13 @@
>  #define MEDIA_BUS_FMT_RGB888_3X8_DELTA 0x101d
>  #define MEDIA_BUS_FMT_RGB888_1X7X4_SPWG0x1011
>  #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA   0x1012
> +#define MEDIA_BUS_FMT_RGB666_1X30_CPADLO   0x101e
> +#define MEDIA_BUS_FMT_RGB888_1X30_CPADLO   0x101f
>  #define MEDIA_BUS_FMT_ARGB_1X320x100d
>  #define MEDIA_BUS_FMT_RGB888_1X32_PADHI0x100f
>  #define MEDIA_BUS_FMT_RGB101010_1X30   0x1018
> +#define MEDIA_BUS_FMT_RGB666_1X36_CPADLO   0x1020
> +#define MEDIA_BUS_FMT_RGB888_1X36_CPADLO   0x1021
>  #define MEDIA_BUS_FMT_RGB121212_1X36   0x1019
>  #define MEDIA_BUS_FMT_RGB161616_1X48   0x101a

I haven't figured out what exactly the idea of this strange ordering of things 
is about? Could you enlighten
me?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 05/14] dt-bindings: display: bridge: Add i.MX8qm/qxp display pixel link binding

2021-03-23 Thread Marcel Ziswiler
On Wed, 2021-03-17 at 11:42 +0800, Liu Ying wrote:
> This patch adds bindings for i.MX8qm/qxp display pixel link.
> 
> Reviewed-by: Rob Herring 
> Signed-off-by: Liu Ying 
> ---
> v5->v6:
> * No change.
> 
> v4->v5:
> * No change.
> 
> v3->v4:
> * No change.
> 
> v2->v3:
> * Add Rob's R-b tag.
> 
> v1->v2:
> * Use graph schema. (Laurent)
> * Require all four pixel link output ports. (Laurent)
> * Mention pixel link is accessed via SCU firmware. (Rob)
> 
>  .../display/bridge/fsl,imx8qxp-pixel-link.yaml | 106 
> +
>  1 file changed, 106 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-link.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-link.yaml
> b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-link.yaml
> new file mode 100644
> index ..3af67cc
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-link.yaml
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/display/bridge/fsl,imx8qxp-pixel-link.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8qm/qxp Display Pixel Link
> +
> +maintainers:
> +  - Liu Ying 
> +
> +description: |
> +  The Freescale i.MX8qm/qxp Display Pixel Link(DPL) forms a standard
> +  asynchronous linkage between pixel sources(display controller or
> +  camera module) and pixel consumers(imaging or displays).
> +  It consists of two distinct functions, a pixel transfer function and a
> +  control interface.  Multiple pixel channels can exist per one control 
> channel.
> +  This binding documentation is only for pixel links whose pixel sources are
> +  display controllers.
> +
> +  The i.MX8qm/qxp Display Pixel Link is accessed via System Controller 
> Unit(SCU)
> +  firmware.
> +
> +properties:
> +  compatible:
> +    enum:
> +  - fsl,imx8qm-dc-pixel-link
> +  - fsl,imx8qxp-dc-pixel-link
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +  port@0:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: The pixel link input port node from upstream video 
> source.
> +
> +    patternProperties:
> +  "^port@[1-4]$":
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: The pixel link output port node to downstream bridge.
> +
> +    required:
> +  - port@0
> +  - port@1
> +  - port@2
> +  - port@3
> +  - port@4
> +
> +required:
> +  - compatible
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dc0-pixel-link0 {
> +    compatible = "fsl,imx8qxp-dc-pixel-link";
> +
> +    ports {
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +
> +    /* from dc0 pixel combiner channel0 */
> +    port@0 {
> +    reg = <0>;
> +
> +    dc0_pixel_link0_dc0_pixel_combiner_ch0: endpoint {
> +    remote-endpoint = 
> <&dc0_pixel_combiner_ch0_dc0_pixel_link0>;
> +    };
> +    };
> +
> +    /* to PXL2DPIs in MIPI/LVDS combo subsystems */
> +    port@1 {
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +    reg = <1>;
> +
> +    dc0_pixel_link0_mipi_lvds_0_pxl2dpi: endpoint@0 {
> +    reg = <0>;
> +    remote-endpoint = <&mipi_lvds_0_pxl2dpi_dc0_pixel_link0>;
> +    };
> +
> +    dc0_pixel_link0_mipi_lvds_1_pxl2dpi: endpoint@1 {
> +    reg = <1>;
> +    remote-endpoint = <&mipi_lvds_1_pxl2dpi_dc0_pixel_link0>;

Those also seem absent from other examples.

> +    };
> +    };
> +
> +    /* unused */
> +    port@2 {
> +    reg = <2>;
> +    };
> +
> +    /* unused */
> +    port@3 {
> +    reg = <3>;
> +    };
> +
> +    /* to imaging subsystem */
> +    port@4 {
> +    reg = <4>;
> +    };
> +    };
> +    };
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 03/14] dt-bindings: display: bridge: Add i.MX8qm/qxp pixel combiner binding

2021-03-23 Thread Marcel Ziswiler
On Wed, 2021-03-17 at 11:42 +0800, Liu Ying wrote:
> This patch adds bindings for i.MX8qm/qxp pixel combiner.
> 
> Reviewed-by: Rob Herring 
> Signed-off-by: Liu Ying 
> ---
> v5->v6:
> * No change.
> 
> v4->v5:
> * No change.
> 
> v3->v4:
> * No change.
> 
> v2->v3:
> * Add Rob's R-b tag.
> 
> v1->v2:
> * Use graph schema. (Laurent)
> * Use enum instead of oneOf + const for the reg property of pixel combiner
>   channels. (Rob)
> 
>  .../display/bridge/fsl,imx8qxp-pixel-combiner.yaml | 144 
> +
>  1 file changed, 144 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml
> b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml
> new file mode 100644
> index ..50bae21
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml
> @@ -0,0 +1,144 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/display/bridge/fsl,imx8qxp-pixel-combiner.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8qm/qxp Pixel Combiner
> +
> +maintainers:
> +  - Liu Ying 
> +
> +description: |
> +  The Freescale i.MX8qm/qxp Pixel Combiner takes two output streams from a
> +  single display controller and manipulates the two streams to support a 
> number
> +  of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) configured as
> +  either one screen, two screens, or virtual screens.  The pixel combiner is
> +  also responsible for generating some of the control signals for the pixel 
> link
> +  output channel.
> +
> +properties:
> +  compatible:
> +    enum:
> +  - fsl,imx8qm-pixel-combiner
> +  - fsl,imx8qxp-pixel-combiner
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: apb
> +
> +  power-domains:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^channel@[0-1]$":
> +    type: object
> +    description: Represents a display stream of pixel combiner.
> +
> +    properties:
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  reg:
> +    description: The display stream index.
> +    enum: [ 0, 1 ]
> +
> +  port@0:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: Input endpoint of the display stream.
> +
> +  port@1:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: Output endpoint of the display stream.
> +
> +    required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - reg
> +  - port@0
> +  - port@1
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - reg
> +  - clocks
> +  - clock-names
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include 
> +    #include 
> +    pixel-combiner@5602 {
> +    compatible = "fsl,imx8qxp-pixel-combiner";
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +    reg = <0x5602 0x1>;
> +    clocks = <&dc0_pixel_combiner_lpcg IMX_LPCG_CLK_4>;
> +    clock-names = "apb";
> +    power-domains = <&pd IMX_SC_R_DC_0>;
> +
> +    channel@0 {
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +    reg = <0>;
> +
> +    port@0 {
> +    reg = <0>;
> +
> +    dc0_pixel_combiner_ch0_dc0_dpu_disp0: endpoint {
> +    remote-endpoint = 
> <&dc0_dpu_disp0_dc0_pixel_combiner_ch0>;

While I acknowledge this just being an example you seem to call these as 
follows elsewhere:

Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml

dc0_dpu_disp0_dc0_pixel_combiner_ch0
pixel_combiner0_ch0_dpu0_disp0

Or am I just missing something?

> +    };
> +    };
> +
> +    port@1 {
> +    reg = <1>;
> +
> +    dc0_pixel_combiner_ch0_dc0_pixel_link0: endpoint {
> +    remote-endpoint = 
> <&dc0_pixel_link0_dc0_pixel_combiner_ch0>;
> +    };
> +    };
> +    };
> +
> +    channel@1 {
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +    reg = <1>;
> +
> +    port@0 {
> +    reg = <0>;
> +
> +    dc0_pixel_combiner_ch1_dc0_dpu_disp1: endpoint {
> +    remote-endpoint = 
> <&dc0_dpu_disp1_dc0_pixel_combiner_ch1>;

ditto

> +    };
> +    };
> +
> +    port@1 {
> +    reg = <1>;
> +
> +    dc0_pixel_combiner_ch1_dc0_pixel_link1: endpoint {
> +    remote-en

Re: [PATCH v6 00/14] Add some DRM bridge drivers support for i.MX8qm/qxp SoCs

2021-03-23 Thread Marcel Ziswiler
Hi Liu

Some further discrepancy with them binding examples:

arch/arm64/boot/dts/freescale/imx8qxp.dtsi:335.9-36: Warning (reg_format): 
/dpu@5618:reg: property has
invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
arch/arm64/boot/dts/freescale/imx8qxp.dtsi:508.9-35: Warning (reg_format): 
/syscon@56221000:reg: property has
invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
arch/arm64/boot/dts/freescale/imx8qxp.dtsi:601.9-34: Warning (reg_format): 
/phy@56228300:reg: property has
invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)
arch/arm64/boot/dts/freescale/imx8qxp.dtsi:613.9-36: Warning (reg_format): 
/pixel-combiner@5602:reg:
property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 2)

And with that I am unable to bring it up:

[1.714498] imx8qxp-ldb 562210001000.syscon:ldb: [drm:ldb_init_helper] 
*ERROR* failed to get regmap: -12
[1.724441] imx8qxp-ldb: probe of 562210001000.syscon:ldb failed with 
error -12
[1.734983] imx8qxp-pixel-combiner 56020001.pixel-combiner: invalid 
resource
[1.742830] imx8qxp-pixel-combiner: probe of 56020001.pixel-combiner 
failed with error -22
[1.754040] imx8qxp-display-pixel-link dc0-pixel-link0: 
[drm:imx8qxp_pixel_link_bridge_probe] *ERROR* failed
to get pixel link node alias id: -19
[1.769626] imx8qxp-pxl2dpi 562210001000.syscon:pxl2dpi: 
[drm:imx8qxp_pxl2dpi_bridge_probe] *ERROR*
failed to get regmap: -12
[1.781397] imx8qxp-pxl2dpi: probe of 562210001000.syscon:pxl2dpi failed 
with error -12
[1.840547] imx8qxp-lpcg-clk 5958.clock-controller: deferred probe 
timeout, ignoring dependency
[1.840571] imx8qxp-lpcg-clk: probe of 5958.clock-controller failed with 
error -110

Any suggestions welcome. Thanks!

Cheers

Marcel

On Wed, 2021-03-17 at 11:42 +0800, Liu Ying wrote:
> Hi,
> 
> This is the v6 series to add some DRM bridge drivers support
> for i.MX8qm/qxp SoCs.
> 
> The bridges may chain one by one to form display pipes to support
> LVDS displays.  The relevant display controller is DPU embedded in
> i.MX8qm/qxp SoCs.
> 
> The DPU KMS driver can be found at:
> https://www.spinics.net/lists/arm-kernel/msg878542.html
> 
> This series supports the following display pipes:
> 1) i.MX8qxp:
> prefetch eng -> DPU -> pixel combiner -> pixel link ->
> pixel link to DPI(PXL2DPI) -> LVDS display bridge(LDB)
> 
> 2) i.MX8qm:
> prefetch eng -> DPU -> pixel combiner -> pixel link -> LVDS display 
> bridge(LDB)
> 
> 
> This series dropped the patch 'phy: Add LVDS configuration options', as
> suggested by Robert Foss, because it has already been sent with the following
> series to add Mixel combo PHY found in i.MX8qxp:
> https://www.spinics.net/lists/arm-kernel/msg879957.html
> 
> So, this version depends on that series.
> 
> 
> Patch 1/14 and 2/14 add bus formats used by pixel combiner.
> 
> Patch 7/14 adds dt-binding for Control and Status Registers module(a syscon
> used by PXL2DPI and LDB), which references the PXL2DPI and LDB schemas.
> 
> Patch 10/14 adds a helper for LDB bridge drivers.
> 
> Patch 3/14 ~ 6/14, 8/14, 9/14 and 11/14 ~ 13/14 add drm bridge drivers and
> dt-bindings support for the bridges.
> 
> Patch 14/14 updates MAINTAINERS.
> 
> 
> I've tested this series with a koe,tx26d202vm0bwa dual link LVDS panel and
> a LVDS to HDMI bridge(with a downstream drm bridge driver).
> 
> 
> Welcome comments, thanks.
> 
> v5->v6:
> * Fix data organizations in documentation(patch 2/14) for
>   MEDIA_BUS_FMT_RGB{666,888}_1X30-CPADLO. (Laurent)
> * Add Laurent's R-b tags on patch 1/14 and 2/14.
> * Drop 'select' schema from the CSR dt-binding documentation(patch 7/14). 
> (Rob)
> * Add Rob's R-b tag on patch 8/14.
> 
> v4->v5:
> * Drop the patch 'phy: Add LVDS configuration options'. (Robert)
> * Add Robert's R-b tags on patch 1/14, 2/14, 4/14 and 6/14.
> * Drop the 'PC_BUF_PARA_REG' register definition from the pixel combiner 
> bridge
>   driver(patch 4/14). (Robert)
> * Make a comment occupy a line in the pixel link bridge driver(patch 6/14).
>   (Robert)
> * Introduce a new patch(patch 7/14) to add dt-binding for Control and Status
>   Registers module. (Rob)
> * Make imx-ldb-helper be a pure object to be linked with i.MX8qxp LDB bridge
>   driver and i.MX8qm LDB bridge driver, instead of a module.  Correspondingly,
>   rename 'imx8{qm, qxp}-ldb.c' to 'imx8{qm, qxp}-ldb-drv.c'. (Robert)
> * Move 'imx_ldb_helper.h' to 'drivers/gpu/drm/bridge/imx/imx-ldb-helper.h'.
>   (Robert)
> * s/__FSL_IMX_LDB__/__IMX_LDB_HELPER__/  for 'imx-ldb-helper.h'.
> 
> v3->v4:
> * Use 'fsl,sc-resource' DT property to get the SCU resource ID associated with
>   the PXL2DPI instance instead of using alias ID. (Rob)
> * Add Rob's R-b tag on patch 11/14.
> 
> v2->v3:
> * Drop 'fsl,syscon' DT properties from fsl,imx8qxp-ldb.yaml and
>   fsl,imx8qxp-pxl2dpi.yaml. (Rob)
> * Mention the CSR module controls LDB and PXL2DPI in fsl,imx8qxp-ldb.yaml and
>   fsl,imx8q

Re: [PATCH v2 3/3] dt-bindings: display: panel: add bindings for logic technologies displays

2020-01-20 Thread Marcel Ziswiler
On Wed, 2019-10-30 at 09:29 -0500, Rob Herring wrote:
> On Sun, Oct 27, 2019 at 03:26:09PM +0100, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler 
> > 
> > Add bindings for the following 3 previously added display panels
> > manufactured by Logic Technologies Limited:
> > 
> > - LT161010-2NHC e.g. as found in the Toradex Capacitive Touch
> > Display
> > 7" Parallel [1]
> > - LT161010-2NHR e.g. as found in the Toradex Resistive Touch
> > Display 7"
> > Parallel [2]
> > - LT170410-2WHC e.g. as found in the Toradex Capacitive Touch
> > Display
> > 10.1" LVDS [3]
> > 
> > Those panels may also be distributed by Endrich Bauelemente
> > Vertriebs
> > GmbH [4].
> > 
> > [1] 
> > https://docs.toradex.com/104497-7-inch-parallel-capacitive-touch-display-800x480-datasheet.pdf
> > [2] 
> > https://docs.toradex.com/104498-7-inch-parallel-resistive-touch-display-800x480.pdf
> > [3] 
> > https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
> > [4] 
> > https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30
> > 
> > Signed-off-by: Marcel Ziswiler 
> > 
> > ---
> > 
> > Changes in v2:
> > - New patch adding display panel bindings as well as suggested by
> > Rob.
> > 
> >  .../panel/logictechno,lt161010-2nhc.yaml  | 44
> > +++
> >  .../panel/logictechno,lt161010-2nhr.yaml  | 44
> > +++
> >  .../panel/logictechno,lt170410-2whc.yaml  | 44
> > +++
> >  3 files changed, 132 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt16101
> > 0-2nhc.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt16101
> > 0-2nhr.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt17041
> > 0-2whc.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/logictechno,lt161
> > 010-2nhc.yaml
> > b/Documentation/devicetree/bindings/display/panel/logictechno,lt161
> > 010-2nhc.yaml
> > new file mode 100644
> > index ..0dfe94d38a47
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/panel/logictechno,lt161
> > 010-2nhc.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Except the license for new bindings should be: 
> 
> (GPL-2.0-only OR BSD-2-Clause)

OK, will send a v3 shortly.

> Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 3/3] dt-bindings: display: panel: add bindings for logic technologies displays

2020-01-20 Thread Marcel Ziswiler
Sorry, just noticed that this has not gone through yet.

On Wed, 2019-10-30 at 09:28 -0500, Rob Herring wrote:
> On Sun, Oct 27, 2019 at 03:26:09PM +0100, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler 
> > 
> > Add bindings for the following 3 previously added display panels
> > manufactured by Logic Technologies Limited:
> > 
> > - LT161010-2NHC e.g. as found in the Toradex Capacitive Touch
> > Display
> > 7" Parallel [1]
> > - LT161010-2NHR e.g. as found in the Toradex Resistive Touch
> > Display 7"
> > Parallel [2]
> > - LT170410-2WHC e.g. as found in the Toradex Capacitive Touch
> > Display
> > 10.1" LVDS [3]
> > 
> > Those panels may also be distributed by Endrich Bauelemente
> > Vertriebs
> > GmbH [4].
> > 
> > [1] 
> > https://docs.toradex.com/104497-7-inch-parallel-capacitive-touch-display-800x480-datasheet.pdf
> > [2] 
> > https://docs.toradex.com/104498-7-inch-parallel-resistive-touch-display-800x480.pdf
> > [3] 
> > https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
> > [4] 
> > https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30
> > 
> > Signed-off-by: Marcel Ziswiler 
> > 
> > ---
> > 
> > Changes in v2:
> > - New patch adding display panel bindings as well as suggested by
> > Rob.
> > 
> >  .../panel/logictechno,lt161010-2nhc.yaml  | 44
> > +++
> >  .../panel/logictechno,lt161010-2nhr.yaml  | 44
> > +++
> >  .../panel/logictechno,lt170410-2whc.yaml  | 44
> > +++
> >  3 files changed, 132 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt16101
> > 0-2nhc.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt16101
> > 0-2nhr.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/display/panel/logictechno,lt17041
> > 0-2whc.yaml
> 
> I would just put these into 1 document as the compatible is the only 
> difference.

No, not quite just the compatible are different as the first and last
panel feature capacitive touch while the middle one is resistive and
the first two panels are parallel RGB ones while the last one is an
LVDS panel.

> Either way:
> 
> Reviewed-by: Rob Herring 
> 
> Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/3] drm/panel: simple: add display timings for logic technologies displays

2020-01-20 Thread Marcel Ziswiler
Hi Sam

On Sun, 2020-01-19 at 23:47 +0100, Sam Ravnborg wrote:
> Hi Marcel.
> 
> On Sun, Jan 19, 2020 at 11:02:03PM +0100, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler 
> > 
> > Add display timings for the following 3 display panels manufactured
> > by
> > Logic Technologies Limited:
> > 
> > - LT161010-2NHC e.g. as found in the Toradex Capacitive Touch
> > Display
> >   7" Parallel [1]
> > - LT161010-2NHR e.g. as found in the Toradex Resistive Touch
> > Display 7"
> >   Parallel [2]
> > - LT170410-2WHC e.g. as found in the Toradex Capacitive Touch
> > Display
> >   10.1" LVDS [3]
> > 
> > Those panels may also be distributed by Endrich Bauelemente
> > Vertriebs
> > GmbH [4].
> > 
> > [1] 
> > https://docs.toradex.com/104497-7-inch-parallel-capacitive-touch-display-800x480-datasheet.pdf
> > [2] 
> > https://docs.toradex.com/104498-7-inch-parallel-resistive-touch-display-800x480.pdf
> > [3] 
> > https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
> > [4] 
> > https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30
> > 
> > Signed-off-by: Marcel Ziswiler 
> > Reviewed-by: Philippe Schenker 
> > 
> > ---
> > 
> > Changes in v3:
> > - Fix typo in pixelclock frequency for lt170410_2whc as recently
> >   discovered by Philippe.
> > 
> > Changes in v2:
> > - Added Philippe's reviewed-by.
> > 
> >  drivers/gpu/drm/panel/panel-simple.c | 65
> > 
> >  1 file changed, 65 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index d6f77bc494c7..4140e0faff06 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -2107,6 +2107,62 @@ static const struct panel_desc lg_lp129qe =
> > {
> > },
> >  };
> >  
> > +static const struct display_timing logictechno_lt161010_2nh_timing
> > = {
> > +   .pixelclock = { 2640, 3330, 4680 },
> > +   .hactive = { 800, 800, 800 },
> > +   .hfront_porch = { 16, 210, 354 },
> > +   .hback_porch = { 46, 46, 46 },
> > +   .hsync_len = { 1, 20, 40 },
> > +   .vactive = { 480, 480, 480 },
> > +   .vfront_porch = { 7, 22, 147 },
> > +   .vback_porch = { 23, 23, 23 },
> > +   .vsync_len = { 1, 10, 20 },
> > +   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> > +DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE
> > |
> > +DISPLAY_FLAGS_SYNC_POSEDGE,
> > +};
> > +
> > +static const struct panel_desc logictechno_lt161010_2nh = {
> > +   .timings = &logictechno_lt161010_2nh_timing,
> > +   .num_timings = 1,
> > +   .size = {
> > +   .width = 154,
> > +   .height = 86,
> > +   },
> > +   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > +   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
> > +DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> > +DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
> > +};
> connector_type needs to be specified for all panels.
> This is something we have added recently and is today mandatory
> for new panel-simple panels.

Sorry, forgot about that one. Will add it in v4.

> Also please re-order so we add bindings before we driver support for
> the panels.
> This makes checkpatch (and me) more happy.

Not that my checkpatch would warn me about that but kinda makes sense.

> Ohh, and bonus points for using display_timing and specifying
> min,typ,max values.

Thanks, man.

>   Sam

Cheers

Marcel

> > +
> > +static const struct display_timing
> > logictechno_lt170410_2whc_timing = {
> > +   .pixelclock = { 6890, 7110, 7340 },
> > +   .hactive = { 1280, 1280, 1280 },
> > +   .hfront_porch = { 23, 60, 71 },
> > +   .hback_porch = { 23, 60, 71 },
> > +   .hsync_len = { 15, 40, 47 },
> > +   .vactive = { 800, 800, 800 },
> > +   .vfront_porch = { 5, 7, 10 },
> > +   .vback_porch = { 5, 7, 10 },
> > +   .vsync_len = { 6, 9, 12 },
> > +   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> > +DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE
> > |
> > +DISPLAY_FLAGS_SYNC_POSEDGE,
> > +};
> > +
> > +static const struct panel_desc logictechno_lt170410_2whc = {
> > +   .timings = &logictechno_lt170410_2whc_timing,
> > +   .num_timings = 1,
> > +   .size = {
>

[PATCH v1 1/2] dt-bindings: add vendor prefix for logic technologies limited

2019-09-20 Thread Marcel Ziswiler
From: Marcel Ziswiler 

Add vendor prefix for Logic Technologies Limited [1] which is a Chinese
display manufacturer e.g. distributed by German Endrich Bauelemente
Vertriebs GmbH [2].

[1] https://logictechno.com/contact-us/
[2] https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30

Signed-off-by: Marcel Ziswiler 

---

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml 
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 967e78c5ec0a..1441146f394f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -541,6 +541,8 @@ patternProperties:
 description: Linear Technology Corporation
   "^logicpd,.*":
 description: Logic PD, Inc.
+  "^logictechno,.*":
+description: Logic Technologies Limited
   "^longcheer,.*":
 description: Longcheer Technology (Shanghai) Co., Ltd.
   "^lsi,.*":
-- 
2.21.0



[PATCH v1 2/2] drm/panel: simple: add display timings for logic technologies displays

2019-09-20 Thread Marcel Ziswiler
From: Marcel Ziswiler 

Add display timings for the following 3 display panels manufactured by
Logic Technologies Limited:

- LT161010-2NHC e.g. as found in the Toradex Capacitive Touch Display
  7" Parallel [1]
- LT161010-2NHR e.g. as found in the Toradex Resistive Touch Display 7"
  Parallel [2]
- LT170410-2WHC e.g. as found in the Toradex Capacitive Touch Display
  10.1" LVDS [3]

Those panels may also be distributed by Endrich Bauelemente Vertriebs
GmbH [4].

[1] 
https://docs.toradex.com/104497-7-inch-parallel-capacitive-touch-display-800x480-datasheet.pdf
[2] 
https://docs.toradex.com/104498-7-inch-parallel-resistive-touch-display-800x480.pdf
[3] 
https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
[4] https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30

Signed-off-by: Marcel Ziswiler 

---

 drivers/gpu/drm/panel/panel-simple.c | 65 
 1 file changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 28fa6ba7b767..42bd0de25167 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2034,6 +2034,62 @@ static const struct panel_desc lg_lp129qe = {
},
 };
 
+static const struct display_timing logictechno_lt161010_2nh_timing = {
+   .pixelclock = { 2640, 3330, 4680 },
+   .hactive = { 800, 800, 800 },
+   .hfront_porch = { 16, 210, 354 },
+   .hback_porch = { 46, 46, 46 },
+   .hsync_len = { 1, 20, 40 },
+   .vactive = { 480, 480, 480 },
+   .vfront_porch = { 7, 22, 147 },
+   .vback_porch = { 23, 23, 23 },
+   .vsync_len = { 1, 10, 20 },
+   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
+DISPLAY_FLAGS_SYNC_POSEDGE,
+};
+
+static const struct panel_desc logictechno_lt161010_2nh = {
+   .timings = &logictechno_lt161010_2nh_timing,
+   .num_timings = 1,
+   .size = {
+   .width = 154,
+   .height = 86,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
+DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
+DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
+};
+
+static const struct display_timing logictechno_lt170410_2whc_timing = {
+   .pixelclock = { 6890, 7110, 734 },
+   .hactive = { 1280, 1280, 1280 },
+   .hfront_porch = { 23, 60, 71 },
+   .hback_porch = { 23, 60, 71 },
+   .hsync_len = { 15, 40, 47 },
+   .vactive = { 800, 800, 800 },
+   .vfront_porch = { 5, 7, 10 },
+   .vback_porch = { 5, 7, 10 },
+   .vsync_len = { 6, 9, 12 },
+   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
+DISPLAY_FLAGS_SYNC_POSEDGE,
+};
+
+static const struct panel_desc logictechno_lt170410_2whc = {
+   .timings = &logictechno_lt170410_2whc_timing,
+   .num_timings = 1,
+   .size = {
+   .width = 217,
+   .height = 136,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
+DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
+DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
+};
+
 static const struct drm_display_mode mitsubishi_aa070mc01_mode = {
.clock = 30400,
.hdisplay = 800,
@@ -3264,6 +3320,15 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "lg,lp129qe",
.data = &lg_lp129qe,
+   }, {
+   .compatible = "logictechno,lt161010-2nhc",
+   .data = &logictechno_lt161010_2nh,
+   }, {
+   .compatible = "logictechno,lt161010-2nhr",
+   .data = &logictechno_lt161010_2nh,
+   }, {
+   .compatible = "logictechno,lt170410-2whc",
+   .data = &logictechno_lt170410_2whc,
}, {
.compatible = "mitsubishi,aa070mc01-ca1",
.data = &mitsubishi_aa070mc01,
-- 
2.21.0



Re: TK1: DRM, Nouveau and VIC

2018-12-11 Thread Marcel Ziswiler
Hi Thierry

On Mon, 2018-12-10 at 12:00 +0100, Thierry Reding wrote:
> On Mon, Dec 10, 2018 at 11:21:47AM +0100, Thierry Reding wrote:
> > On Sat, Dec 08, 2018 at 02:54:45PM +0000, Marcel Ziswiler wrote:
> > > Hi Thierry et al.
> > > 
> > > I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on
> > > Tegra124") graphics on Apalis TK1 is broken. During boot it fails
> > > loading the vic firmware:
> > > 
> > > [1.595824] tegra-vic 5434.vic: Direct firmware load for
> > > nvidia/tegra124/vic03_ucode.bin failed with error -2
> > > [1.606140] tegra-vic: probe of 5434.vic failed with error
> > > -2
> > > 
> > > Subsequently Tegra HDMI seems to fail completely:
> > > 
> > > [2.379860] tegra-hdmi 5428.hdmi: failed to get PLL
> > > regulator
> > > 
> > > And finally, Nouveau even crashes:
> > > 
> > > [8.241115] nouveau 5700.gpu: Linked as a consumer to
> > > regulator.31
> > > [8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1)
> > > [8.253396] nouveau 5700.gpu: imem: using IOMMU
> > > [8.270210] Unable to handle kernel NULL pointer dereference
> > > at
> > > virtual address 006c
> > > [8.278340] pgd = (ptrval)
> > > [8.281250] [006c] *pgd=
> > > [8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> > > [8.290260] Modules linked in: nouveau(+) ttm
> > > [8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted
> > > 4.20.0-
> > > rc5-next-20181207-8-g85b0f8e25f86-dirty #110
> > > [8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device
> > > Tree)
> > > [8.311331] PC is at drm_plane_register_all+0x18/0x50
> > > [8.316373] LR is at drm_modeset_register_all+0xc/0x70
> > > [8.321513] pc : []lr : []psr:
> > > a0060013
> > > [8.327768] sp : ed527c70  ip : ecc43ec0  fp : 
> > > [8.332993] r10: 0016  r9 : ecc43e80  r8 : 
> > > [8.338209] r7 : bf182c80  r6 :   r5 : ed61b24c  r4 :
> > > fffc
> > > [8.344735] r3 : 0002f000  r2 :   r1 : 2e124000  r0 :
> > > ed61b000
> > > [8.351260] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA
> > > ARM  Segment none
> > > [8.358383] Control: 10c5387d  Table: ad64c06a  DAC: 0051
> > > [8.364127] Process systemd-udevd (pid: 203, stack limit =
> > > 0x(ptrval))
> > > [8.370654] Stack: (0xed527c70 to 0xed528000)
> > > [8.375004] 7c60: ed61b000
> > > ed61b000  c0564cc8
> > > [8.383177] 7c80: ed61b000   c054b5b8 0001
> > > 0001  
> > > [8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000  
> > >  bf180c5c bf0dc900
> > > [8.399531] 7cc0: eda29208 5dfe844b  ee9f2a10 
> > > bf180c5c  c05a9328
> > > [8.407695] 7ce0: c1006828 ee9f2a10 c100682c  
> > > c05a744c ee9f2a10 bf180c5c
> > > [8.415871] 7d00: ee9f2a44 c05a77a8  c0f08c48 bf182980
> > > c05a769c eefd14d0 c05a77a8
> > > [8.424048] 7d20:  ee9f2a10 bf180c5c ee9f2a44 c05a77a8
> > >  c0f08c48 bf182980
> > > [8.432226] 7d40:  c05a7884 ee9ebfb4 c0f08c48 bf180c5c
> > > c05a5790  ee88135c
> > > [8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80
> > > c0f71168  c05a692c
> > > [8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8
> > > e000 bf1aa000 c05a84a0
> > > [8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220
> > > c0f08c48 e000 c0102ed0
> > > [8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113
> > > bf182980 0040 c02592d0
> > > [8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0
> > > c01b3d98 000c c025a8cc
> > > [8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980
> > > 0002 ed53f4c0 0002
> > > [8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 
> > > ed527f40 0002 eceb9fc0
> > > [8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980
> > > c01b2f88  c01b279c
> > > [8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0
> > > c0d58964 c0ca726c c0ca7278
> > > [8.513978] 7e80: c0ca72d0 c0f08c48 0

Re: TK1: DRM, Nouveau and VIC

2018-12-11 Thread Marcel Ziswiler
Hi Thierry

On Mon, 2018-12-10 at 17:23 +0100, Thierry Reding wrote:

Snip.

> > Looks like with pci_disable_device() it may take a rather strange
> > path...
> 
> Yikes... it has no business at all calling pci_disable_device() on
> Tegra. Unless if you happen to have a GPU plugged into the PCIe slot.
> I'm assuming that's not what you're doing?

Nope, I only have a Wi-Fi card behind a PCIe switch though (;-p).

> I'll see if I can reproduce (and fix) that crash on unload.
> Admittedly
> it's not something that I regularly test. Perhaps that's something
> that
> I should change...

Don't worry. After a couple of years working on this I happen to try
this the first time myself just now (;-p).

> Thierry

Cheers

Marcel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: drm/nouveau: tegra: Call nouveau_drm_device_init()

2018-12-11 Thread Marcel Ziswiler
On Fri, 2018-11-23 at 13:11 +0100, Thierry Reding wrote:
> From: Thierry Reding 
> 
> As part of commit cfea88a4d866 ("drm/nouveau: Start using new drm_dev
> initialization helpers"), the initialization of the Nouveau DRM
> device
> was reworked and along the way the platform driver initialization was
> left incomplete. Add a call to nouveau_drm_device_init() to make sure
> all of the structures are properly initialized.
> 
> Signed-off-by: Thierry Reding 
> Reviewed-by: Lyude Paul 

Tested-by: Marcel Ziswiler 

> ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 2b2baf6e0e0d..d2928d43f29a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1171,10 +1171,16 @@ nouveau_platform_device_create(const struct
> nvkm_device_tegra_func *func,
>   goto err_free;
>   }
>  
> + err = nouveau_drm_device_init(drm);
> + if (err)
> + goto err_put;
> +
>   platform_set_drvdata(pdev, drm);
>  
>   return drm;
>  
> +err_put:
> + drm_dev_put(drm);
>  err_free:
>   nvkm_device_del(pdevice);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: TK1: DRM, Nouveau and VIC

2018-12-11 Thread Marcel Ziswiler
Hi Thierry

On Mon, 2018-12-10 at 11:21 +0100, Thierry Reding wrote:
> On Sat, Dec 08, 2018 at 02:54:45PM +0000, Marcel Ziswiler wrote:
> > Hi Thierry et al.
> > 
> > I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on
> > Tegra124") graphics on Apalis TK1 is broken. During boot it fails
> > loading the vic firmware:
> > 
> > [1.595824] tegra-vic 5434.vic: Direct firmware load for
> > nvidia/tegra124/vic03_ucode.bin failed with error -2
> > [1.606140] tegra-vic: probe of 5434.vic failed with error
> > -2
> > 
> > Subsequently Tegra HDMI seems to fail completely:
> > 
> > [2.379860] tegra-hdmi 5428.hdmi: failed to get PLL
> > regulator
> > 
> > And finally, Nouveau even crashes:
> > 
> > [8.241115] nouveau 5700.gpu: Linked as a consumer to
> > regulator.31
> > [8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1)
> > [8.253396] nouveau 5700.gpu: imem: using IOMMU
> > [8.270210] Unable to handle kernel NULL pointer dereference at
> > virtual address 006c
> > [8.278340] pgd = (ptrval)
> > [8.281250] [006c] *pgd=
> > [8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> > [8.290260] Modules linked in: nouveau(+) ttm
> > [8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted
> > 4.20.0-
> > rc5-next-20181207-8-g85b0f8e25f86-dirty #110
> > [8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device
> > Tree)
> > [8.311331] PC is at drm_plane_register_all+0x18/0x50
> > [8.316373] LR is at drm_modeset_register_all+0xc/0x70
> > [8.321513] pc : []lr : []psr:
> > a0060013
> > [8.327768] sp : ed527c70  ip : ecc43ec0  fp : 
> > [8.332993] r10: 0016  r9 : ecc43e80  r8 : 
> > [8.338209] r7 : bf182c80  r6 :   r5 : ed61b24c  r4 :
> > fffc
> > [8.344735] r3 : 0002f000  r2 :   r1 : 2e124000  r0 :
> > ed61b000
> > [8.351260] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA
> > ARM  Segment none
> > [8.358383] Control: 10c5387d  Table: ad64c06a  DAC: 0051
> > [8.364127] Process systemd-udevd (pid: 203, stack limit =
> > 0x(ptrval))
> > [8.370654] Stack: (0xed527c70 to 0xed528000)
> > [8.375004] 7c60: ed61b000
> > ed61b000  c0564cc8
> > [8.383177] 7c80: ed61b000   c054b5b8 0001
> > 0001  
> > [8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000  
> >  bf180c5c bf0dc900
> > [8.399531] 7cc0: eda29208 5dfe844b  ee9f2a10 
> > bf180c5c  c05a9328
> > [8.407695] 7ce0: c1006828 ee9f2a10 c100682c  
> > c05a744c ee9f2a10 bf180c5c
> > [8.415871] 7d00: ee9f2a44 c05a77a8  c0f08c48 bf182980
> > c05a769c eefd14d0 c05a77a8
> > [8.424048] 7d20:  ee9f2a10 bf180c5c ee9f2a44 c05a77a8
> >  c0f08c48 bf182980
> > [8.432226] 7d40:  c05a7884 ee9ebfb4 c0f08c48 bf180c5c
> > c05a5790  ee88135c
> > [8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80
> > c0f71168  c05a692c
> > [8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8
> > e000 bf1aa000 c05a84a0
> > [8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220
> > c0f08c48 e000 c0102ed0
> > [8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113
> > bf182980 0040 c02592d0
> > [8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0
> > c01b3d98 000c c025a8cc
> > [8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980
> > 0002 ed53f4c0 0002
> > [8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 
> > ed527f40 0002 eceb9fc0
> > [8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980
> > c01b2f88  c01b279c
> > [8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0
> > c0d58964 c0ca726c c0ca7278
> > [8.513978] 7e80: c0ca72d0 c0f08c48  c02654a0 
> >  e000 bf00
> > [8.522157] 7ea0:     
> >  6e72656b 6c65
> > [8.530336] 7ec0:     
> >   
> > [8.538502] 7ee0:     
> > 5dfe844b 7fff c0f08c48
> > [8.546677] 7f00:  000f b6f761cc c0101204 ed526000
> > 017b 004a3270 c01

TK1: DRM, Nouveau and VIC

2018-12-10 Thread Marcel Ziswiler
Hi Thierry et al.

I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on
Tegra124") graphics on Apalis TK1 is broken. During boot it fails
loading the vic firmware:

[1.595824] tegra-vic 5434.vic: Direct firmware load for
nvidia/tegra124/vic03_ucode.bin failed with error -2
[1.606140] tegra-vic: probe of 5434.vic failed with error -2

Subsequently Tegra HDMI seems to fail completely:

[2.379860] tegra-hdmi 5428.hdmi: failed to get PLL regulator

And finally, Nouveau even crashes:

[8.241115] nouveau 5700.gpu: Linked as a consumer to
regulator.31
[8.247889] nouveau 5700.gpu: NVIDIA GK20A (0ea000a1)
[8.253396] nouveau 5700.gpu: imem: using IOMMU
[8.270210] Unable to handle kernel NULL pointer dereference at
virtual address 006c
[8.278340] pgd = (ptrval)
[8.281250] [006c] *pgd=
[8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[8.290260] Modules linked in: nouveau(+) ttm
[8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted 4.20.0-
rc5-next-20181207-8-g85b0f8e25f86-dirty #110
[8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[8.311331] PC is at drm_plane_register_all+0x18/0x50
[8.316373] LR is at drm_modeset_register_all+0xc/0x70
[8.321513] pc : []lr : []psr: a0060013
[8.327768] sp : ed527c70  ip : ecc43ec0  fp : 
[8.332993] r10: 0016  r9 : ecc43e80  r8 : 
[8.338209] r7 : bf182c80  r6 :   r5 : ed61b24c  r4 :
fffc
[8.344735] r3 : 0002f000  r2 :   r1 : 2e124000  r0 :
ed61b000
[8.351260] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA
ARM  Segment none
[8.358383] Control: 10c5387d  Table: ad64c06a  DAC: 0051
[8.364127] Process systemd-udevd (pid: 203, stack limit =
0x(ptrval))
[8.370654] Stack: (0xed527c70 to 0xed528000)
[8.375004] 7c60: ed61b000
ed61b000  c0564cc8
[8.383177] 7c80: ed61b000   c054b5b8 0001
0001  
[8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000  
 bf180c5c bf0dc900
[8.399531] 7cc0: eda29208 5dfe844b  ee9f2a10 
bf180c5c  c05a9328
[8.407695] 7ce0: c1006828 ee9f2a10 c100682c  
c05a744c ee9f2a10 bf180c5c
[8.415871] 7d00: ee9f2a44 c05a77a8  c0f08c48 bf182980
c05a769c eefd14d0 c05a77a8
[8.424048] 7d20:  ee9f2a10 bf180c5c ee9f2a44 c05a77a8
 c0f08c48 bf182980
[8.432226] 7d40:  c05a7884 ee9ebfb4 c0f08c48 bf180c5c
c05a5790  ee88135c
[8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80
c0f71168  c05a692c
[8.448570] 7d80: bf15dc00 bf180ac8 e000 bf180c5c bf180ac8
e000 bf1aa000 c05a84a0
[8.456746] 7da0: bf182b80 bf180ac8 e000 bf1aa170 c0fbd220
c0f08c48 e000 c0102ed0
[8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 000c 6113
bf182980 0040 c02592d0
[8.473102] 7de0: eda60200 2e124000 ee80 006000c0 006000c0
c01b3d98 000c c025a8cc
[8.481281] 7e00: c024ce54 a113 bf182980 5dfe844b bf182980
0002 ed53f4c0 0002
[8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 
ed527f40 0002 eceb9fc0
[8.497625] 7e40: 0002 c01b61a4 bf18298c 7fff bf182980
c01b2f88  c01b279c
[8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0
c0d58964 c0ca726c c0ca7278
[8.513978] 7e80: c0ca72d0 c0f08c48  c02654a0 
 e000 bf00
[8.522157] 7ea0:     
 6e72656b 6c65
[8.530336] 7ec0:     
  
[8.538502] 7ee0:     
5dfe844b 7fff c0f08c48
[8.546677] 7f00:  000f b6f761cc c0101204 ed526000
017b 004a3270 c01b66a4
[8.554855] 7f20: 7fff  0003 0001 004a3270
f0ced000 06e8994c 
[8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c
f7b75d34 f63e62dc 0016b000
[8.571209] 7f60: 0017f6f0    00050a48
003b 003c 0023
[8.579388] 7f80:  0014  5dfe844b 
004c0ec0  0001
[8.587554] 7fa0: 017b c0101000 004c0ec0  000f
b6f761cc  0002
[8.595730] 7fc0: 004c0ec0  0001 017b 0048e114
  004a3270
[8.603908] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0 400d0010
000f  
[8.612096] [] (drm_plane_register_all) from []
(drm_modeset_register_all+0xc/0x70)  
[8.621499] [] (drm_modeset_register_all) from
[] (drm_dev_register+0x168/0x1c4)
[8.630855] [] (drm_dev_register) from []
(nouveau_platform_probe+0x6c/0x88 [nouveau])
[8.640739] [] (nouveau_platform_probe [nouveau]) from
[] (platform_drv_probe+0x48/0x98)
[8.650574] [] (platform_drv_probe) from []
(really_probe+0x1e0/0x2cc

[PATCH v2] drm/tegra: hdmi: probe deferral error reporting

2018-08-15 Thread Marcel Ziswiler
From: Marcel Ziswiler 

Actually report the error code from devm_regulator_get() unless it
is just a probe deferral.

Signed-off-by: Marcel Ziswiler 

---

Changes in v2:
- Silence probe deferral as suggested by Lucas.
- Fix line over 80 characters as reported by checkpatch.

 drivers/gpu/drm/tegra/hdmi.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0082468f703c..073d54b7225e 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1693,14 +1693,22 @@ static int tegra_hdmi_probe(struct platform_device 
*pdev)
 
hdmi->hdmi = devm_regulator_get(&pdev->dev, "hdmi");
if (IS_ERR(hdmi->hdmi)) {
-   dev_err(&pdev->dev, "failed to get HDMI regulator\n");
-   return PTR_ERR(hdmi->hdmi);
+   err = PTR_ERR(hdmi->hdmi);
+   if (err != -EPROBE_DEFER)
+   dev_err(&pdev->dev, "failed to get HDMI regulator: 
%d\n",
+   err);
+
+   return err;
}
 
hdmi->pll = devm_regulator_get(&pdev->dev, "pll");
if (IS_ERR(hdmi->pll)) {
-   dev_err(&pdev->dev, "failed to get PLL regulator\n");
-   return PTR_ERR(hdmi->pll);
+   err = PTR_ERR(hdmi->pll);
+   if (err != -EPROBE_DEFER)
+   dev_err(&pdev->dev, "failed to get PLL regulator: %d\n",
+   err);
+
+   return err;
}
 
hdmi->vdd = devm_regulator_get(&pdev->dev, "vdd");
-- 
2.14.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/tegra: hdmi: probe deferral error reporting

2018-07-24 Thread Marcel Ziswiler


On July 23, 2018 10:49:15 PM GMT+02:00, Lucas Stach  wrote:
>Am Freitag, den 20.07.2018, 09:55 +0200 schrieb Marcel Ziswiler:
>> From: Marcel Ziswiler 
>> 
>> Actually report the error code from devm_regulator_get() which may as
>> well just be a probe deferral.
>
>This is still noisy, so while you are changing this I would prefer if
>this wouldn't print the error message if it's a simple probe deferral.

Yeah, trust me, I'm not the inventor of this probe deferral disaster!

>Suppressing the log message in that case gets us much cleaner kernel
>logs, where real errors stand out.

Ok, agreed. Will send a v2.

>Regards,
>Lucas
>
>> 
>> Signed-off-by: Marcel Ziswiler 
>> 
>> ---
>> 
>>  drivers/gpu/drm/tegra/hdmi.c | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/tegra/hdmi.c
>b/drivers/gpu/drm/tegra/hdmi.c
>> index 0082468f703c..94c182dbb6d0 100644
>> --- a/drivers/gpu/drm/tegra/hdmi.c
>> +++ b/drivers/gpu/drm/tegra/hdmi.c
>> @@ -1693,14 +1693,16 @@ static int tegra_hdmi_probe(struct
>platform_device *pdev)
>>  
>>  hdmi->hdmi = devm_regulator_get(&pdev->dev, "hdmi");
>>  if (IS_ERR(hdmi->hdmi)) {
>> -dev_err(&pdev->dev, "failed to get HDMI regulator\n");
>> -return PTR_ERR(hdmi->hdmi);
>> +err = PTR_ERR(hdmi->hdmi);
>> +dev_err(&pdev->dev, "failed to get HDMI regulator: %d\n", err);
>> +return err;
>>  }
>>  
>>  hdmi->pll = devm_regulator_get(&pdev->dev, "pll");
>>  if (IS_ERR(hdmi->pll)) {
>> -dev_err(&pdev->dev, "failed to get PLL regulator\n");
>> -return PTR_ERR(hdmi->pll);
>> +err = PTR_ERR(hdmi->pll);
>> +dev_err(&pdev->dev, "failed to get PLL regulator: %d\n", err);
>> +return err;
>>  }
>>  
>>  hdmi->vdd = devm_regulator_get(&pdev->dev, "vdd");
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/tegra: hdmi: probe deferral error reporting

2018-07-23 Thread Marcel Ziswiler
From: Marcel Ziswiler 

Actually report the error code from devm_regulator_get() which may as
well just be a probe deferral.

Signed-off-by: Marcel Ziswiler 

---

 drivers/gpu/drm/tegra/hdmi.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0082468f703c..94c182dbb6d0 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1693,14 +1693,16 @@ static int tegra_hdmi_probe(struct platform_device 
*pdev)
 
hdmi->hdmi = devm_regulator_get(&pdev->dev, "hdmi");
if (IS_ERR(hdmi->hdmi)) {
-   dev_err(&pdev->dev, "failed to get HDMI regulator\n");
-   return PTR_ERR(hdmi->hdmi);
+   err = PTR_ERR(hdmi->hdmi);
+   dev_err(&pdev->dev, "failed to get HDMI regulator: %d\n", err);
+   return err;
}
 
hdmi->pll = devm_regulator_get(&pdev->dev, "pll");
if (IS_ERR(hdmi->pll)) {
-   dev_err(&pdev->dev, "failed to get PLL regulator\n");
-   return PTR_ERR(hdmi->pll);
+   err = PTR_ERR(hdmi->pll);
+   dev_err(&pdev->dev, "failed to get PLL regulator: %d\n", err);
+   return err;
}
 
hdmi->vdd = devm_regulator_get(&pdev->dev, "vdd");
-- 
2.14.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

2018-07-19 Thread Marcel Ziswiler
On Wed, 2018-07-18 at 11:12 +0100, Daniel Thompson wrote:
> On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote:
> > On Wed, 18 Jul 2018, Marcel Ziswiler wrote:
> > 
> > > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> > > > On Mon, 16 Jul 2018, Daniel Thompson wrote:
> > > > 
> > > > > Currently, if the DT does not define num-interpolated-steps
> > > > > then
> > > > > num_steps is undefined and the interpolation code will deploy
> > > > > randomly.
> > > > > Fix this.
> > > > > 
> > > > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation
> > > > > between
> > > > > brightness-levels")
> > > > > Reported-by: Marcel Ziswiler 
> > > > > Signed-off-by: Daniel Thompson 
> > > > > Signed-off-by: Marcel Ziswiler 
> > > > 
> > > > This line is confusing.  Did you guys author this patch
> > > > together?
> > > 
> > > Yes, I reported it and we came to a conclusion together.
> > 
> > It sounds like you need to have all of the tags (except this one).
> > :)
> > 
> >  Reported-by:  for reporting the issue
> >  Suggested-by: for suggesting a resolution
> >  Acked-by: for reviewing it
> >  Tested-by:for testing it
> > 
> > Signed-off-by usually means you either wrote a significant amount
> > of
> > the diffstat or you were part of the submission path.
> 
> He did [I don't object to but wouldn't have used the extra brackets
> you
> brought up ;-) ].

Yes, I take all the blame for the extra brackets. Regardless of having
a masters in CS or not I still prefer too many then too few of them (;-
p).

> > > > My guess is that this line should be dropped and the RB and TB
> > > > tags
> > > > should remain?  If it was reviewed too, perhaps an AB too?
> > > 
> > > I'm OK either way and do not need any explicit authorship to be
> > > expressed for me.
> > 
> > In this instance I suggest keeping Reported-by and Tested-by.
> > 
> > > > > Tested-by: Marcel Ziswiler 
> > > > > ---
> > > > >  drivers/video/backlight/pwm_bl.c | 17 -
> > > > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/video/backlight/pwm_bl.c
> > > > > b/drivers/video/backlight/pwm_bl.c
> > > > > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > @@ -299,15 +299,14 @@ static int
> > > > > pwm_backlight_parse_dt(struct
> > > > > device *dev,
> > > > >* interpolation between each of the values
> > > > > of
> > > > > brightness levels
> > > > >* and creates a new pre-computed table.
> > > > >*/
> > > > > - of_property_read_u32(node, "num-
> > > > > interpolated-
> > > > > steps",
> > > > > -  &num_steps);
> > > > > -
> > > > > - /*
> > > > > -  * Make sure that there is at least two
> > > > > entries in
> > > > > the
> > > > > -  * brightness-levels table, otherwise we
> > > > > can't
> > > > > interpolate
> > > > > -  * between two points.
> > > > > -  */
> > > > > - if (num_steps) {
> > > > > + if ((of_property_read_u32(node, "num-
> > > > > interpolated-
> > > > > steps",
> > > > > +   &num_steps) == 0)
> > > > > &&
> > > > > num_steps) {
> > > > 
> > > > This is pretty ugly, and isn't it suffering from over-
> > > > bracketing?  My
> > > > suggestion would be to break out the invocation of
> > > > of_property_read_u32() from the if and test only the result.
> > > > 
> > > > of_property_read_u32(node, "num-interpolated-
> > > > steps", 
> > > > &num_steps);
> > > 
> > > you mean:
> > > 
> > >   ret = of_property_read_u32(node, "num-interpolated-
> > > steps", &num_steps);
> > > 
> > > > if (!ret && num_steps) {
> > > > 
> > > > I haven't checked the underling code, but is it even feasible
> > > > for
> > > > of_property_read_u32() to not succeed AND for num_steps to be
> > > > set?
> > > > 
> > > > If not, the check for !ret if superfluous and you can drop it.
> > > 
> > > No, then we are back to the initial issue of num_steps
> > > potentially not
> > > being initialised. We really want both of_property_read_u32() to
> > > succeed AND num_steps to actually be set.
> > 
> > I also think num_steps should be pre-initialised.

Yes, I guess it definitely does not hurt.

> > Then it will only be set if of_property_read_u32() succeeds.

Yes, but we still need to check for both, the function not failing and
num_steps to actually be non zero.

> > -- 
> > Lee Jones [李琼斯]
> > Linaro Services Technical Lead
> > Linaro.org │ Open source software for ARM SoCs
> > Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

2018-07-19 Thread Marcel Ziswiler
On Wed, 2018-07-18 at 14:08 +0100, Lee Jones wrote:
> On Wed, 18 Jul 2018, Marcel Ziswiler wrote:
> 
> > On Wed, 2018-07-18 at 11:12 +0100, Daniel Thompson wrote:
> > > On Wed, Jul 18, 2018 at 10:53:35AM +0100, Lee Jones wrote:
> > > > On Wed, 18 Jul 2018, Marcel Ziswiler wrote:
> > > > 
> > > > > On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> > > > > > On Mon, 16 Jul 2018, Daniel Thompson wrote:
> > > > > > 
> > > > > > > Currently, if the DT does not define num-interpolated-
> > > > > > > steps
> > > > > > > then
> > > > > > > num_steps is undefined and the interpolation code will
> > > > > > > deploy
> > > > > > > randomly.
> > > > > > > Fix this.
> > > > > > > 
> > > > > > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear
> > > > > > > interpolation
> > > > > > > between
> > > > > > > brightness-levels")
> > > > > > > Reported-by: Marcel Ziswiler  > > > > > > >
> > > > > > > Signed-off-by: Daniel Thompson  > > > > > > g>
> > > > > > > Signed-off-by: Marcel Ziswiler  > > > > > > om>
> > > > > > 
> > > > > > This line is confusing.  Did you guys author this patch
> > > > > > together?
> > > > > 
> > > > > Yes, I reported it and we came to a conclusion together.
> > > > 
> > > > It sounds like you need to have all of the tags (except this
> > > > one).
> > > > :)
> > > > 
> > > >  Reported-by:  for reporting the issue
> > > >  Suggested-by: for suggesting a resolution
> > > >  Acked-by: for reviewing it
> > > >  Tested-by:for testing it
> > > > 
> > > > Signed-off-by usually means you either wrote a significant
> > > > amount
> > > > of
> > > > the diffstat or you were part of the submission path.
> > > 
> > > He did [I don't object to but wouldn't have used the extra
> > > brackets
> > > you
> > > brought up ;-) ].
> > 
> > Yes, I take all the blame for the extra brackets. Regardless of
> > having
> > a masters in CS or not I still prefer too many then too few of them
> > (;-
> > p).
> > 
> > > > > > My guess is that this line should be dropped and the RB and
> > > > > > TB
> > > > > > tags
> > > > > > should remain?  If it was reviewed too, perhaps an AB too?
> > > > > 
> > > > > I'm OK either way and do not need any explicit authorship to
> > > > > be
> > > > > expressed for me.
> > > > 
> > > > In this instance I suggest keeping Reported-by and Tested-by.
> > > > 
> > > > > > > Tested-by: Marcel Ziswiler 
> > > > > > > ---
> > > > > > >  drivers/video/backlight/pwm_bl.c | 17 -
> > > > > > >  1 file changed, 8 insertions(+), 9 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/video/backlight/pwm_bl.c
> > > > > > > b/drivers/video/backlight/pwm_bl.c
> > > > > > > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > > > > > > --- a/drivers/video/backlight/pwm_bl.c
> > > > > > > +++ b/drivers/video/backlight/pwm_bl.c
> > > > > > > @@ -299,15 +299,14 @@ static int
> > > > > > > pwm_backlight_parse_dt(struct
> > > > > > > device *dev,
> > > > > > >* interpolation between each of the
> > > > > > > values
> > > > > > > of
> > > > > > > brightness levels
> > > > > > >* and creates a new pre-computed table.
> > > > > > >*/
> > > > > > > - of_property_read_u32(node, "num-
> > > > > > > interpolated-
> > > > > > > steps",
> > > > > > > -  &num_steps);
> > > > > > > -
> > > > > > > - /*
> > > > > > > -  * Make sure that there is at least two
> > > 

Re: [PATCH] backlight: pwm_bl: Fix uninitialized variable

2018-07-19 Thread Marcel Ziswiler
On Wed, 2018-07-18 at 09:09 +0100, Lee Jones wrote:
> On Mon, 16 Jul 2018, Daniel Thompson wrote:
> 
> > Currently, if the DT does not define num-interpolated-steps then
> > num_steps is undefined and the interpolation code will deploy
> > randomly.
> > Fix this.
> > 
> > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation
> > between
> > brightness-levels")
> > Reported-by: Marcel Ziswiler 
> > Signed-off-by: Daniel Thompson 
> > Signed-off-by: Marcel Ziswiler 
> 
> This line is confusing.  Did you guys author this patch together?

Yes, I reported it and we came to a conclusion together.

> My guess is that this line should be dropped and the RB and TB tags
> should remain?  If it was reviewed too, perhaps an AB too?

I'm OK either way and do not need any explicit authorship to be
expressed for me.

> > Tested-by: Marcel Ziswiler 
> > ---
> >  drivers/video/backlight/pwm_bl.c | 17 -
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/pwm_bl.c
> > b/drivers/video/backlight/pwm_bl.c
> > index 9ee4c1b735b2..e3c22b79fbcd 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -299,15 +299,14 @@ static int pwm_backlight_parse_dt(struct
> > device *dev,
> >  * interpolation between each of the values of
> > brightness levels
> >  * and creates a new pre-computed table.
> >  */
> > -   of_property_read_u32(node, "num-interpolated-
> > steps",
> > -&num_steps);
> > -
> > -   /*
> > -* Make sure that there is at least two entries in
> > the
> > -* brightness-levels table, otherwise we can't
> > interpolate
> > -* between two points.
> > -*/
> > -   if (num_steps) {
> > +   if ((of_property_read_u32(node, "num-interpolated-
> > steps",
> > + &num_steps) == 0) &&
> > num_steps) {
> 
> This is pretty ugly, and isn't it suffering from over-bracketing?  My
> suggestion would be to break out the invocation of
> of_property_read_u32() from the if and test only the result.
> 
>   of_property_read_u32(node, "num-interpolated-steps", 
> &num_steps);

you mean:

ret = of_property_read_u32(node, "num-interpolated-
steps", &num_steps);

>   if (!ret && num_steps) {
> 
> I haven't checked the underling code, but is it even feasible for
> of_property_read_u32() to not succeed AND for num_steps to be set?
> 
> If not, the check for !ret if superfluous and you can drop it.

No, then we are back to the initial issue of num_steps potentially not
being initialised. We really want both of_property_read_u32() to
succeed AND num_steps to actually be set.

> > +   /*
> > +* Make sure that there is at least two
> > entries in the
> 
> s/is/are/
> 
> > +* brightness-levels table, otherwise we
> > can't
> > +* interpolate
> 
> Why break the line here?

That's probably a remnant of going back and forth plus quoting on the
mailing list.

> > +* between two points.
> > +*/
> > if (data->max_brightness < 2) {
> > dev_err(dev, "can't
> > interpolate\n");
> > return -EINVAL;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [GIT PULL] drm/tegra: Changes for v4.17-rc1

2018-03-22 Thread Marcel Ziswiler
On Mon, 2018-03-19 at 17:52 +0100, Thierry Reding wrote:
> On Mon, Mar 19, 2018 at 02:32:08PM +0000, Marcel Ziswiler wrote:
> > Hi Thierry
> > 
> > On Mon, 2018-03-19 at 10:59 +0100, Thierry Reding wrote:
> > > Hi Dave,
> > > 
> > > The following changes since commit
> > > 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:
> > > 
> > >   Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-
> > > 4.17-
> > > rc1
> > > 
> > > for you to fetch changes up to
> > > 27e92f1f1600c214bf649daddb9b88b68330a8d1:
> > > 
> > >   drm/tegra: prime: Implement ->{begin,end}_cpu_access() (2018-
> > > 03-17
> > > 00:04:20 +0100)
> > > 
> > > Apologies for the delay. I originally wanted to send this out on
> > > Friday
> > > but then ran into a pair of bugs that I thought might be caused
> > > by
> > > this
> > > branch. Turns out that they were both already broken in v4.16-rc1 
> > > and
> > > I
> > > just hadn't tested for the corner case, so I caught them only
> > > very
> > > late
> > > in the release cycle.
> > > 
> > > Anyway, the fixes for that are on the drm/tegra/fixes branch for
> > > which
> > > I sent an updated pull request earlier. The stuff here's fairly
> > > trivial
> > > and incremental improvements.
> > 
> > Both linux-next as of Friday and today as well as your
> > tags/drm/tegra/for-4.17-rc1 fail for me on TK1 as follows:
> > 
> > [3.609146] +V1.05_AVDD_HDMI_PLL: supplied by +V1.05
> > [3.614294] +V3.3_AVDD_HDMI: supplied by +V1.05
> > [3.622078] [ cut here ]
> > [3.626719] WARNING: CPU: 2 PID: 87 at
> > /run/media/zim/Build/Sources/linux-
> > tegra.git/drivers/gpu/drm/drm_fourcc.c:204
> > drm_format_info+0x48/0x50
> > [3.639588] Modules linked in:
> > [3.642673] CPU: 2 PID: 87 Comm: kworker/2:1 Not tainted 4.16.0-
> > rc1
> > #2
> > [3.649213] Hardware name: NVIDIA Tegra SoC (Flattened Device
> > Tree)
> > [3.655495] Workqueue: events deferred_probe_work_func
> > [3.660672] [] (unwind_backtrace) from []
> > (show_stack+0x10/0x14)
> > [3.668430] [] (show_stack) from []
> > (dump_stack+0x8c/0xa0)
> > [3.675684] [] (dump_stack) from []
> > (__warn+0xe0/0xf8)
> > [3.682587] [] (__warn) from []
> > (warn_slowpath_null+0x40/0x48)
> > [3.690189] [] (warn_slowpath_null) from []
> > (drm_format_info+0x48/0x50)
> > [3.698551] [] (drm_format_info) from []
> > (tegra_plane_format_mod_supported+0x14/0x30)
> > [3.708150] [] (tegra_plane_format_mod_supported) from
> > [] (drm_universal_plane_init+0x2ec/0x59c)
> > [3.718703] [] (drm_universal_plane_init) from
> > [] (tegra_dc_init+0x1b8/0x510)
> > [3.727611] [] (tegra_dc_init) from []
> > (host1x_device_init+0x44/0xd0)
> > [3.735821] [] (host1x_device_init) from []
> > (tegra_drm_load+0x228/0x308)
> > [3.744291] [] (tegra_drm_load) from []
> > (drm_dev_register+0x138/0x1d0)
> > [3.752588] [] (drm_dev_register) from []
> > (host1x_drm_probe+0x34/0x58)
> > [3.760883] [] (host1x_drm_probe) from []
> > (driver_probe_device+0x254/0x32c)
> > [3.769612] [] (driver_probe_device) from []
> > (bus_for_each_drv+0x58/0xb8)
> > [3.778145] [] (bus_for_each_drv) from []
> > (__device_attach+0xd0/0x138)
> > [3.786436] [] (__device_attach) from []
> > (bus_probe_device+0x84/0x8c)
> > [3.794645] [] (bus_probe_device) from []
> > (device_add+0x3b4/0x5b8)
> > [3.802599] [] (device_add) from []
> > (host1x_subdev_register+0xac/0xd4)
> > [3.810897] [] (host1x_subdev_register) from
> > []
> > (host1x_client_register+0x108/0x128)
> > [3.820412] [] (host1x_client_register) from
> > []
> > (tegra_hdmi_probe+0x1e4/0x2d0)
> > [3.829406] [] (tegra_hdmi_probe) from []
> > (platform_drv_probe+0x50/0xac)
> > [3.837855] [] (platform_drv_probe) from []
> > (driver_probe_device+0x254/0x32c)
> > [3.846756] [] (driver_probe_device) from []
> > (bus_for_each_drv+0x58/0xb8)
> > [3.855309] [] (bus_for_each_drv) from []
> > (__device_attach+0xd0/0x138)
> > [3.863603] [] (__device_attach) from []
> > (bus_probe_device+0x84/0x8c)
> > [3.871809] [] (bus_prob

Re: [GIT PULL] drm/tegra: Changes for v4.17-rc1

2018-03-22 Thread Marcel Ziswiler
Hi Thierry

On Mon, 2018-03-19 at 10:59 +0100, Thierry Reding wrote:
> Hi Dave,
> 
> The following changes since commit
> 7928b2cbe55b2a410a0f5c1f154610059c57b1b2:
> 
>   Linux 4.16-rc1 (2018-02-11 15:04:29 -0800)
> 
> are available in the Git repository at:
> 
>   git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.17-
> rc1
> 
> for you to fetch changes up to
> 27e92f1f1600c214bf649daddb9b88b68330a8d1:
> 
>   drm/tegra: prime: Implement ->{begin,end}_cpu_access() (2018-03-17
> 00:04:20 +0100)
> 
> Apologies for the delay. I originally wanted to send this out on
> Friday
> but then ran into a pair of bugs that I thought might be caused by
> this
> branch. Turns out that they were both already broken in v4.16-rc1 and
> I
> just hadn't tested for the corner case, so I caught them only very
> late
> in the release cycle.
> 
> Anyway, the fixes for that are on the drm/tegra/fixes branch for
> which
> I sent an updated pull request earlier. The stuff here's fairly
> trivial
> and incremental improvements.

Both linux-next as of Friday and today as well as your
tags/drm/tegra/for-4.17-rc1 fail for me on TK1 as follows:

[3.609146] +V1.05_AVDD_HDMI_PLL: supplied by +V1.05
[3.614294] +V3.3_AVDD_HDMI: supplied by +V1.05
[3.622078] [ cut here ]
[3.626719] WARNING: CPU: 2 PID: 87 at
/run/media/zim/Build/Sources/linux-
tegra.git/drivers/gpu/drm/drm_fourcc.c:204 drm_format_info+0x48/0x50
[3.639588] Modules linked in:
[3.642673] CPU: 2 PID: 87 Comm: kworker/2:1 Not tainted 4.16.0-rc1
#2
[3.649213] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[3.655495] Workqueue: events deferred_probe_work_func
[3.660672] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[3.668430] [] (show_stack) from []
(dump_stack+0x8c/0xa0)
[3.675684] [] (dump_stack) from []
(__warn+0xe0/0xf8)
[3.682587] [] (__warn) from []
(warn_slowpath_null+0x40/0x48)
[3.690189] [] (warn_slowpath_null) from []
(drm_format_info+0x48/0x50)
[3.698551] [] (drm_format_info) from []
(tegra_plane_format_mod_supported+0x14/0x30)
[3.708150] [] (tegra_plane_format_mod_supported) from
[] (drm_universal_plane_init+0x2ec/0x59c)
[3.718703] [] (drm_universal_plane_init) from
[] (tegra_dc_init+0x1b8/0x510)
[3.727611] [] (tegra_dc_init) from []
(host1x_device_init+0x44/0xd0)
[3.735821] [] (host1x_device_init) from []
(tegra_drm_load+0x228/0x308)
[3.744291] [] (tegra_drm_load) from []
(drm_dev_register+0x138/0x1d0)
[3.752588] [] (drm_dev_register) from []
(host1x_drm_probe+0x34/0x58)
[3.760883] [] (host1x_drm_probe) from []
(driver_probe_device+0x254/0x32c)
[3.769612] [] (driver_probe_device) from []
(bus_for_each_drv+0x58/0xb8)
[3.778145] [] (bus_for_each_drv) from []
(__device_attach+0xd0/0x138)
[3.786436] [] (__device_attach) from []
(bus_probe_device+0x84/0x8c)
[3.794645] [] (bus_probe_device) from []
(device_add+0x3b4/0x5b8)
[3.802599] [] (device_add) from []
(host1x_subdev_register+0xac/0xd4)
[3.810897] [] (host1x_subdev_register) from []
(host1x_client_register+0x108/0x128)
[3.820412] [] (host1x_client_register) from []
(tegra_hdmi_probe+0x1e4/0x2d0)
[3.829406] [] (tegra_hdmi_probe) from []
(platform_drv_probe+0x50/0xac)
[3.837855] [] (platform_drv_probe) from []
(driver_probe_device+0x254/0x32c)
[3.846756] [] (driver_probe_device) from []
(bus_for_each_drv+0x58/0xb8)
[3.855309] [] (bus_for_each_drv) from []
(__device_attach+0xd0/0x138)
[3.863603] [] (__device_attach) from []
(bus_probe_device+0x84/0x8c)
[3.871809] [] (bus_probe_device) from []
(deferred_probe_work_func+0x4c/0x148)
[3.880885] [] (deferred_probe_work_func) from
[] (process_one_work+0x1ec/0x55c)
[3.890047] [] (process_one_work) from []
(worker_thread+0x29c/0x598)
[3.898237] [] (worker_thread) from []
(kthread+0x14c/0x154)
[3.905662] [] (kthread) from []
(ret_from_fork+0x14/0x2c)
[3.912901] Exception stack(0xee2b1fb0 to 0xee2b1ff8)
[3.917958] 1fa0: 
  
[3.926153] 1fc0:     
  
[3.934348] 1fe0:     0013

[3.941050] ---[ end trace f2913c9fb893aca6 ]---
...
[4.594476] Unable to handle kernel NULL pointer dereference at
virtual address 0005
[4.602584] pgd = b237c3d6
[4.605293] [0005] *pgd=
[4.608895] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[4.614209] Modules linked in:
[4.617274] CPU: 2 PID: 87 Comm: kworker/2:1 Tainted:
GW4.16.0-rc1 #2
[4.625105] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[4.631376] Workqueue: events deferred_probe_work_func
[4.636525] PC is at tegra_plane_format_mod_supported+0x18/0x30
[4.642448] LR is at warn_slowpath_null+0x40/0x48
[4.647153] pc : []lr : []psr: 2113
[4.653419] sp : ee2b1be0  ip 

[PATCH v2] drm/tegra: dc: use correct format array for Tegra124.

2018-03-22 Thread Marcel Ziswiler
From: Stefan Agner 

Use tegra124_(primary|overlay)_formats for Tegra124.

Fixes: 511c7023cf23 ("drm/tegra: dc: Support more formats")

Signed-off-by: Stefan Agner 
Acked-by: Marcel Ziswiler 

---

Changes in v2:
- re-based on top of today's next
- added my ACK

 drivers/gpu/drm/tegra/dc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 71152776b04c..e0ea5a46ecc9 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -2015,9 +2015,9 @@ static const struct tegra_dc_soc_info 
tegra124_dc_soc_info = {
.coupled_pm = false,
.has_nvdisplay = false,
.num_primary_formats = ARRAY_SIZE(tegra124_primary_formats),
-   .primary_formats = tegra114_primary_formats,
+   .primary_formats = tegra124_primary_formats,
.num_overlay_formats = ARRAY_SIZE(tegra124_overlay_formats),
-   .overlay_formats = tegra114_overlay_formats,
+   .overlay_formats = tegra124_overlay_formats,
.modifiers = tegra124_modifiers,
 };
 
-- 
2.14.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] drm/tegra: fb: Implement ->fb_mmap() callback

2018-02-08 Thread Marcel Ziswiler
On Wed, 2018-02-07 at 18:45 +0100, Thierry Reding wrote:
> From: Thierry Reding 
> 
> This fixes hangs with legacy applications that use the mmap() syscall
> on
> the fbdev device to map framebuffer memory. The fbdev implementation
> for
> mmap() creates a mapping that conflicts with DRM usage and causes a
> hang
> when the memory is accessed through the mapping.

With that legacy Qt4e applications run just fine now on Apalis TK1
running Linux kernel 4.14.18.

Thanks, Thierry!

Tested-by: Marcel Ziswiler 

Reported-by: Marcel Ziswiler 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/tegra/fb.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index 001cb77e2f59..0786159edef3 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -224,12 +224,28 @@ struct drm_framebuffer *tegra_fb_create(struct
> drm_device *drm,
>  }
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> +static int tegra_fb_mmap(struct fb_info *info, struct vm_area_struct
> *vma)
> +{
> + struct drm_fb_helper *helper = info->par;
> + struct tegra_bo *bo;
> + int err;
> +
> + bo = tegra_fb_get_plane(helper->fb, 0);
> +
> + err = drm_gem_mmap_obj(&bo->gem, bo->gem.size, vma);
> + if (err < 0)
> + return err;
> +
> + return __tegra_gem_mmap(&bo->gem, vma);
> +}
> +
>  static struct fb_ops tegra_fb_ops = {
>   .owner = THIS_MODULE,
>   DRM_FB_HELPER_DEFAULT_OPS,
>   .fb_fillrect = drm_fb_helper_sys_fillrect,
>   .fb_copyarea = drm_fb_helper_sys_copyarea,
>   .fb_imageblit = drm_fb_helper_sys_imageblit,
> + .fb_mmap = tegra_fb_mmap,
>  };
>  
>  static int tegra_fbdev_probe(struct drm_fb_helper *helper,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


imx-drm: regression due to commit 503fe87bd0a8 ("gpu: ipu-v3: Fix imx-ipuv3-crtc module autoloading")

2016-05-17 Thread Marcel Ziswiler
Hi there

On Fri, 2016-05-13 at 12:33 +0200, Lothar Waßmann wrote:
> Hi,
> 
> the commit 503fe87bd0a8 ("gpu: ipu-v3: Fix imx-ipuv3-crtc module
> autoloading")
> indeed fixes the autoloading issue, but completely breaks the driver
> in
> non-modular mode (at least with the parallel-display driver I didn't
> yet check with the imx-ldb driver.
> Can anyone confirm that the imx-drm driver in current linux-next
> (next-20160512) works for them with any i.MX6 or i.MX53 board?

I can confirm that this is actually broken both in v4.6 and next-
20160517. Reverting commit 407c9eba7897 ("drm/imx: Remove of_node
assignment from ipuv3-crtc driver probe") in both cases makes regular
imx_v6_v7_defconfig work again.

> I'm always very suspicious when seeing code moving of_node's from
> one device to another or assigning of_node's to platform devices that
> weren't instantiated via DT.
> 
> 
> Lothar Waßmann

Cheers

Marcel

> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel