Re: [PATCH v3 0/4] drm: lcdif: Improve YUV support

2022-10-15 Thread Marek Vasut

On 9/29/22 22:42, Laurent Pinchart wrote:

Hello,

This small patch series improves YUV support in the i.MX8MP LCDIF
driver. After patches 1/4 and 2/4 that fix tiny cosmetic issues, patch
3/4 fixes YUV quantization range for the RGB to YUV conversion. Patch
4/4 addresses the other direction and adds support for YUV planes.

Please see individual patches for details.

Compared to v2, review comments have been taken into account, and the
YUV to RGB coefficients in patch 4/4 have been fixed (they were
blatantly wrong due to a stupid mistake).

The series has been tested on a Polyhex Debix board with the currently
out-of-tree HDMI encoder support patches developed by Lucas Stach, with
the kmstest and the libcamera 'cam' applications.

There is a know issue with the way the driver programs the format and
pitch, which produces incorrect output when testing YUV formats with the
legacy (non-atomic) KMS API, in particular with the modetest
application. The framebuffer is accessed from the plane state in
function called from the .atomic_enable() handler, which in some
circumstances results in the format and/or pitch of the old frame buffer
being used. This issue preexists, and can be triggered by selecting a
different RGB format with modetest (XR15 for instance). It should be
fixed separately, and I wouldn't consider it as a blocker for this
series as YUV formats can already be used correctly when using the KMS
atomic API.

Kieran Bingham (1):
   drm: lcdif: Add support for YUV planes

Laurent Pinchart (3):
   drm: lcdif: Fix indentation in lcdif_regs.h
   drm: lcdif: Don't use BIT() for multi-bit register fields
   drm: lcdif: Switch to limited range for RGB to YUV conversion

  drivers/gpu/drm/mxsfb/lcdif_kms.c  | 232 ++---
  drivers/gpu/drm/mxsfb/lcdif_regs.h |  37 ++---
  2 files changed, 229 insertions(+), 40 deletions(-)


Applied all, thanks


Re: [PATCH v3 2/4] dt-bindings: display: add new bus-format property for panel-dpi

2022-10-15 Thread Laurent Pinchart
Hello,

On Fri, Oct 14, 2022 at 03:08:49PM +0100, Dave Stevenson wrote:
> On Thu, 13 Oct 2022 at 13:58, Francesco Dolcini wrote:
> > On Tue, Jun 28, 2022 at 08:18:36PM +0200, Max Krummenacher wrote:
> > > From: Max Krummenacher 
> > >
> > > The property is used to set the enum bus_format and infer the bpc
> > > for a panel defined by 'panel-dpi'.
> > > This specifies how the panel is connected to the display interface.
> > >
> > > Signed-off-by: Max Krummenacher 
> > >
> >
> > 
> >
> > > diff --git 
> > > a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml 
> > > b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > index dae0676b5c6e..52f5db03b6a8 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > @@ -26,7 +26,28 @@ properties:
> > >height-mm: true
> > >label: true
> > >panel-timing: true
> > > -  port: true
> > > +
> > > +  port:
> > > +$ref: /schemas/graph.yaml#/$defs/port-base
> > > +description:
> > > +  Input port node, receives the panel data.
> > > +
> > > +properties:
> > > +  endpoint:
> > > +$ref: /schemas/graph.yaml#/$defs/endpoint-base
> > > +
> > > +properties:
> > > +  bus-format:
> > > +$ref: /schemas/types.yaml#/definitions/uint32
> > > +minimum: 0x1001
> > > +maximum: 0x1fff
> > > +description: |
> > > +  Describes how the display panel is connected to the 
> > > display interface.
> > > +  Valid values are defined in 
> > > .
> > > +  The mapping between the color/significance of the panel 
> > > lines to the
> > > +  parallel data lines are defined in:
> > > +  
> > > https://www.kernel.org/doc/html/v5.17/userspace-api/media/v4l/subdev-formats.html#packed-rgb-formats
> > > +
> >
> > Last month I had the chance to talk in person about this topic with
> > Dave, Marek and Max in Dublin.
> >
> > My understanding is that this change is addressing a general need, Dave
> > confirmed me they have a downstream patch for raspberrypi [1].
> >
> > From what I could tell the only concern is about the actual encoding of
> > this `bus-format` property.
> >
> > I am personally convinced that a simple enum is the way to go, I think
> > that Marek proposal is adding complexity and not flexibility (from my
> > understanding Dave is on the same page, just correct me if I
> > misunderstood you).
> 
> Yes I agree with you here.
> 
> This binding is for the panel, and currently the only path to pass the
> panel mode to the DPI transmitter is one or more MEDIA_BUS_FMT_* enums
> in struct drm_display_info *bus_formats.
> 
> Looking at Marek's comment over DSI and data-lanes, yes both source
> and sink could advertise a data-lanes property to cover the condition
> where they aren't wired up in a 1:1 fashion. Reality is that most
> drivers don't support reordering the lanes - looking at the bindings,
> only one (msm) documents the use of data-lanes on the host side.
> rcar_mipi_dsi looks at the number of lanes specified only, and then
> checks that the number requested by the device is <= the number
> configured.
> 
> As I see it, the comparison here is that this "bus-format" property is
> the equivalent of the data-lanes on the sink, and is the desired
> number of lanes value passed from sink to source (one integer, not a
> mapping).
> If the source can reorder the lanes, then that is a property of the
> source. This binding is for the sink, and so isn't a reasonable
> comparison. It also doesn't have to be called "bus-format" on the
> source, and can take a totally different form.
> I'll admit that I know data-lane configuration more from CSI2, but
> within V4L2 it is the node that can support reordering that should
> have the lanes in a non-incrementing order, and that is normally the
> SoC rather than the sensor. The same would seem to apply here - it's
> the SoC that can remap the signals, not the panel.
> 
> It could be argued that for DPI the panel should only advertise the
> panel's bit depth for each channel, not the padding. The panel is
> generic and could handle any wiring/padding options, and it isn't
> necessarily a simple 16/18/24/32 bit bus representation, just a
> collection of N wires.
> Padding and wiring is a function of the DPI transmitter / SoC, or
> potentially an interconnect node between the two.

Sooo... I'm not sure where to start :-)

I think the trouble when describing the connection between a source and
a sink in DT is that none of the source or sink is an ideal place to
describe properties of the connection.

For DSI we have it relatively easy, as we only have to describe the
number of lanes that are routed on the board and possibly how the lanes
are rearranged. The former is a value that is common between the source
and the sink, that's the easiest case, it can be specified in 

[PATCH] drm/bridge: tc358767: Enable DSI burst mode, LPM, non-continuous clock

2022-10-15 Thread Marek Vasut
The TC358767/TC358867/TC9595 are capable of DSI burst mode, which
is more energy efficient than the non-burst modes. Make use of it.

The TC358767/TC358867/TC9595 are capable of DSI non-continuous clock,
since it sources the internal PLL clock from external clock source.
The DSI non-continuous clock further reduces power utilization.

The TC358767/TC358867/TC9595 may use DSI LPM for command transmissions,
make sure this is configured correctly in the DSI mode flags.

Signed-off-by: Marek Vasut 
---
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/tc358767.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 67f294f96e823..b5f4e5328eaf9 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1899,10 +1899,10 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc)
 "failed to create dsi device\n");
 
tc->dsi = dsi;
-
dsi->lanes = dsi_lanes;
dsi->format = MIPI_DSI_FMT_RGB888;
-   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
ret = mipi_dsi_attach(dsi);
if (ret < 0) {
-- 
2.35.1



[PATCH] drm/bridge: tc358767: Set default CLRSIPO count

2022-10-15 Thread Marek Vasut
The current CLRSIPO count is still marginal and does not work with high
DSI clock rates in burst mode. Increase it further to allow the DSI link
to work at up to 1Gbps lane speed. This returns the counts to defaults
as provided by datasheet.

Fixes: ea6490b02240b ("drm/bridge: tc358767: increase CLRSIPO count")
Signed-off-by: Marek Vasut 
---
Cc: Jonas Karlman 
Cc: Laurent Pinchart 
Cc: Lucas Stach 
Cc: Maxime Ripard 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/bridge/tc358767.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c 
b/drivers/gpu/drm/bridge/tc358767.c
index 98cfb50a83bec..67f294f96e823 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -1267,10 +1267,10 @@ static int tc_dsi_rx_enable(struct tc_data *tc)
u32 value;
int ret;
 
-   regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 5);
-   regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 5);
-   regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 5);
-   regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 5);
+   regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 25);
+   regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 25);
+   regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 25);
+   regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 25);
regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
-- 
2.35.1



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

2022-10-15 Thread Marek Vasut
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 
---
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] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5433] = _dsi_driver_data,
[SAMSUNG_DSIM_TYPE_IMX8MM] = _dsi_driver_data,
+   [SAMSUNG_DSIM_TYPE_IMX8MP] = _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 = _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,
 };
 
-static const struct drm_bridge_timings samsung_dsim_bridge_timings = {
+static const struct drm_bridge_timings samsung_dsim_bridge_timings_de_high = {
+   .input_bus_flags = DRM_BUS_FLAG_DE_HIGH,
+};
+
+static const struct drm_bridge_timings samsung_dsim_bridge_timings_de_low = {
.input_bus_flags = DRM_BUS_FLAG_DE_LOW,
 };
 
@@ -1733,9 +1747,17 @@ int samsung_dsim_probe(struct platform_device *pdev)
 
dsi->bridge.funcs = _dsim_bridge_funcs;
dsi->bridge.of_node = dev->of_node;
-   dsi->bridge.timings = _dsim_bridge_timings;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
 
+   /*
+* The i.MX8M Mini/Nano LCDIF-DSIM glue logic inverts DE signal
+* polarity, see comment in samsung_dsim_atomic_check().
+*/
+   if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM)
+   dsi->bridge.timings = _dsim_bridge_timings_de_low;
+   else
+   dsi->bridge.timings = 

Re: [PATCH v7 09/10] dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

Samsung MIPI DSIM bridge can also be found in i.MX8MM SoC.

Add dt-bingings for it.

v7, v6, v5, v4:
* none

v3:
* collect Rob Acked-by

v2:
* updated comments

v1:
* new patch

Acked-by: Rob Herring 
Signed-off-by: Jagan Teki 
---
  Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt | 1 +
  1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt 
b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
index be377786e8cd..8efcf4728e0b 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
@@ -7,6 +7,7 @@ Required properties:
"samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs 
*/
"samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */
"samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */
+   "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini SoCs */


This also covers Nano, can you add it into the comment ?


Re: [PATCH v7 07/10] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

[...]


@@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct 
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
  }
  
+#define MAX_INPUT_SEL_FORMATS	1

+
+static u32 *
+samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+  struct drm_bridge_state *bridge_state,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state *conn_state,
+  u32 output_fmt,
+  unsigned int *num_input_fmts)
+{
+   u32 *input_fmts;
+
+   *num_input_fmts = 0;
+
+   input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+GFP_KERNEL);
+   if (!input_fmts)
+   return NULL;
+
+   /* This is the DSI-end bus format */
+   input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+   *num_input_fmts = 1;


Is this the only supported format ? NXP AN13573 lists the following:

i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
3.7.4 Pixel formats
Table 14. DSI pixel packing formats

Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
Packed Pixel Stream, 24-bit YCbCr, 4:2:2
Packed Pixel Stream, 16-bit YCbCr, 4:2:2
Packed Pixel Stream, 30-bit RGB, 10-10-10
Packed Pixel Stream, 36-bit RGB, 12-12-12
Packed Pixel Stream, 12-bit YCbCr, 4:2:0
Packed Pixel Stream, 16-bit RGB, 5-6-5
Packed Pixel Stream, 18-bit RGB, 6-6-6
Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
Packed Pixel Stream, 24-bit RGB, 8-8-8 Format

The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP 
LCDIFv3 can also generate the 16bit YCbCr .


It seems there should be more formats here.


Re: [PATCH v7 06/10] drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

Look like PLL PMS_P offset value varies between platforms that have
Samsung DSIM IP.

However, there is no clear evidence for it as both Exynos and i.MX
8M Mini Application Processor Reference Manual is still referring
the PMS_P offset as 13.

The offset 13 is not working for i.MX8M Mini SoCs but the downstream
NXP sec-dsim.c driver is using offset 14 for i.MX8M Mini SoC platforms
[1] [2].

PMS_P value set in sec_mipi_dsim_check_pll_out using PLLCTRL_SET_P()
with offset 13 and then an additional offset of one bit added in
sec_mipi_dsim_config_pll via PLLCTRL_SET_PMS().

Not sure whether it is reference manual documentation or something else
but this patch trusts the downstream code and handle PLL_P offset via
platform driver data so-that imx8mm driver data shall use pll_p_offset
to 14.

[1] 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n210
[2] 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n211


You also add to the commit message that all of 8M Mini/Nano/Plus have 
P=14 , unlike Exynos. It also seems like the PLL in the DSIM and the 
clock PLLs in the 8M are the same design from Samsung.


Reviewed-by: Marek Vasut 


Re: [PATCH v7 03/10] drm: bridge: samsung-dsim: Mark PHY as optional

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

In i.MX8M Mini/Nano SoC the DSI Phy requires a MIPI DPHY bit


8M Plus too.


to reset in order to activate the PHY and that can be done via
upstream i.MX8M blk-ctrl driver.

So, mark the phy get as optional.


Reviewed-by: Marek Vasut 


Re: [PATCH v7 02/10] drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

The child devices in MIPI DSI can be binding with OF-graph
and also via child nodes.

The OF-graph interface represents the child devices via
remote and associated endpoint numbers like

dsi {
compatible = "fsl,imx8mm-mipi-dsim";

ports {
port@0 {
 reg = <0>;

 dsi_in_lcdif: endpoint@0 {
  reg = <0>;
  remote-endpoint = <_out_dsi>;
 };
};

port@1 {
 reg = <1>;

 dsi_out_bridge: endpoint {
  remote-endpoint = <_in_dsi>;
 };
};
};

The child node interface represents the child devices via
conventional child nodes on given DSI parent like

dsi {
compatible = "samsung,exynos5433-mipi-dsi";

ports {
 port@0 {
  reg = <0>;

  dsi_to_mic: endpoint {
   remote-endpoint = <_to_dsi>;
  };
 };
};

panel@0 {
 reg = <0>;
};
};

As Samsung DSIM bridge is common DSI IP across all Exynos DSI
and NXP i.MX8M host controllers, this patch adds support to
lookup the child devices whether its bindings on the associated
host represent OF-graph or child node interfaces.


This looks like a good candidate for common/helper code which can be 
reused by other similar drivers.


Re: [PATCH v7 01/10] drm: bridge: Add Samsung DSIM bridge driver

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

Samsung MIPI DSIM controller is common DSI IP that can be used in various
SoCs like Exynos, i.MX8M Mini/Nano.

In order to access this DSI controller between various platform SoCs,
the ideal way to incorporate this in the drm stack is via the drm bridge
driver.

This patch is trying to differentiate platform-specific and bridge driver
code by maintaining exynos platform glue code in exynos_drm_dsi.c driver
and common bridge driver code in samsung-dsim.c providing that the new
platform-specific glue should be supported in the bridge driver, unlike
exynos platform drm drivers.

- Add samsung_dsim_plat_data for keeping platform-specific attributes like
   host_ops, irq_ops, and hw_type.

- Initialize the plat_data hooks for exynos platform in exynos_drm_dsi.c.

- samsung_dsim_probe is the common probe call across exynos_drm_dsi.c and
   samsung-dsim.c.

- plat_data hooks like host_ops and irq_ops are invoked during the
   respective bridge call chains.


Maybe the Subject should say "Split ... driver" or "Move ... driver" , 
since it is not adding a new driver here ?


Re: [PATCH v7 08/10] drm: bridge: samsung-dsim: Add input_bus_flags

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

[...]


diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index f714e49c1eab..f5cd80641cea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1601,6 +1601,10 @@ static const struct samsung_dsim_host_ops 
samsung_dsim_generic_host_ops = {
.unregister_host = samsung_dsim_unregister_host,
  };
  
+static const struct drm_bridge_timings samsung_dsim_bridge_timings = {

+   .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
+};
+
  int samsung_dsim_probe(struct platform_device *pdev)
  {
struct device *dev = >dev;
@@ -1681,6 +1685,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
  
  	dsi->bridge.funcs = _dsim_bridge_funcs;

dsi->bridge.of_node = dev->of_node;
+   dsi->bridge.timings = _dsim_bridge_timings;


Is this really valid on Exynos too ?

My impression is that the DE_LOW requirement is only needed for MX8MM/MN 
where the LCDIF-DSIM glue logic inverts the HS/VS/DE signals, while on 
all other platforms (at least MX8MP) the inversion does not take place, 
so DE is really active HIGH there.


Re: [PATCH v7 10/10] drm: bridge: samsung-dsim: Add i.MX8MM support

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

[...]


+static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
+   .reg_ofs = exynos5433_reg_ofs,
+   .plltmr_reg = 0xa0,
+   .has_clklane_stop = 1,
+   .num_clks = 2,
+   .max_freq = 2100,
+   .wait_for_reset = 0,
+   .num_bits_resol = 12,
+   /**


Should this really be kerneldoc style /** comment or plain /* comment ?
If the later, please fix globally, there are multiple such FIXME comments.


+* FIXME:
+* Offset value used from downstream drivers/gpu/drm/bridge/sec-dsim.c
+* remove this comment if it is true else update the logic.
+*/
+   .pll_p_offset = 14,
+   .reg_values = imx8mm_dsim_reg_values,
+};


[...]


Re: [PATCH v7 05/10] drm: bridge: samsung-dsim: Add atomic_check

2022-10-15 Thread Marek Vasut

On 10/5/22 17:13, Jagan Teki wrote:

Look like an explicit fixing up of mode_flags is required for DSIM IP
present in i.MX8M Mini/Nano SoCs.

At least the LCDIF + DSIM needs active low sync polarities in order
to correlate the correct sync flags of the surrounding components in
the chain to make sure the whole pipeline can work properly.

On the other hand the 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."

No clear evidence about whether it can be documentation issues or
something, so added a comment FIXME for this and updated the active low
sync polarities using SAMSUNG_DSIM_TYPE_IMX8MM hw_type.


[...]


+static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+   struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   struct drm_display_mode *adjusted_mode = _state->adjusted_mode;
+
+   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);
+   }


It would be good to explain what exactly is going on here in the 
comment, the comment says "Vsync, Hsync, and VDEN are active high 
signals." and the code below does exact opposite and sets NxSYNC flags.


Yes, the MX8MM/MN does need HS/VS/DE active LOW, it is a quirk of that 
MXSFB-DSIM glue logic. The MX8MP needs the exact opposite on all three, 
active HIGH.


It would also be good to mention both MX8MM and MX8MN are affected, not 
only MX8MM.


Re: [PATCH v4 1/2] dt-bindings: it6505: add properties to restrict output bandwidth

2022-10-15 Thread Laurent Pinchart
Hello,

On Fri, Oct 14, 2022 at 03:28:31AM +, allen.c...@ite.com.tw wrote:
> On Friday, October 14, 2022 3:20 AM, Rob Herring wrote:
> > On Thu, Oct 13, 2022 at 02:05:45PM +0300, Laurent Pinchart wrote:
> > > On Thu, Oct 13, 2022 at 06:51:13PM +0800, allen wrote:
> > > > From: allen chen 
> > > > 
> > > > Add properties to restrict dp output data-lanes and clock.
> > > > 
> > > > Signed-off-by: Pin-Yen Lin 
> > > > Signed-off-by: Allen Chen 
> > > > ---
> > > >  .../bindings/display/bridge/ite,it6505.yaml   | 43 +++
> > > >  1 file changed, 43 insertions(+)
> > > > 
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml 
> > > > b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > > > index 833d11b2303a7..f2c3d1d10359e 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
> > > > @@ -52,10 +52,51 @@ properties:
> > > >  maxItems: 1
> > > >  description: extcon specifier for the Power Delivery
> > > >  
> > > > +  data-lanes:
> > > > +oneOf:
> > > > +  - minItems: 1
> > > > +maxItems: 1
> > > > +uniqueItems: true
> > > > +items:
> > > > +  enum:
> > > > +- 0
> > > > +- 1
> > > > +description: For one lane operation.
> > > > +
> > > > +  - minItems: 2
> > > > +maxItems: 2
> > > > +uniqueItems: true
> > > > +items:
> > > > +  enum:
> > > > +- 0
> > > > +- 1
> > > > +description: For two lanes operation.
> > > > +
> > > > +  - minItems: 4
> > > > +maxItems: 4
> > > > +uniqueItems: true
> > > > +items:
> > > > +  enum:
> > > > +- 0
> > > > +- 1
> > > > +- 2
> > > > +- 3
> > > > +description: For four lanes operation.
> > > 
> > > The data lanes should be in the output endpoint. If there's no output 
> > > port, one should be added.
> 
> ==> In this dt-binding, our output point is "extcon" so doesn't have output 
> endpoint.
> I don't know how to add the endpoint.
> If need to add the endpoint to this dt-binding, what is your recommend about 
> adding the endpoint?

You will also need to add a port to the USB-C connector. Then endpoints
can be added to connect the two.

> By the way, Krzysztof Kozlowski  said
> we could put "data-lanes" here.

If I read him correctly, Krzysztof said we have a standard property for
the data lanes (and that's true, we do), but I don't think he implied it
could be put outside of the endpoint (Krzysztof, please correct me if
I'm wrong).

> > > > +
> > > >port:
> > > >  $ref: /schemas/graph.yaml#/properties/port
> > 
> > To fix the error, this must be:
> > 
> > $ref: /schemas/graph.yaml#/$defs/port-base
> > unevaluatedProperties: false
> > 
> > > >  description: A port node pointing to DPI host port node
> > > >  
> > > > +properties:
> > > > +  endpoint:
> > > > +$ref: /schemas/graph.yaml#/$defs/endpoint-base
> > > > +
> > > > +properties:
> > > > +  link-frequencies:
> > > > +minItems: 1
> > > > +maxItems: 1
> > > > +description: Allowed max link frequencies in Hz.
> > > > +
> > > >  required:
> > > >- compatible
> > > >- ovdd-supply
> > > > @@ -84,10 +125,12 @@ examples:
> > > >  pwr18-supply = <_pp18_reg>;
> > > >  reset-gpios = < 179 1>;
> > > >  extcon = <_extcon>;
> > > > +data-lanes = <0 1>;
> > > >  
> > > >  port {
> > > >  it6505_in: endpoint {
> > > >  remote-endpoint = <_out>;
> > > > +link-frequencies = /bits/ 64 <15000>;
> > > >  };
> > > >  };
> > > >  };

-- 
Regards,

Laurent Pinchart


Re: [PATCH v2] drm/omapdrm: Remove unused struct csc_coef_rgb2yuv

2022-10-15 Thread Laurent Pinchart
Hi Yuan,

Thank you for the patch.

On Fri, Oct 14, 2022 at 02:48:10AM +, Yuan Can wrote:
> After commit 64ff18911878, struct csc_coef_rgb2yuv is not used any more
> and can be removed as well.
> 
> Fixes: 64ff18911878 ("drm/omap: Enable COLOR_ENCODING and COLOR_RANGE 
> properties for planes")
> Signed-off-by: Yuan Can 

Reviewed-by: Laurent Pinchart 

> ---
> changes in v2:
> - adjust commit msg
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
> b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 0ee344ebcd1c..aacad5045e95 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -855,11 +855,6 @@ struct csc_coef_yuv2rgb {
>   bool full_range;
>  };
>  
> -struct csc_coef_rgb2yuv {
> - int yr, yg, yb, cbr, cbg, cbb, crr, crg, crb;
> - bool full_range;
> -};
> -
>  static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc,
>   enum omap_plane_id plane,
>   const struct csc_coef_yuv2rgb *ct)

-- 
Regards,

Laurent Pinchart


Re: [PATCH 5/7] drm/vc4: dpi: Support BGR666 formats

2022-10-15 Thread Laurent Pinchart
On Sat, Oct 15, 2022 at 08:26:48PM +0300, Laurent Pinchart wrote:
> Hi Maxime and Joerg,
> 
> Thank you for the patch.
> 
> On Thu, Oct 13, 2022 at 11:56:49AM +0200, Maxime Ripard wrote:
> > From: Joerg Quinten 
> > 
> > The VC4 DPI output can support multiple BGR666 variants, but they were
> > never added to the driver. Let's add the the support for those formats.
> > 
> > Signed-off-by: Joerg Quinten 
> > Signed-off-by: Maxime Ripard 
> 
> Reviewed-by: Laurent Pinchart 
> 
> > ---
> >  drivers/gpu/drm/vc4/vc4_dpi.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> > index 7da3dd1db50e..ecbe4cd87036 100644
> > --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> > @@ -170,10 +170,16 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
> > *encoder)
> > dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR,
> >DPI_ORDER);
> > break;
> > +   case MEDIA_BUS_FMT_BGR666_1X24_CPADHI:
> > +   dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
> > DPI_ORDER);
> > +   fallthrough;

Upon closer inspection of the code, I think you also need

-   dpi_c &= ~DPI_FORMAT_MASK;
+   dpi_c &= ~(DPI_ORDER_MASK | DPI_FORMAT_MASK);

a few lines above.

> > case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> > dpi_c |= 
> > VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_2,
> >DPI_FORMAT);
> > break;
> > +   case MEDIA_BUS_FMT_BGR666_1X18:
> > +   dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
> > DPI_ORDER);
> > +   fallthrough;
> > case MEDIA_BUS_FMT_RGB666_1X18:
> > dpi_c |= 
> > VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1,
> >DPI_FORMAT);
> > 

-- 
Regards,

Laurent Pinchart


Re: [PATCH 4/7] drm/vc4: dpi: Support RGB565 format

2022-10-15 Thread Laurent Pinchart
Hi Maxime and Chris,

Thank you for the patch.

On Thu, Oct 13, 2022 at 11:56:48AM +0200, Maxime Ripard wrote:
> From: Chris Morgan 
> 
> The RGB565 format with padding over 24 bits
> (MEDIA_BUS_FMT_RGB565_1X24_CPADHI) is supported by the vc4 DPI
> controller as "mode 3".  This is what the Geekworm MZP280 DPI display

The code below uses DPI_FORMAT_16BIT_565_RGB_2. Is that mode 3, or
should the commit message refer to mode 2 ?

With this fixed,

Reviewed-by: Laurent Pinchart 

> uses, so let's add support for it in the DPI controller driver.
> 
> Reviewed-by: Dave Stevenson 
> Signed-off-by: Chris Morgan 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 1f8f44b7b5a5..7da3dd1db50e 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -182,6 +182,10 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
> *encoder)
>   dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3,
>  DPI_FORMAT);
>   break;
> + case MEDIA_BUS_FMT_RGB565_1X24_CPADHI:
> + dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_2,
> +DPI_FORMAT);
> + break;
>   default:
>   DRM_ERROR("Unknown media bus format %d\n",
> bus_format);
> 

-- 
Regards,

Laurent Pinchart


Re: [PATCH 5/7] drm/vc4: dpi: Support BGR666 formats

2022-10-15 Thread Laurent Pinchart
Hi Maxime and Joerg,

Thank you for the patch.

On Thu, Oct 13, 2022 at 11:56:49AM +0200, Maxime Ripard wrote:
> From: Joerg Quinten 
> 
> The VC4 DPI output can support multiple BGR666 variants, but they were
> never added to the driver. Let's add the the support for those formats.
> 
> Signed-off-by: Joerg Quinten 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 7da3dd1db50e..ecbe4cd87036 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -170,10 +170,16 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
> *encoder)
>   dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR,
>  DPI_ORDER);
>   break;
> + case MEDIA_BUS_FMT_BGR666_1X24_CPADHI:
> + dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
> DPI_ORDER);
> + fallthrough;
>   case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
>   dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_2,
>  DPI_FORMAT);
>   break;
> + case MEDIA_BUS_FMT_BGR666_1X18:
> + dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, 
> DPI_ORDER);
> + fallthrough;
>   case MEDIA_BUS_FMT_RGB666_1X18:
>   dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1,
>  DPI_FORMAT);
> 

-- 
Regards,

Laurent Pinchart


Re: [PATCH 7/7] drm/vc4: dpi: Fix format mapping for RGB565

2022-10-15 Thread Laurent Pinchart
Hi Maxime and Dave,

Thank you for the patch.

On Thu, Oct 13, 2022 at 11:56:51AM +0200, Maxime Ripard wrote:
> From: Dave Stevenson 
> 
> The mapping is incorrect for RGB565_1X16 as it should be
> DPI_FORMAT_18BIT_666_RGB_1 instead of DPI_FORMAT_18BIT_666_RGB_3.

The driver includes the following macro definitions and comments:

/* Outputs rggb */
# define DPI_FORMAT_16BIT_565_RGB_1 1
/* Outputs 000r00gg000b */
# define DPI_FORMAT_16BIT_565_RGB_2 2
/* Outputs 00r000gg00b0 */
# define DPI_FORMAT_16BIT_565_RGB_3 3

MEDIA_BUS_FMT_RGB565_1X16 is defined as described in
https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#id3:

Bit  | 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
Data | r4 r3 r2 r1 r0 g5 g4 g3 g2 g1 g0 b4 b3 b2 b1 b0 

This seems to match DPI_FORMAT_16BIT_565_RGB_1 indeed.

Reviewed-by: Laurent Pinchart 

> Fixes: 08302c35b59d ("drm/vc4: Add DPI driver")
> Signed-off-by: Dave Stevenson 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index fdae02760b6d..a7bebfa5d5b0 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -185,7 +185,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
> *encoder)
>  DPI_FORMAT);
>   break;
>   case MEDIA_BUS_FMT_RGB565_1X16:
> - dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3,
> + dpi_c |= 
> VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_1,
>  DPI_FORMAT);
>   break;
>   case MEDIA_BUS_FMT_RGB565_1X24_CPADHI:
> 

-- 
Regards,

Laurent Pinchart


Re: [PATCH 6/7] drm/vc4: dpi: Change the default DPI format to being 18bpp, not 24.

2022-10-15 Thread Laurent Pinchart
Hi Maxime (and Dave),

Thank you for the patch.

On Thu, Oct 13, 2022 at 11:56:50AM +0200, Maxime Ripard wrote:
> From: Dave Stevenson 
> 
> DPI hasn't really been used up until now, so the default has
> been meaningless.
> In theory we should be able to pass the desired format for the
> adjacent bridge chip through, but framework seems to be missing
> for that.

Doesn't the bridge infrastructure allow that ? Or maybe this commit
message was written a while ago, before it was possible ?

In any case, it would be nice to use the bus format exposed by the next
bridge in the chain, but that can be done in a subsequent step. The new
default seems reasonable.

Reviewed-by: Laurent Pinchart 

> As the main device to use DPI is the VGA666 or Adafruit Kippah,
> both of which use RGB666, change the default to being RGB666 instead
> of RGB888.
> 
> Signed-off-by: Dave Stevenson 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index ecbe4cd87036..fdae02760b6d 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -150,8 +150,8 @@ static void vc4_dpi_encoder_enable(struct drm_encoder 
> *encoder)
>   }
>   drm_connector_list_iter_end(_iter);
>  
> - /* Default to 24bit if no connector or format found. */
> - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT);
> + /* Default to 18bit if no connector or format found. */
> + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1, DPI_FORMAT);
>  
>   if (connector) {
>   if (connector->display_info.num_bus_formats) {
> 

-- 
Regards,

Laurent Pinchart


Re: [PATCH 2/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X18

2022-10-15 Thread Laurent Pinchart
Hi Maxime,

Thank you for the patch.

On Thu, Oct 13, 2022 at 11:56:46AM +0200, Maxime Ripard wrote:
> From: Joerg Quinten 
> 
> Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X18 supported by the
> RaspberryPi.
> 
> Signed-off-by: Joerg Quinten 
> Signed-off-by: Maxime Ripard 
> ---
>  include/uapi/linux/media-bus-format.h | 3 ++-

New formats need documentation in
Documentation/userspace-api/media/v4l/subdev-formats.rst. Same for 
patches 1/7 and 3/7.

Apart from that, the patch looks good to me.

>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/media-bus-format.h 
> b/include/uapi/linux/media-bus-format.h
> index b0a945eb7040..2ee0b38c0a71 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_FIXED  0x0001
>  
> -/* RGB - next is 0x1023 */
> +/* RGB - next is 0x1024 */
>  #define MEDIA_BUS_FMT_RGB444_1X120x1016
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002
> @@ -46,6 +46,7 @@
>  #define MEDIA_BUS_FMT_BGR565_2X8_LE  0x1006
>  #define MEDIA_BUS_FMT_RGB565_2X8_BE  0x1007
>  #define MEDIA_BUS_FMT_RGB565_2X8_LE  0x1008
> +#define MEDIA_BUS_FMT_BGR666_1X180x1023
>  #define MEDIA_BUS_FMT_RGB666_1X180x1009
>  #define MEDIA_BUS_FMT_RBG888_1X240x100e
>  #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015
> 

-- 
Regards,

Laurent Pinchart


Re: [PATCH v5 07/22] drm/client: Add some tests for drm_connector_pick_cmdline_mode()

2022-10-15 Thread Noralf Trønnes



Den 13.10.2022 15.18, skrev Maxime Ripard:
> drm_connector_pick_cmdline_mode() is in charge of finding a proper
> drm_display_mode from the definition we got in the video= command line
> argument.
> 
> Let's add some unit tests to make sure we're not getting any regressions
> there.
> 
> Signed-off-by: Maxime Ripard 
> 
> ---
> Changes in v5:
> - Removed useless (for now) count and modes intermediate variables in
>   get_modes
> - Switched to kunit assertions in test init, and to KUNIT_ASSERT_NOT_NULL
>   instead of KUNIT_ASSERT_PTR_NE(..., NULL)
> 
> Changes in v4:
> - Removed MODULE macros
> ---
>  drivers/gpu/drm/drm_client_modeset.c|  4 +
>  drivers/gpu/drm/tests/drm_client_modeset_test.c | 99 
> +
>  2 files changed, 103 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> b/drivers/gpu/drm/drm_client_modeset.c
> index bbc535cc50dd..d553e793e673 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -1237,3 +1237,7 @@ int drm_client_modeset_dpms(struct drm_client_dev 
> *client, int mode)
>   return ret;
>  }
>  EXPORT_SYMBOL(drm_client_modeset_dpms);
> +
> +#ifdef CONFIG_DRM_KUNIT_TEST
> +#include "tests/drm_client_modeset_test.c"
> +#endif

I can't say I like including the file like this, but exporting the
static function for testing isn't attractive either and doing it like
this is shown in the kunit docs, so:

Acked-by: Noralf Trønnes 


Re: [PATCH] dt-bindings: Remove "status" from schema examples, again

2022-10-15 Thread Jonathan Cameron
On Fri, 14 Oct 2022 15:51:04 -0500
Rob Herring  wrote:

> There's no reason to have "status" properties in examples. "okay" is the
> default, and "disabled" turns off some schema checks ('required'
> specifically).
> 
> A meta-schema check for this is pending, so hopefully the last time to
> fix these.
> 
> Fix the indentation in intel,phy-thunderbay-emmc while we're here.
> 
> Signed-off-by: Rob Herring 
Acked-by: Jonathan Cameron  #for-iio

> ---
>  .../arm/tegra/nvidia,tegra-ccplex-cluster.yaml|  1 -
>  .../display/tegra/nvidia,tegra124-dpaux.yaml  |  1 -
>  .../display/tegra/nvidia,tegra186-display.yaml|  2 --
>  .../bindings/iio/addac/adi,ad74413r.yaml  |  1 -
>  .../devicetree/bindings/net/cdns,macb.yaml|  1 -
>  .../devicetree/bindings/net/nxp,dwmac-imx.yaml|  1 -
>  .../bindings/phy/intel,phy-thunderbay-emmc.yaml   | 15 +++
>  7 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra-ccplex-cluster.yaml
>  
> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra-ccplex-cluster.yaml
> index 711bb4d08c60..869c266e7ebc 100644
> --- 
> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra-ccplex-cluster.yaml
> +++ 
> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra-ccplex-cluster.yaml
> @@ -47,5 +47,4 @@ examples:
>compatible = "nvidia,tegra234-ccplex-cluster";
>reg = <0x0e00 0x5>;
>nvidia,bpmp = <>;
> -  status = "okay";
>  };
> diff --git 
> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml 
> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml
> index 9ab123cd2325..5cdbc527a560 100644
> --- 
> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml
> +++ 
> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml
> @@ -128,7 +128,6 @@ examples:
>  resets = <_car 181>;
>  reset-names = "dpaux";
>  power-domains = <_sor>;
> -status = "disabled";
>  
>  state_dpaux_aux: pinmux-aux {
>  groups = "dpaux-io";
> diff --git 
> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
>  
> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
> index 8c0231345529..ce5c673f940c 100644
> --- 
> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
> +++ 
> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
> @@ -138,7 +138,6 @@ examples:
>   < TEGRA186_CLK_NVDISPLAY_DSC>,
>   < TEGRA186_CLK_NVDISPLAYHUB>;
>  clock-names = "disp", "dsc", "hub";
> -status = "disabled";
>  
>  power-domains = < TEGRA186_POWER_DOMAIN_DISP>;
>  
> @@ -227,7 +226,6 @@ examples:
>  clocks = < TEGRA194_CLK_NVDISPLAY_DISP>,
>   < TEGRA194_CLK_NVDISPLAYHUB>;
>  clock-names = "disp", "hub";
> -status = "disabled";
>  
>  power-domains = < TEGRA194_POWER_DOMAIN_DISP>;
>  
> diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml 
> b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> index 03bb90a7f4f8..d2a9f92c0a6d 100644
> --- a/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74413r.yaml
> @@ -114,7 +114,6 @@ examples:
>#size-cells = <0>;
>  
>cs-gpios = < 17 GPIO_ACTIVE_LOW>;
> -  status = "okay";
>  
>ad74413r@0 {
>  compatible = "adi,ad74413r";
> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml 
> b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> index 318f4efe7f6f..bef5e0f895be 100644
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -203,7 +203,6 @@ examples:
>  power-domains = <_firmware PD_ETH_1>;
>  resets = <_reset ZYNQMP_RESET_GEM1>;
>  reset-names = "gem1_rst";
> -status = "okay";
>  phy-mode = "sgmii";
>  phys = < 1 PHY_TYPE_SGMII 1 1>;
>  fixed-link {
> diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml 
> b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> index 4c155441acbf..0270b0ca166b 100644
> --- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> @@ -92,5 +92,4 @@ examples:
>   < IMX8MP_CLK_ENET_QOS>;
>  clock-names = "stmmaceth", "pclk", "ptp_ref", "tx";
>  phy-mode = "rgmii";
> -status = "disabled";
>  };
> diff --git 
> a/Documentation/devicetree/bindings/phy/intel,phy-thunderbay-emmc.yaml 
> b/Documentation/devicetree/bindings/phy/intel,phy-thunderbay-emmc.yaml
> 

[Bug 214425] [drm][amdgpu][TTM] Page pool memory never gets freed

2022-10-15 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=214425

--- Comment #4 from Rafael Ristovski (rafael.ristov...@gmail.com) ---
For what its worth, the following horrible incantation managed to release 2+GB
of TTM buffers on one of my machines, after I purposefully ran a VRAM intensive
game:
> for i in {1..1000}; do cat /sys/kernel/debug/ttm/page_pool_shrink; done

This seems to be the only sysfs mechanism to cause the memory to get released,
and as of now I am not aware of a... better and mainly "cleaner" alternative.

Newer kernel versions seem to feature
https://www.kernel.org/doc/html/next/admin-guide/mm/shrinker_debugfs.html,
which might be a better alternative, but I have not tested it yet, and its
usage is not exactly clear to me.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

RE: [PATCH v8 0/3] Add RZ/G2L DSI driver

2022-10-15 Thread Biju Das
Hi Laurent,

> Subject: Re: [PATCH v8 0/3] Add RZ/G2L DSI driver
> 
> Hi Biju,
> 
> On Sat, Oct 15, 2022 at 01:11:20PM +, Biju Das wrote:
> > Gentle ping.
> 
> I've reviewed v8, everything looks fine. I've applied the patches to
> my tree ([1]) for v6.2.

Thanks,
Biju

> 
> [1] git://linuxtv.org/pinchartl/media.git drm/du/next
> 
> > > Subject: [PATCH v8 0/3] Add RZ/G2L DSI driver
> > >
> > > This patch series aims to support the MIPI DSI encoder found in
> the
> > > RZ/G2L SoC. It currently supports DSI video mode only.
> > >
> > > This unit supports MIPI Alliance Specification for Display Serial
> > > Interface (DSI) Specification. This unit provides a solution for
> > > transmitting MIPI DSI compliant digital video and packets.
> Normative
> > > References are below.
> > > * MIPI Alliance Specification for Display Serial Interface Version
> > > 1.3.1
> > > * MIPI Alliance Specification for D-PHY Version 2.1
> > >
> > > The following are key features of this unit.
> > >
> > > * 1 channel
> > > * The number of Lane: 4-lane
> > > * Support up to Full HD (1920 × 1080), 60 fps (RGB888)
> > > * Maximum Bandwidth: 1.5 Gbps per lane
> > > * Support Output Data Format: RGB666 / RGB888
> > >
> > > This patch series is based on drm_misc and patches from
> drm/du/next
> > > [1]
> > >
> > >
> > > v7->v8:
> > >  * Added Rb tag from Laurent.
> > >  * Added hsfreq_max to struct rzg2l_mipi_dsi_timings.
> > >  * Removed enums rzg2l_mipi_dsi_dphy_timings.
> > >  * Replaced if else with for loop for finding dphy_timings
> > >based on hsfreq.
> > >  * Removed checking "number of lanes capability" from
> rzg2l_mipi_dsi_
> > >startup() and added patch#3 for handling it in probe() and
> enforcing
> > >it in rzg2l_mipi_dsi_host_attach().
> > >  * Added Labels with an "err_" prefix.
> > >   out_pm_put->err_pm_put
> > >   out_assert_rst_and_stop_clocks->err_stop
> > >   out_stop_hs_clock->err_stop_clock
> > >   out_pm_disable->err_pm_disable
> > >  * Added error message for lane check in
> > > rzg2l_mipi_dsi_host_attach()
> > >  * Replaced dev_warn->dev_err for the format error in
> rzg2l_mipi_dsi_host
> > >_attach(). Added missing "\n" and print the format for
> debugging.
> > > v6->v7:
> > >  * Added rzg2l_mipi_dsi_stop() counterpart of
> rzg2l_mipi_dsi_startup().
> > >  * Error labels are named according to the cleanup operation they
> perform.
> > >  * Restored Max lane capability read after dphy timing
> initialization
> > >as per the guide lines from SoC design team.
> > >  * Added recommended lut values for the Global Operation Timing
> > >parameters for MIPI DPHY.
> > > v5->v6:
> > >  * Updated commit description
> > >  * Moved handling of arst and prst from rzg2l_mipi_dsi_startup-
> >runtime
> > >PM suspend/resume handlers.
> > >  * Max lane capability read at probe(), and enforced in
> > >rzg2l_mipi_dsi_host_attach()
> > >  * Simplified vich1ppsetr setting.
> > >  * Renamed hsclk_running_mode,hsclk_mode->is_clk_cont.
> > >  * Fixed typo in probe error message(arst->rst).
> > >  * Reordered DRM bridge initaization in probe()
> > >  * Updated typo in e-mail address.
> > > v4->v5:
> > >  * Added Ack from Sam.
> > >  * Added a trivial change, replaced rzg2l_mipi_dsi_parse_dt()
> > >with drm_of_get_data_lanes_count_ep() in probe.
> > > v3->v4:
> > >  * Updated error handling in rzg2l_mipi_dsi_startup() and
> rzg2l_mipi_dsi_atomic_enable().
> > > v2->v3:
> > >  * Added Rb tag from Geert and Laurent
> > >  * Fixed the typo "Receive" -> "transmit"
> > >  * Added accepible values for data-lanes
> > >  * Sorted Header file in the example
> > >  * Added SoC specific compaible along with generic one.
> > >  * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr
> function instead
> > >of the memory pointer
> > >  * Fixed the comment in rzg2l_mipi_dsi_startup()
> > >  * Removed unnecessary dbg message from
> rzg2l_mipi_dsi_start_video()
> > >  * DRM bridge parameter initialization moved to probe
> > >  * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
> > >  * Inserted the missing blank lane after return in probe()
> > >  * Added missing MODULE_DEVICE_TABLE
> > >  * Added include linux/bits.h in header file
> > >  * Fixed various macros in header file.
> > >  * Reorder the make file for DSI, so that it is no more dependent
> > >on RZ/G2L DU patch series.
> > > v1->v2:
> > >  * Added full path for dsi-controller.yaml
> > >  * Modeled DSI + D-PHY as single block and updated reg property
> > >  * Fixed typo D_PHY->D-PHY
> > >  * Updated description
> > >  * Added interrupts and interrupt-names and updated the example
> > >  * Driver rework based on dt-binding changes (DSI + D-PHY) as
> single
> > > block
> > >  * Replaced link_mmio and phy_mmio with mmio in struct
> > > rzg2l_mipi_dsi
> > >  * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
> > >and rzg2l_mipi_dsi_link_write
> > >  * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> > > RFC->v1:
> > >  * 

Re: [PATCH v8 0/3] Add RZ/G2L DSI driver

2022-10-15 Thread Laurent Pinchart
Hi Biju,

On Sat, Oct 15, 2022 at 01:11:20PM +, Biju Das wrote:
> Gentle ping.

I've reviewed v8, everything looks fine. I've applied the patches to my
tree ([1]) for v6.2.

[1] git://linuxtv.org/pinchartl/media.git drm/du/next

> > Subject: [PATCH v8 0/3] Add RZ/G2L DSI driver
> > 
> > This patch series aims to support the MIPI DSI encoder found in the
> > RZ/G2L SoC. It currently supports DSI video mode only.
> > 
> > This unit supports MIPI Alliance Specification for Display Serial
> > Interface (DSI) Specification. This unit provides a solution for
> > transmitting MIPI DSI compliant digital video and packets. Normative
> > References are below.
> > * MIPI Alliance Specification for Display Serial Interface Version
> > 1.3.1
> > * MIPI Alliance Specification for D-PHY Version 2.1
> > 
> > The following are key features of this unit.
> > 
> > * 1 channel
> > * The number of Lane: 4-lane
> > * Support up to Full HD (1920 × 1080), 60 fps (RGB888)
> > * Maximum Bandwidth: 1.5 Gbps per lane
> > * Support Output Data Format: RGB666 / RGB888
> > 
> > This patch series is based on drm_misc and patches from drm/du/next
> > [1]
> > 
> > 
> > v7->v8:
> >  * Added Rb tag from Laurent.
> >  * Added hsfreq_max to struct rzg2l_mipi_dsi_timings.
> >  * Removed enums rzg2l_mipi_dsi_dphy_timings.
> >  * Replaced if else with for loop for finding dphy_timings
> >based on hsfreq.
> >  * Removed checking "number of lanes capability" from rzg2l_mipi_dsi_
> >startup() and added patch#3 for handling it in probe() and enforcing
> >it in rzg2l_mipi_dsi_host_attach().
> >  * Added Labels with an "err_" prefix.
> > out_pm_put->err_pm_put
> > out_assert_rst_and_stop_clocks->err_stop
> > out_stop_hs_clock->err_stop_clock
> > out_pm_disable->err_pm_disable
> >  * Added error message for lane check in rzg2l_mipi_dsi_host_attach()
> >  * Replaced dev_warn->dev_err for the format error in rzg2l_mipi_dsi_host
> >_attach(). Added missing "\n" and print the format for debugging.
> > v6->v7:
> >  * Added rzg2l_mipi_dsi_stop() counterpart of rzg2l_mipi_dsi_startup().
> >  * Error labels are named according to the cleanup operation they perform.
> >  * Restored Max lane capability read after dphy timing initialization
> >as per the guide lines from SoC design team.
> >  * Added recommended lut values for the Global Operation Timing
> >parameters for MIPI DPHY.
> > v5->v6:
> >  * Updated commit description
> >  * Moved handling of arst and prst from rzg2l_mipi_dsi_startup->runtime
> >PM suspend/resume handlers.
> >  * Max lane capability read at probe(), and enforced in
> >rzg2l_mipi_dsi_host_attach()
> >  * Simplified vich1ppsetr setting.
> >  * Renamed hsclk_running_mode,hsclk_mode->is_clk_cont.
> >  * Fixed typo in probe error message(arst->rst).
> >  * Reordered DRM bridge initaization in probe()
> >  * Updated typo in e-mail address.
> > v4->v5:
> >  * Added Ack from Sam.
> >  * Added a trivial change, replaced rzg2l_mipi_dsi_parse_dt()
> >with drm_of_get_data_lanes_count_ep() in probe.
> > v3->v4:
> >  * Updated error handling in rzg2l_mipi_dsi_startup() and 
> > rzg2l_mipi_dsi_atomic_enable().
> > v2->v3:
> >  * Added Rb tag from Geert and Laurent
> >  * Fixed the typo "Receive" -> "transmit"
> >  * Added accepible values for data-lanes
> >  * Sorted Header file in the example
> >  * Added SoC specific compaible along with generic one.
> >  * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr function instead
> >of the memory pointer
> >  * Fixed the comment in rzg2l_mipi_dsi_startup()
> >  * Removed unnecessary dbg message from rzg2l_mipi_dsi_start_video()
> >  * DRM bridge parameter initialization moved to probe
> >  * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
> >  * Inserted the missing blank lane after return in probe()
> >  * Added missing MODULE_DEVICE_TABLE
> >  * Added include linux/bits.h in header file
> >  * Fixed various macros in header file.
> >  * Reorder the make file for DSI, so that it is no more dependent
> >on RZ/G2L DU patch series.
> > v1->v2:
> >  * Added full path for dsi-controller.yaml
> >  * Modeled DSI + D-PHY as single block and updated reg property
> >  * Fixed typo D_PHY->D-PHY
> >  * Updated description
> >  * Added interrupts and interrupt-names and updated the example
> >  * Driver rework based on dt-binding changes (DSI + D-PHY) as single block
> >  * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi
> >  * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
> >and rzg2l_mipi_dsi_link_write
> >  * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> > RFC->v1:
> >  * Added a ref to dsi-controller.yaml.
> >  * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
> >and dropped DRM as it is implied by DRM_BRIDGE
> >  * Used devm_reset_control_get_exclusive() for reset handle
> >  * Removed bool hsclkmode from struct rzg2l_mipi_dsi
> >  * Added error check for pm, using 

Re: [PATCH v8 3/3] drm: rcar-du: rzg2l_mipi_dsi: Enhance device lanes check

2022-10-15 Thread Laurent Pinchart
Hi Biju,

Thank you for the patch.

On Tue, Sep 20, 2022 at 11:55:01AM +0100, Biju Das wrote:
> Enhance device lanes check by reading TXSETR register at probe(),
> and enforced in rzg2l_mipi_dsi_host_attach().
> 
> As per HW manual, we can read TXSETR register only after
> DPHY initialization.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Biju Das 

Reviewed-by: Laurent Pinchart 

> ---
> v8:
>  * New patch.
> ---
>  drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c | 122 ---
>  1 file changed, 88 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c 
> b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> index 8579208db218..aa95b85a2964 100644
> --- a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> @@ -171,6 +171,11 @@ static void rzg2l_mipi_dsi_link_write(struct 
> rzg2l_mipi_dsi *dsi, u32 reg, u32 d
>   iowrite32(data, dsi->mmio + LINK_REG_OFFSET + reg);
>  }
>  
> +static u32 rzg2l_mipi_dsi_phy_read(struct rzg2l_mipi_dsi *dsi, u32 reg)
> +{
> + return ioread32(dsi->mmio + reg);
> +}
> +
>  static u32 rzg2l_mipi_dsi_link_read(struct rzg2l_mipi_dsi *dsi, u32 reg)
>  {
>   return ioread32(dsi->mmio + LINK_REG_OFFSET + reg);
> @@ -180,19 +185,11 @@ static u32 rzg2l_mipi_dsi_link_read(struct 
> rzg2l_mipi_dsi *dsi, u32 reg)
>   * Hardware Setup
>   */
>  
> -static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> -   const struct drm_display_mode *mode)
> +static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
> + unsigned long hsfreq)
>  {
>   const struct rzg2l_mipi_dsi_timings *dphy_timings;
> - unsigned long hsfreq;
> - unsigned int i, bpp;
> - u32 txsetr;
> - u32 clstptsetr;
> - u32 lptrnstsetr;
> - u32 clkkpt;
> - u32 clkbfht;
> - u32 clkstpt;
> - u32 golpbkt;
> + unsigned int i;
>   u32 dphyctrl0;
>   u32 dphytim0;
>   u32 dphytim1;
> @@ -200,19 +197,6 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi 
> *dsi,
>   u32 dphytim3;
>   int ret;
>  
> - /*
> -  * Relationship between hsclk and vclk must follow
> -  * vclk * bpp = hsclk * 8 * lanes
> -  * where vclk: video clock (Hz)
> -  *   bpp: video pixel bit depth
> -  *   hsclk: DSI HS Byte clock frequency (Hz)
> -  *   lanes: number of data lanes
> -  *
> -  * hsclk(bit) = hsclk(byte) * 8
> -  */
> - bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> - hsfreq = (mode->clock * bpp * 8) / (8 * dsi->lanes);
> -
>   /* All DSI global operation timings are set with recommended setting */
>   for (i = 0; i < ARRAY_SIZE(rzg2l_mipi_dsi_global_timings); ++i) {
>   dphy_timings = _mipi_dsi_global_timings[i];
> @@ -220,12 +204,6 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi 
> *dsi,
>   break;
>   }
>  
> - ret = pm_runtime_resume_and_get(dsi->dev);
> - if (ret < 0)
> - return ret;
> -
> - clk_set_rate(dsi->vclk, mode->clock * 1000);
> -
>   /* Initializing DPHY before accessing LINK */
>   dphyctrl0 = DSIDPHYCTRL0_CAL_EN_HSRX_OFS | DSIDPHYCTRL0_CMN_MASTER_EN |
>   DSIDPHYCTRL0_RE_VDD_DETVCCQLV18 | DSIDPHYCTRL0_EN_BGR;
> @@ -259,10 +237,62 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi 
> *dsi,
>  
>   ret = reset_control_deassert(dsi->rstc);
>   if (ret < 0)
> - goto err_pm_put;
> + return ret;
>  
>   udelay(1);
>  
> + return 0;
> +}
> +
> +static void rzg2l_mipi_dsi_dphy_exit(struct rzg2l_mipi_dsi *dsi)
> +{
> + u32 dphyctrl0;
> +
> + dphyctrl0 = rzg2l_mipi_dsi_phy_read(dsi, DSIDPHYCTRL0);
> +
> + dphyctrl0 &= ~(DSIDPHYCTRL0_EN_LDO1200 | DSIDPHYCTRL0_EN_BGR);
> + rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYCTRL0, dphyctrl0);
> +
> + reset_control_assert(dsi->rstc);
> +}
> +
> +static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> +   const struct drm_display_mode *mode)
> +{
> + unsigned long hsfreq;
> + unsigned int bpp;
> + u32 txsetr;
> + u32 clstptsetr;
> + u32 lptrnstsetr;
> + u32 clkkpt;
> + u32 clkbfht;
> + u32 clkstpt;
> + u32 golpbkt;
> + int ret;
> +
> + /*
> +  * Relationship between hsclk and vclk must follow
> +  * vclk * bpp = hsclk * 8 * lanes
> +  * where vclk: video clock (Hz)
> +  *   bpp: video pixel bit depth
> +  *   hsclk: DSI HS Byte clock frequency (Hz)
> +  *   lanes: number of data lanes
> +  *
> +  * hsclk(bit) = hsclk(byte) * 8
> +  */
> + bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + hsfreq = (mode->clock * bpp * 8) / (8 * dsi->lanes);
> +
> + ret = pm_runtime_resume_and_get(dsi->dev);
> + if (ret < 0)
> + return ret;
> +
> + clk_set_rate(dsi->vclk, mode->clock * 1000);
> +
> + ret = 

Re: [PATCH] dt-bindings: Remove "status" from schema examples, again

2022-10-15 Thread Krzysztof Kozlowski
On 14/10/2022 16:51, Rob Herring wrote:
> There's no reason to have "status" properties in examples. "okay" is the
> default, and "disabled" turns off some schema checks ('required'
> specifically).
> 
> A meta-schema check for this is pending, so hopefully the last time to
> fix these.
> 
> Fix the indentation in intel,phy-thunderbay-emmc while we're here.
> 


Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof



Re: [PATCH v5 01/22] drm/tests: Add Kunit Helpers

2022-10-15 Thread Noralf Trønnes



Den 13.10.2022 15.18, skrev Maxime Ripard:
> As the number of kunit tests in KMS grows further, we start to have
> multiple test suites that, for example, need to register a mock DRM
> driver to interact with the KMS function they are supposed to test.
> 
> Let's add a file meant to provide those kind of helpers to avoid
> duplication.
> 
> Signed-off-by: Maxime Ripard 
> 
> ---

Reviewed-by: Noralf Trønnes 


[Bug 214425] [drm][amdgpu][TTM] Page pool memory never gets freed

2022-10-15 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=214425

--- Comment #3 from Rafael Ristovski (rafael.ristov...@gmail.com) ---
(In reply to Martin Doucha from comment #2)
> (In reply to Rafael Ristovski from comment #1)
> > According to amdgpu devs, this is a feature where the allocated pages are
> > kept around in case they are needed later on. TTM is able to release the
> > memory in case the memory pressure increases.
> 
> I understand the logic behind keeping idle buffers allocated for a while.
> But it does not make sense to keep them for hours after last use and the
> release mechanism on increased memory pressure does not seem to be working.
> 
> When I run a large compilation overnight, starting from a fresh reboot and
> shutting down all graphics software including the X server, I'll often come
> back in the morning to find that 70% of all RAM is allocated in idle TTM
> buffers and GCC is stuck swapping for hours. The TTM buffers were likely
> allocated by some GPU-accelerated build computation halfway through the
> night. But this is harder to reproduce than the games I've mentioned in the
> initial bugreport.

Indeed, I too run into situations where even if I purposefully trigger an OOM
situation just to get the TTM "cache" to evict itself through memory pressure,
_it still does not end up releasing all of the memory_.

There are also the following two sysfs files, simply reading them triggers an
eviction of GTT/VRAM:
> cat /sys/kernel/debug/dri/0/amdgpu_evict_vram
> cat /sys/kernel/debug/dri/0/amdgpu_evict_gtt

this can be confirmed as working with tools like `radeontop`/`nvtop`.

However, this once again does not release the TTM buffers.

As you can see in the issue I linked, I never got a reply about a mechanism to
manually release TTM memory. I will attempt coercing an answer on IRC, perhaps
I will have better luck asking directly there.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH v4 11/30] drm/modes: Add a function to generate analog display modes

2022-10-15 Thread Noralf Trønnes



Den 13.10.2022 10.36, skrev Maxime Ripard:
> Hi Noralf,
> 
> On Sat, Oct 01, 2022 at 02:52:06PM +0200, Noralf Trønnes wrote:
>> Den 29.09.2022 18.31, skrev Maxime Ripard:
>>> Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and
>>> 625-lines modes in their drivers.
>>>
>>> Since those modes are fairly standard, and that we'll need to use them
>>> in more places in the future, it makes sense to move their definition
>>> into the core framework.
>>>
>>> However, analog display usually have fairly loose timings requirements,
>>> the only discrete parameters being the total number of lines and pixel
>>> clock frequency. Thus, we created a function that will create a display
>>> mode from the standard, the pixel frequency and the active area.
>>>
>>> Signed-off-by: Maxime Ripard 
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - Reworded the line length check comment
>>> - Switch to HZ_PER_KHZ in tests
>>> - Use previous timing to fill our mode
>>> - Move the number of lines check earlier
>>> ---
>>>  drivers/gpu/drm/drm_modes.c| 474 
>>> +
>>>  drivers/gpu/drm/tests/Makefile |   1 +
>>>  drivers/gpu/drm/tests/drm_modes_test.c | 144 ++
>>>  include/drm/drm_modes.h|  17 ++
>>>  4 files changed, 636 insertions(+)
>>>
>>
>> I haven't followed the discussion on this patch, but it seems rather
>> excessive to add over 600 lines of code (including tests) to add 2 fixed
>> modes. And it's very difficult to see from the code what the actual
>> display mode timings really are, which would be useful for other
>> developers down the road wanting to use them.
>>
>> Why not just hardcode the modes?
> 
> Yeah, I have kind of the same feeling tbh, but it was asked back on the
> v1 to ease the transition of old fbdev drivers, since they will need
> such a function:
> https://lore.kernel.org/dri-devel/CAMuHMdUrwzPYjA0wdR7ADj5Ov6+m03JbnY8fBYzRYyWDuNm5=g...@mail.gmail.com/
> 

If that's the case I suggest you just hardcode them for now and leave
the complexity to the developer doing the actual conversion of the fbdev
driver. Who knows when that will happen, but that person will have your
well documented and discussed work to fall back on.

Noralf.


[Bug 214425] [drm][amdgpu][TTM] Page pool memory never gets freed

2022-10-15 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=214425

--- Comment #2 from Martin Doucha (dou...@swarmtech.cz) ---
(In reply to Rafael Ristovski from comment #1)
> According to amdgpu devs, this is a feature where the allocated pages are
> kept around in case they are needed later on. TTM is able to release the
> memory in case the memory pressure increases.

I understand the logic behind keeping idle buffers allocated for a while. But
it does not make sense to keep them for hours after last use and the release
mechanism on increased memory pressure does not seem to be working.

When I run a large compilation overnight, starting from a fresh reboot and
shutting down all graphics software including the X server, I'll often come
back in the morning to find that 70% of all RAM is allocated in idle TTM
buffers and GCC is stuck swapping for hours. The TTM buffers were likely
allocated by some GPU-accelerated build computation halfway through the night.
But this is harder to reproduce than the games I've mentioned in the initial
bugreport.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 214425] [drm][amdgpu][TTM] Page pool memory never gets freed

2022-10-15 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=214425

Rafael Ristovski (rafael.ristov...@gmail.com) changed:

   What|Removed |Added

 CC||rafael.ristov...@gmail.com

--- Comment #1 from Rafael Ristovski (rafael.ristov...@gmail.com) ---
According to amdgpu devs, this is a feature where the allocated pages are kept
around in case they are needed later on. TTM is able to release the memory in
case the memory pressure increases.

See comment here:
https://gitlab.freedesktop.org/drm/amd/-/issues/1942#note_1311016

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

RE: [PATCH v8 0/3] Add RZ/G2L DSI driver

2022-10-15 Thread Biju Das
Gentle ping.

Cheers,
Biju

> Subject: [PATCH v8 0/3] Add RZ/G2L DSI driver
> 
> This patch series aims to support the MIPI DSI encoder found in the
> RZ/G2L SoC. It currently supports DSI video mode only.
> 
> This unit supports MIPI Alliance Specification for Display Serial
> Interface (DSI) Specification. This unit provides a solution for
> transmitting MIPI DSI compliant digital video and packets. Normative
> References are below.
> * MIPI Alliance Specification for Display Serial Interface Version
> 1.3.1
> * MIPI Alliance Specification for D-PHY Version 2.1
> 
> The following are key features of this unit.
> 
> * 1 channel
> * The number of Lane: 4-lane
> * Support up to Full HD (1920 × 1080), 60 fps (RGB888)
> * Maximum Bandwidth: 1.5 Gbps per lane
> * Support Output Data Format: RGB666 / RGB888
> 
> This patch series is based on drm_misc and patches from drm/du/next
> [1]
> 
> 
> v7->v8:
>  * Added Rb tag from Laurent.
>  * Added hsfreq_max to struct rzg2l_mipi_dsi_timings.
>  * Removed enums rzg2l_mipi_dsi_dphy_timings.
>  * Replaced if else with for loop for finding dphy_timings
>based on hsfreq.
>  * Removed checking "number of lanes capability" from rzg2l_mipi_dsi_
>startup() and added patch#3 for handling it in probe() and
> enforcing
>it in rzg2l_mipi_dsi_host_attach().
>  * Added Labels with an "err_" prefix.
>   out_pm_put->err_pm_put
>   out_assert_rst_and_stop_clocks->err_stop
>   out_stop_hs_clock->err_stop_clock
>   out_pm_disable->err_pm_disable
>  * Added error message for lane check in rzg2l_mipi_dsi_host_attach()
>  * Replaced dev_warn->dev_err for the format error in
> rzg2l_mipi_dsi_host
>_attach(). Added missing "\n" and print the format for debugging.
> v6->v7:
>  * Added rzg2l_mipi_dsi_stop() counterpart of
> rzg2l_mipi_dsi_startup().
>  * Error labels are named according to the cleanup operation they
> perform.
>  * Restored Max lane capability read after dphy timing initialization
>as per the guide lines from SoC design team.
>  * Added recommended lut values for the Global Operation Timing
>parameters for MIPI DPHY.
> v5->v6:
>  * Updated commit description
>  * Moved handling of arst and prst from rzg2l_mipi_dsi_startup-
> >runtime
>PM suspend/resume handlers.
>  * Max lane capability read at probe(), and enforced in
>rzg2l_mipi_dsi_host_attach()
>  * Simplified vich1ppsetr setting.
>  * Renamed hsclk_running_mode,hsclk_mode->is_clk_cont.
>  * Fixed typo in probe error message(arst->rst).
>  * Reordered DRM bridge initaization in probe()
>  * Updated typo in e-mail address.
> v4->v5:
>  * Added Ack from Sam.
>  * Added a trivial change, replaced rzg2l_mipi_dsi_parse_dt()
>with drm_of_get_data_lanes_count_ep() in probe.
> v3->v4:
>  * Updated error handling in rzg2l_mipi_dsi_startup() and
> rzg2l_mipi_dsi_atomic_enable().
> v2->v3:
>  * Added Rb tag from Geert and Laurent
>  * Fixed the typo "Receive" -> "transmit"
>  * Added accepible values for data-lanes
>  * Sorted Header file in the example
>  * Added SoC specific compaible along with generic one.
>  * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr function
> instead
>of the memory pointer
>  * Fixed the comment in rzg2l_mipi_dsi_startup()
>  * Removed unnecessary dbg message from rzg2l_mipi_dsi_start_video()
>  * DRM bridge parameter initialization moved to probe
>  * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
>  * Inserted the missing blank lane after return in probe()
>  * Added missing MODULE_DEVICE_TABLE
>  * Added include linux/bits.h in header file
>  * Fixed various macros in header file.
>  * Reorder the make file for DSI, so that it is no more dependent
>on RZ/G2L DU patch series.
> v1->v2:
>  * Added full path for dsi-controller.yaml
>  * Modeled DSI + D-PHY as single block and updated reg property
>  * Fixed typo D_PHY->D-PHY
>  * Updated description
>  * Added interrupts and interrupt-names and updated the example
>  * Driver rework based on dt-binding changes (DSI + D-PHY) as single
> block
>  * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi
>  * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
>and rzg2l_mipi_dsi_link_write
>  * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> RFC->v1:
>  * Added a ref to dsi-controller.yaml.
>  * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
>and dropped DRM as it is implied by DRM_BRIDGE
>  * Used devm_reset_control_get_exclusive() for reset handle
>  * Removed bool hsclkmode from struct rzg2l_mipi_dsi
>  * Added error check for pm, using pm_runtime_resume_and_get() instead
> of
>pm_runtime_get_sync()
>  * Added check for unsupported formats in rzg2l_mipi_dsi_host_attach()
>  * Avoided read-modify-write stopping hsclock
>  * Used devm_platform_ioremap_resource for resource allocation
>  * Removed unnecessary assert call from probe and remove.
>  * wrap the line after the PTR_ERR() in probe()
>  * Updated reset failure 

[Bug 216119] 087451f372bf76d breaks hibernation on amdgpu Radeon R9 390

2022-10-15 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=216119

--- Comment #42 from Harald Judt (h.j...@gmx.at) ---
Created attachment 303004
  --> https://bugzilla.kernel.org/attachment.cgi?id=303004=edit
dmesg.out

Here is a new dmesg with linux-stable-5.19.11 and amdgpu.dc=0.

There are a couple more "Fence fallbasck timer expired on ring sdma0" than
before, but nothing else directly after resuming.

However, VT-switching to the console and back to X causes the GPU fault in the
last lines. I could login and produce this dmesg, but the machine wouldn't
reboot with sudo reboot and I had to do sys-rq finally.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: Re: [PATCH v5 22/22] drm/sun4i: tv: Convert to the new TV mode property

2022-10-15 Thread Jernej Škrabec
Dne petek, 14. oktober 2022 ob 09:38:10 CEST je Maxime Ripard napisal(a):
> Hi Jernej,
> 
> On Thu, Oct 13, 2022 at 08:23:51PM +0200, Jernej Škrabec wrote:
> > Dne četrtek, 13. oktober 2022 ob 15:19:06 CEST je Maxime Ripard 
napisal(a):
> > > Now that the core can deal fine with analog TV modes, let's convert the
> > > sun4i TV driver to leverage those new features.
> > > 
> > > Acked-by: Noralf Trønnes 
> > > Signed-off-by: Maxime Ripard 
> > > 
> > > ---
> > > Changes in v5:
> > > - Removed the count variable in get_modes
> > > - Removed spurious vc4 change
> > > ---
> > > 
> > >  drivers/gpu/drm/sun4i/sun4i_tv.c | 145
> > > 
> > > +-- 1 file changed, 48
> > > insertions(+),
> > > 97 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tv.c index c65f0a89b6b0..4f07aff11551
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > @@ -141,23 +141,14 @@ struct resync_parameters {
> > > 
> > >  struct tv_mode {
> > >  
> > >   char*name;
> > > 
> > > + unsigned inttv_mode;
> > > +
> > > 
> > >   u32 mode;
> > >   u32 chroma_freq;
> > >   u16 back_porch;
> > >   u16 front_porch;
> > > 
> > > - u16 line_number;
> > > 
> > >   u16 vblank_level;
> > 
> > isn't there a way to get or calculate back_porch, front_porch and
> > vblank_level from mode? From quick glance over removed values below, I
> > would say that at least back_porch can be removed too?
> 
> I tried actually, but I'm not sure what the front porch and back porch
> parameters actually are. They are called that way by Allwinner, but it
> doesn't match the PAL or NTSC timings at all.
> 
> For example, back_porch is 118 for NTSC and 138 for PAL. Actual back
> porches would be around 12 and 16, respectively. Actually, the entire
> blanking area are 138 and 144. This is close enough for PAL, but pretty
> far off for NTSC.
> 
> Allwinner has the habit of integrating the sync period into one of the
> porches, but still there's no obvious match.
> 
> front_porch is pretty much in the same case.

Ok then.

> 
> Since it affected the display output quite a lot, I chose to keep it for
> now unfortunately.
> 
> > Otherwise this patch looks ok.
> 
> Can I add your Acked-by/Reviewed-by then?

Sure.
Reviewed-by: Jernej Skrabec 

Best regards,
Jernej