date:20221202

Am Dienstag, dem 08.11.2022 um 15:14 +0100 schrieb Philipp Zabel:
> ipu_src_rect_width() was introduced to support odd screen resolutions
> such as 1366x768 by internally rounding up primary plane width to a
> multiple of 8 and compensating with reduced horizontal blanking.
> This also caused overlay plane width to be rounded up, which was not
> intended. Fix overlay plane width by limiting the rounding up to the
> primary plane.
> 
> drm_rect_width(_state->src) >> 16 is the same value as
> drm_rect_width(dst) because there is no plane scaling support.

Heh, I stumbled at exactly this point while reading the code changes
and was about to suggest it be added to the changelog until I realized
that it's already here. :)
> 
> Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> Signed-off-by: Philipp Zabel 

Reviewed-by: Lucas Stach 

One note below.

> ---
>  drivers/gpu/drm/imx/ipuv3-plane.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index dba4f7d81d69..80142d9a4a55 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -614,6 +614,11 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   break;
>   }
>  
> + if (ipu_plane->dp_flow == IPU_DP_FLOW_SYNC_BG)
> + width = ipu_src_rect_width(new_state);
> + else
> + width = drm_rect_width(_state->src) >> 16;
> +
>   eba = drm_plane_state_to_eba(new_state, 0);
>  
>   /*
> @@ -622,8 +627,7 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>*/
>   if (ipu_state->use_pre) {
>   axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
> - ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id,
> -   ipu_src_rect_width(new_state),
> + ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
> drm_rect_height(_state->src) >> 
> 16,
> fb->pitches[0], fb->format->format,
> fb->modifier, );
> @@ -678,9 +682,8 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   break;
>   }
>  
> - ipu_dmfc_config_wait4eot(ipu_plane->dmfc, ALIGN(drm_rect_width(dst), 
> 8));
> + ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
>  
> - width = ipu_src_rect_width(new_state);
>   height = drm_rect_height(_state->src) >> 16;

There's a opportunity for a follow-up cleanup here:
"drm_rect_height(_state->src) >> 16" is used multiple times in this
function, which could be replaced by using this local height variable
instead.

>   info = drm_format_info(fb->format->format);
>   ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
> @@ -744,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
>  
>   ipu_cpmem_zero(ipu_plane->alpha_ch);
> - ipu_cpmem_set_resolution(ipu_plane->alpha_ch,
> -  ipu_src_rect_width(new_state),
> + ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
>drm_rect_height(_state->src) >> 
> 16);
>   ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
>   ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
> 
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780

Re: Try to address the DMA-buf coherency problem

2022-12-02 Thread Christian König

Am 30.11.22 um 11:30 schrieb Daniel Vetter:

On Fri, Nov 25, 2022 at 11:40:04AM -0500, Nicolas Dufresne wrote:

Le mercredi 23 novembre 2022 à 17:33 +0100, Daniel Vetter a écrit :

On Wed, Nov 23, 2022 at 10:33:38AM +0200, Pekka Paalanen wrote:

On Tue, 22 Nov 2022 18:33:59 +0100
Christian König wrote:

We should have come up with dma-heaps earlier and make it clear that
exporting a DMA-buf from a device gives you something device specific
which might or might not work with others.

Apart from that I agree, DMA-buf should be capable of handling this.
Question left is what documentation is missing to make it clear how
things are supposed to work?

Perhaps somewhat related from Daniel Stone that seems to have been
forgotten:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210905122742.86029-1-daniels%40collabora.com%2Fdata=05%7C01%7Cchristian.koenig%40amd.com%7C45786a08e6dc4af2816508dad2bdf957%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638054011293521624%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=GjsoJGNoPozTS2SWeeirURzQatI5vfl9%2B%2BfzoavgTbw%3Dreserved=0

It aimed mostly at userspace, but sounds to me like the coherency stuff
could use a section of its own there?

Hm yeah it would be great to land that and then eventually extend. Daniel?

There is a lot of things documented in this document that have been said to be
completely wrong user-space behaviour in this thread. But it seems to pre-date
the DMA Heaps. The document also assume that DMA Heaps completely solves the CMA
vs system memory issue. But it also underline a very important aspect, that
userland is not aware which one to use. What this document suggest though seems
more realist then what has been said here.

Its overall a great document, it unfortunate that it only makes it into the DRM
mailing list.

The doc is more about document the current status quo/best practices,
which is very much not using dma-heaps.

The issue there is that currently userspace has no idea which dma-heap to
use for shared buffers, plus not all allocators are exposed through heaps
to begin with. We had this noted as a todo item (add some device->heap
sysfs links was the idea), until that's done all you can do is hardcode
the right heaps for the right usage in userspace, which is what android
does. Plus android doesn't have dgpu, so doesn't need the missing ttm
heap.

Hui? Why do you think we should have a TTM heap in the first place?

As far as I can see we need three different ways of allocation:
1. Normal system memory.
2. CMA based.
3. Device specific.

When any of the participating devices needs CMA then user space must use
the CMA heap, when any of the participating devices needs device
specific memory then user space must use device specific memory (from
that device).

It still can be that two devices can't talk with each other because both
needs the buffer to be allocated from device specific memory for a
particular use case, but at least all the cases for both none CPU cache
coherent ARM devices as well as device specific memory for scanout
should be handled with this.

Regards,
Christian.

But yeah the long-term aspiration also hasn't changed, because the
dma-heap todo list is also very, very old by now :-/
-Daniel

Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization

Hi Marek

On Fri, 2 Dec 2022 at 12:21, Marek Vasut  wrote:
>
> On 12/2/22 11:52, Marek Szyprowski wrote:
> > Hi,
> >
> > Sorry for delay, I was on a sick leave last 2 weeks.
> >
> > On 28.11.2022 15:43, Jagan Teki wrote:
> >> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut  wrote:
> >>> On 11/23/22 21:09, Jagan Teki wrote:
>  On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut  wrote:
> > On 11/17/22 14:04, Marek Szyprowski wrote:
> >> On 17.11.2022 05:58, Marek Vasut wrote:
> >>> On 11/10/22 19:38, Jagan Teki wrote:
>  DSI host initialization handling in previous exynos dsi driver has
>  some pitfalls. It initializes the host during host transfer() hook
>  that is indeed not the desired call flow for I2C and any other DSI
>  configured downstream bridges.
> 
>  Host transfer() is usually triggered for downstream DSI panels or
>  bridges and I2C-configured-DSI bridges miss these host initialization
>  as these downstream bridges use bridge operations hooks like 
>  pre_enable,
>  and enable in order to initialize or set up the host.
> 
>  This patch is trying to handle the host init handler to satisfy all
>  downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED 
>  state
>  flag to ensure that host init is also done on first cmd transfer, 
>  this
>  helps existing DSI panels work on exynos platform (form Marek
>  Szyprowski).
> 
>  v8, v7, v6, v5:
>  * none
> 
>  v4:
>  * update init handling to ensure host init done on first cmd transfer
> 
>  v3:
>  * none
> 
>  v2:
>  * check initialized state in samsung_dsim_init
> 
>  v1:
>  * keep DSI init in host transfer
> 
>  Signed-off-by: Marek Szyprowski 
>  Signed-off-by: Jagan Teki 
>  ---
>    drivers/gpu/drm/bridge/samsung-dsim.c | 25 
>  +
>    include/drm/bridge/samsung-dsim.h |  5 +++--
>    2 files changed, 20 insertions(+), 10 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>  b/drivers/gpu/drm/bridge/samsung-dsim.c
>  index bb1f45fd5a88..ec7e01ae02ea 100644
>  --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>  +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>  @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
>  samsung_dsim *dsi)
>    disable_irq(dsi->irq);
>    }
>    -static int samsung_dsim_init(struct samsung_dsim *dsi)
>  +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
>  flag)
>    {
>    const struct samsung_dsim_driver_data *driver_data =
>  dsi->driver_data;
>    +if (dsi->state & flag)
>  +return 0;
>  +
>    samsung_dsim_reset(dsi);
>  -samsung_dsim_enable_irq(dsi);
>  +
>  +if (!(dsi->state & DSIM_STATE_INITIALIZED))
>  +samsung_dsim_enable_irq(dsi);
>  if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>    samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>  @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>  samsung_dsim *dsi)
>    samsung_dsim_set_phy_ctrl(dsi);
>    samsung_dsim_init_link(dsi);
>    +dsi->state |= flag;
>  +
>    return 0;
>    }
>    @@ -1269,6 +1276,10 @@ static void
>  samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>    }
>  dsi->state |= DSIM_STATE_ENABLED;
>  +
>  +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>  +if (ret)
>  +return;
>    }
>  static void samsung_dsim_atomic_enable(struct drm_bridge 
>  *bridge,
>  @@ -1458,12 +1469,9 @@ static ssize_t
>  samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>    if (!(dsi->state & DSIM_STATE_ENABLED))
>    return -EINVAL;
>    -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>  -ret = samsung_dsim_init(dsi);
>  -if (ret)
>  -return ret;
>  -dsi->state |= DSIM_STATE_INITIALIZED;
>  -}
>  +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
> >>> This triggers full controller reset and reprogramming upon first
> >>> command transfer, is such heavy handed reload really necessary ?
> >> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> >> DSI. If this is a real issue

[PATCH v3 1/5] drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs

From: Sam Ravnborg 

The atomic variants of enable/disable in drm_bridge_funcs are the
preferred operations - introduce these.

The ps8640 driver used the non-atomic variants of the 
drm_bridge_chain_pre_enable/
drm_bridge_chain_post_disable - convert these to the atomic variants.

v2:
  - Init state operations in drm_bridge_funcs (Laurent)

Signed-off-by: Sam Ravnborg 
Reviewed-by: Maxime Ripard 
Cc: Jitao Shi 
Cc: Enric Balletbo i Serra 
Cc: Philip Chen 
Cc: Andrzej Hajda 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Laurent Pinchart 
Cc: Jonas Karlman 
Cc: Jernej Skrabec 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Dave Stevenson 
---
 drivers/gpu/drm/bridge/parade-ps8640.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
b/drivers/gpu/drm/bridge/parade-ps8640.c
index f74090a9cc9e..4b361d7d5e44 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -442,7 +443,8 @@ static const struct dev_pm_ops ps8640_pm_ops = {
pm_runtime_force_resume)
 };
 
-static void ps8640_pre_enable(struct drm_bridge *bridge)
+static void ps8640_atomic_pre_enable(struct drm_bridge *bridge,
+struct drm_bridge_state *old_bridge_state)
 {
struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
@@ -476,7 +478,8 @@ static void ps8640_pre_enable(struct drm_bridge *bridge)
ps_bridge->pre_enabled = true;
 }
 
-static void ps8640_post_disable(struct drm_bridge *bridge)
+static void ps8640_atomic_post_disable(struct drm_bridge *bridge,
+  struct drm_bridge_state 
*old_bridge_state)
 {
struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
 
@@ -554,7 +557,7 @@ static struct edid *ps8640_bridge_get_edid(struct 
drm_bridge *bridge,
 * EDID, for this chip, we need to do a full poweron, otherwise it will
 * fail.
 */
-   drm_bridge_chain_pre_enable(bridge);
+   drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state);
 
edid = drm_get_edid(connector,
ps_bridge->page[PAGE0_DP_CNTL]->adapter);
@@ -564,7 +567,7 @@ static struct edid *ps8640_bridge_get_edid(struct 
drm_bridge *bridge,
 * before, return the chip to its original power state.
 */
if (poweroff)
-   drm_bridge_chain_post_disable(bridge);
+   drm_atomic_bridge_chain_post_disable(bridge, 
connector->state->state);
 
return edid;
 }
@@ -579,8 +582,11 @@ static const struct drm_bridge_funcs ps8640_bridge_funcs = 
{
.attach = ps8640_bridge_attach,
.detach = ps8640_bridge_detach,
.get_edid = ps8640_bridge_get_edid,
-   .post_disable = ps8640_post_disable,
-   .pre_enable = ps8640_pre_enable,
+   .atomic_post_disable = ps8640_atomic_post_disable,
+   .atomic_pre_enable = ps8640_atomic_pre_enable,
+   .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+   .atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
 static int ps8640_bridge_get_dsi_resources(struct device *dev, struct ps8640 
*ps_bridge)
-- 
2.34.1

[PATCH v3 5/5] drm/bridge: Document the expected behaviour of DSI host controllers

The exact behaviour of DSI host controllers is not specified,
therefore define it.

Signed-off-by: Dave Stevenson 
Reviewed-by: Laurent Pinchart 
---
 Documentation/gpu/drm-kms-helpers.rst |  7 +
 drivers/gpu/drm/drm_bridge.c  | 39 +++
 2 files changed, 46 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index a4860ffd6e86..b8ab05e42dbb 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -188,6 +188,13 @@ Bridge Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
:export:
 
+MIPI-DSI bridge operation
+-
+
+.. kernel-doc:: drivers/gpu/drm/drm_bridge.c
+   :doc: dsi bridge operations
+
+
 Bridge Connector Helper Reference
 -
 
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 41051869d6bf..bd73d32f29c0 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -153,6 +153,45 @@
  * situation when probing.
  */
 
+/**
+ * DOC: dsi bridge operations
+ *
+ * DSI host interfaces are expected to be implemented as bridges rather than
+ * encoders, however there are a few aspects of their operation that need to
+ * be defined in order to provide a consistent interface.
+ *
+ * A DSI host should keep the PHY powered down until the pre_enable operation 
is
+ * called. All lanes are in an undefined idle state up to this point, and it
+ * must not be assumed that it is LP-11.
+ * pre_enable should initialise the PHY, set the data lanes to LP-11, and the
+ * clock lane to either LP-11 or HS depending on the mode_flag
+ * %MIPI_DSI_CLOCK_NON_CONTINUOUS.
+ *
+ * Ordinarily the downstream bridge DSI peripheral pre_enable will have been
+ * called before the DSI host. If the DSI peripheral requires LP-11 and/or
+ * the clock lane to be in HS mode prior to pre_enable, then it can set the
+ * _enable_prev_first flag to request the pre_enable (and
+ * post_disable) order to be altered to enable the DSI host first.
+ *
+ * Either the CRTC being enabled, or the DSI host enable operation should 
switch
+ * the host to actively transmitting video on the data lanes.
+ *
+ * The reverse also applies. The DSI host disable operation or stopping the 
CRTC
+ * should stop transmitting video, and the data lanes should return to the 
LP-11
+ * state. The DSI host _disable operation should disable the PHY.
+ * If the _enable_prev_first flag is set, then the DSI peripheral's
+ * bridge _disable will be called before the DSI host's post_disable.
+ *
+ * Whilst it is valid to call _transfer prior to pre_enable or after
+ * post_disable, the exact state of the lanes is undefined at this point. The
+ * DSI host should initialise the interface, transmit the data, and then 
disable
+ * the interface again.
+ *
+ * Ultra Low Power State (ULPS) is not explicitly supported by DRM. If
+ * implemented, it therefore needs to be handled entirely within the DSI Host
+ * driver.
+ */
+
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);
 
-- 
2.34.1

[PATCH v3 4/5] drm/panel: Add prepare_prev_first flag to drm_panel

Mapping to the drm_bridge flag pre_enable_prev_first,
add a new flag prepare_prev_first to drm_panel to allow
the panel driver to request that the upstream bridge should
be pre_enabled before the panel prepare.

Signed-off-by: Dave Stevenson 
Reviewed-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/bridge/panel.c |  2 ++
 include/drm/drm_panel.h| 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 216af76d0042..03c3274dc3d9 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -364,6 +364,8 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(struct 
device *dev,
devres_free(ptr);
}
 
+   bridge->pre_enable_prev_first = panel->prepare_prev_first;
+
return bridge;
 }
 EXPORT_SYMBOL(devm_drm_panel_bridge_add_typed);
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 994bfcdd84c5..432fab2347eb 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -188,6 +188,16 @@ struct drm_panel {
 * Panel entry in registry.
 */
struct list_head list;
+
+   /**
+* @prepare_prev_first:
+*
+* The previous controller should be prepared first, before the prepare
+* for the panel is called. This is largely required for DSI panels
+* where the DSI host controller should be initialised to LP-11 before
+* the panel is powered up.
+*/
+   bool prepare_prev_first;
 };
 
 void drm_panel_init(struct drm_panel *panel, struct device *dev,
-- 
2.34.1

[PATCH v3 2/5] drm/bridge: Drop unused drm_bridge_chain functions

From: Sam Ravnborg 

The drm_bridge_chain_{pre_enable,enable,disable,post_disable} has no
users left and we have atomic variants that should be used.
Drop them so they do not gain new users.

Adjust a few comments to avoid references to the dropped functions.

Signed-off-by: Sam Ravnborg 
Reviewed-by: Maxime Ripard 
Reviewed-by: Laurent Pinchart 
Cc: Laurent Pinchart 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: Andrzej Hajda 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Daniel Vetter 
Signed-off-by: Dave Stevenson 
---
 drivers/gpu/drm/drm_bridge.c | 110 ---
 include/drm/drm_bridge.h |  28 -
 2 files changed, 138 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1545c50fd1c8..bb7fc09267af 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -509,61 +509,6 @@ drm_bridge_chain_mode_valid(struct drm_bridge *bridge,
 }
 EXPORT_SYMBOL(drm_bridge_chain_mode_valid);
 
-/**
- * drm_bridge_chain_disable - disables all bridges in the encoder chain
- * @bridge: bridge control structure
- *
- * Calls _bridge_funcs.disable op for all the bridges in the encoder
- * chain, starting from the last bridge to the first. These are called before
- * calling the encoder's prepare op.
- *
- * Note: the bridge passed should be the one closest to the encoder
- */
-void drm_bridge_chain_disable(struct drm_bridge *bridge)
-{
-   struct drm_encoder *encoder;
-   struct drm_bridge *iter;
-
-   if (!bridge)
-   return;
-
-   encoder = bridge->encoder;
-   list_for_each_entry_reverse(iter, >bridge_chain, chain_node) {
-   if (iter->funcs->disable)
-   iter->funcs->disable(iter);
-
-   if (iter == bridge)
-   break;
-   }
-}
-EXPORT_SYMBOL(drm_bridge_chain_disable);
-
-/**
- * drm_bridge_chain_post_disable - cleans up after disabling all bridges in the
- *encoder chain
- * @bridge: bridge control structure
- *
- * Calls _bridge_funcs.post_disable op for all the bridges in the
- * encoder chain, starting from the first bridge to the last. These are called
- * after completing the encoder's prepare op.
- *
- * Note: the bridge passed should be the one closest to the encoder
- */
-void drm_bridge_chain_post_disable(struct drm_bridge *bridge)
-{
-   struct drm_encoder *encoder;
-
-   if (!bridge)
-   return;
-
-   encoder = bridge->encoder;
-   list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
-   if (bridge->funcs->post_disable)
-   bridge->funcs->post_disable(bridge);
-   }
-}
-EXPORT_SYMBOL(drm_bridge_chain_post_disable);
-
 /**
  * drm_bridge_chain_mode_set - set proposed mode for all bridges in the
  *encoder chain
@@ -593,61 +538,6 @@ void drm_bridge_chain_mode_set(struct drm_bridge *bridge,
 }
 EXPORT_SYMBOL(drm_bridge_chain_mode_set);
 
-/**
- * drm_bridge_chain_pre_enable - prepares for enabling all bridges in the
- *  encoder chain
- * @bridge: bridge control structure
- *
- * Calls _bridge_funcs.pre_enable op for all the bridges in the encoder
- * chain, starting from the last bridge to the first. These are called
- * before calling the encoder's commit op.
- *
- * Note: the bridge passed should be the one closest to the encoder
- */
-void drm_bridge_chain_pre_enable(struct drm_bridge *bridge)
-{
-   struct drm_encoder *encoder;
-   struct drm_bridge *iter;
-
-   if (!bridge)
-   return;
-
-   encoder = bridge->encoder;
-   list_for_each_entry_reverse(iter, >bridge_chain, chain_node) {
-   if (iter->funcs->pre_enable)
-   iter->funcs->pre_enable(iter);
-
-   if (iter == bridge)
-   break;
-   }
-}
-EXPORT_SYMBOL(drm_bridge_chain_pre_enable);
-
-/**
- * drm_bridge_chain_enable - enables all bridges in the encoder chain
- * @bridge: bridge control structure
- *
- * Calls _bridge_funcs.enable op for all the bridges in the encoder
- * chain, starting from the first bridge to the last. These are called
- * after completing the encoder's commit op.
- *
- * Note that the bridge passed should be the one closest to the encoder
- */
-void drm_bridge_chain_enable(struct drm_bridge *bridge)
-{
-   struct drm_encoder *encoder;
-
-   if (!bridge)
-   return;
-
-   encoder = bridge->encoder;
-   list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
-   if (bridge->funcs->enable)
-   bridge->funcs->enable(bridge);
-   }
-}
-EXPORT_SYMBOL(drm_bridge_chain_enable);
-
 /**
  * drm_atomic_bridge_chain_disable - disables all bridges in the encoder chain
  * @bridge: bridge control structure
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 6b65b0dfb4fb..796567a203ac

[PATCH v3 3/5] drm/bridge: Introduce pre_enable_prev_first to alter bridge init order

DSI sink devices typically want the DSI host powered up and configured
before they are powered up. pre_enable is the place this would normally
happen, but they are called in reverse order from panel/connector towards
the encoder, which is the "wrong" order.

Add a new flag pre_enable_prev_first that any bridge can set
to swap the order of pre_enable (and post_disable) for that and the
immediately previous bridge.
Should the immediately previous bridge also set the
pre_enable_prev_first flag, the previous bridge to that will be called
before either of those which requested pre_enable_prev_first.

eg:
- Panel
- Bridge 1
- Bridge 2 pre_enable_prev_first
- Bridge 3
- Bridge 4 pre_enable_prev_first
- Bridge 5 pre_enable_prev_first
- Bridge 6
- Encoder
Would result in pre_enable's being called as Panel, Bridge 1, Bridge 3,
Bridge 2, Bridge 6, Bridge 5, Bridge 4, Encoder.

Signed-off-by: Dave Stevenson 
---
 drivers/gpu/drm/drm_bridge.c | 144 +--
 include/drm/drm_bridge.h |   8 ++
 2 files changed, 128 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index bb7fc09267af..41051869d6bf 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -581,6 +581,25 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge 
*bridge,
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
 
+static void drm_atomic_bridge_call_post_disable(struct drm_bridge *bridge,
+   struct drm_atomic_state 
*old_state)
+{
+   if (old_state && bridge->funcs->atomic_post_disable) {
+   struct drm_bridge_state *old_bridge_state;
+
+   old_bridge_state =
+   drm_atomic_get_old_bridge_state(old_state,
+   bridge);
+   if (WARN_ON(!old_bridge_state))
+   return;
+
+   bridge->funcs->atomic_post_disable(bridge,
+  old_bridge_state);
+   } else if (bridge->funcs->post_disable) {
+   bridge->funcs->post_disable(bridge);
+   }
+}
+
 /**
  * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges
  *   in the encoder chain
@@ -592,36 +611,85 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
  * starting from the first bridge to the last. These are called after 
completing
  * _encoder_helper_funcs.atomic_disable
  *
+ * If a bridge sets @pre_enable_prev_first, then the @post_disable for that
+ * bridge will be called before the previous one to reverse the @pre_enable
+ * calling direction.
+ *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
  struct drm_atomic_state *old_state)
 {
struct drm_encoder *encoder;
+   struct drm_bridge *next, *limit;
 
if (!bridge)
return;
 
encoder = bridge->encoder;
+
list_for_each_entry_from(bridge, >bridge_chain, chain_node) {
-   if (bridge->funcs->atomic_post_disable) {
-   struct drm_bridge_state *old_bridge_state;
+   limit = NULL;
+
+   if (!list_is_last(>chain_node, >bridge_chain)) 
{
+   next = list_next_entry(bridge, chain_node);
+
+   if (next->pre_enable_prev_first) {
+   /* next bridge had requested that prev
+* was enabled first, so disabled last
+*/
+   limit = next;
+
+   /* Find the next bridge that has NOT requested
+* prev to be enabled first / disabled last
+*/
+   list_for_each_entry_from(next, 
>bridge_chain,
+chain_node) {
+   if (next->pre_enable_prev_first) {
+   next = list_prev_entry(next, 
chain_node);
+   limit = next;
+   break;
+   }
+   }
+
+   /* Call these bridges in reverse order */
+   list_for_each_entry_from_reverse(next, 
>bridge_chain,
+chain_node) {
+   if (next == bridge)
+   break;
+
+   
drm_atomic_bridge_call_post_disable(next,
+   
old_state);
+

[PATCH v3 0/5] DSI host and peripheral initialisation ordering

Hi All

Changes from v2 (sorry it's taken me a while to get around to these):
- Added Sam's patches to drop drm_bridge_chain functions
- Renamed upstream to previously (Sam)
- Moved copying of panel->prepare_prev_first to bridge->pre_enable_prev_first
from drm_panel_bridge_add_typed to devm_drm_panel_bridge_add_typed (Jagan)

Changes from v1:
- New patch to refactor drm_bridge_chain_post_disable and
drm_bridge_chain_pre_enable
to reuse drm_atomic_bridge_chain_post_disable /
drm_atomic_bridge_chain_pre_enable
but with a NULL state.
- New patch that adds a pre_enable_upstream_first to drm_panel.
- changed from an OPS flag to a bool "pre_enable_upstream_first" in drm_bridge.
- Followed Andrzej's suggestion of using continue in the main loop to avoid
needing 2 additional loops (one forward to find the last bridge wanting
upstream first, and the second backwards again).
- Actioned Laurent's review comments on docs patch.

Original cover letter:

Hopefully I've cc'ed all those that have bashed this problem around previously,
or are otherwise linked to DRM bridges.

There have been numerous discussions around how DSI support is currently broken
as it doesn't support initialising the PHY to LP-11 and potentially the clock
lane to HS prior to configuring the DSI peripheral. There is no op where the
interface is initialised but HS video isn't also being sent.
Currently you have:
- peripheral pre_enable (host not initialised yet)
- host pre_enable
- encoder enable
- host enable
- peripheral enable (video already running)

vc4 and exynos currently implement the DSI host as an encoder, and split the
bridge_chain. This fails if you want to switch to being a bridge and/or use
atomic calls as the state of all the elements split off are not added by
drm_atomic_add_encoder_bridges.

dw-mipi-dsi[1] and now msm[2] use the mode_set hook to initialise the PHY, so
the bridge/panel pre_enable can send commands. In their post_disable they then
call the downstream bridge/panel post_disable op manually so that shutdown
commands can be sent before shutting down the PHY. Nothing handles that fact,
so the framework then continues down the bridge chain and calls the post_disable
again, so we get unbalanced panel prepare/unprepare calls being reported [3].

There have been patches[4] proposing reversing the entire direction of
pre_enable and post_disable, but that risks driving voltage into devices that
have yet to be powered up.
There have been discussions about adding either a pre_pre_enable, or adding a
DSI host_op to initialise the host[5]. Both require significant reworking to all
existing drivers in moving initialisation phases.
We have patches that look like they may well be addressing race conditions in
starting up a DSI peripheral[6].

This patch takes a hybrid of the two: an optional reversing of the order for
specific links within the bridge chain within pre_enable and post_disable done
within the drm_bridge framework.
I'm more than happy to move where the flag exists in structures (currently as
DRM_BRIDGE_OP_UPSTREAM_FIRST in drm_bridge_ops, but it isn't an op), but does
this solve the problem posed? If not, then can you describe the actual scenario
it doesn't cover?
A DSI peripheral can set the flag to get the DSI host initialised first, and
therefore it has a stable LP-11 state before pre_enable. Likewise the peripheral
can still send shutdown commands prior to the DSI host being shut down in
post_disable. It also handles the case where there are multiple devices in the
chain that all want their upstream bridge enabled first, so should there be a
DSI mux between host and peripheral, then it can still get the host to the
correct state.

An example tree is at [7] which is drm-misc-next with these patches and then a
conversion of vc4_dsi to use the atomic bridge functions (will be upstreamed
once we're over this hurdle). It is working happily with the Toshiba TC358762 on
a Raspberry Pi 7" panel.
The same approach but on our vendor 5.15 tree[8] has also been tested
successfully on a TI SN65DSI83 and LVDS panel.

Whilst here, I've also documented the expected behaviour of DSI hosts and
peripherals to aid those who come along after.

Thanks
Dave

[1]
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L940
[2] https://lists.freedesktop.org/archives/dri-devel/2022-January/337769.html
[3] https://lists.freedesktop.org/archives/dri-devel/2021-December/333908.html
[4] https://lists.freedesktop.org/archives/dri-devel/2021-October/328476.html
[5] https://lists.freedesktop.org/archives/dri-devel/2021-October/325853.html
[6] https://lists.freedesktop.org/archives/dri-devel/2022-February/341852.html
[7] https://github.com/6by9/linux/tree/drm-misc-next-vc4_dsi
[8] https://github.com/6by9/linux/tree/rpi-5.15.y-sn65dsi83

Dave Stevenson (3):
drm/bridge: Introduce pre_enable_prev_first to alter bridge init order
drm/panel: Add prepare_prev_first flag to drm_panel
drm/bridge:

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/mtl: Add hardware-level lock for steering

2022-12-02 Thread Balasubramani Vivekanandan

On 28.11.2022 15:30, Matt Roper wrote:
> Starting with MTL, the driver needs to not only protect the steering
> control register from simultaneous software accesses, but also protect
> against races with hardware/firmware agents.  The hardware provides a
> dedicated locking mechanism to support this via the MTL_STEER_SEMAPHORE
> register.  Reading the register acts as a 'trylock' operation; the read
> will return 0x1 if the lock is acquired or 0x0 if something else is
> already holding the lock; once acquired, writing 0x1 to the register
> will release the lock.
> 
> We'll continue to grab the software lock as well, just so lockdep can
> track our locking; assuming the hardware lock is behaving properly,
> there should never be any contention on the software lock in this case.
> 
> v2:
>  - Extend hardware semaphore timeout and add a taint for CI if it ever
>happens (this would imply misbehaving hardware/firmware).  (Mika)
>  - Add "MTL_" prefix to new steering semaphore register.  (Mika)
> 
> Cc: Mika Kuoppala 
> Signed-off-by: Matt Roper 

Reviewed-by: Balasubramani Vivekanandan 

Regards,
Bala

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_mcr.c  | 38 ++---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c 
> b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> index aa070ae57f11..087e4ac5b68d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c
> @@ -347,10 +347,9 @@ static u32 rw_with_mcr_steering(struct intel_gt *gt,
>   * @flags: storage to save IRQ flags to
>   *
>   * Performs locking to protect the steering for the duration of an MCR
> - * operation.  Depending on the platform, this may be a software lock
> - * (gt->mcr_lock) or a hardware lock (i.e., a register that synchronizes
> - * access not only for the driver, but also for external hardware and
> - * firmware agents).
> + * operation.  On MTL and beyond, a hardware lock will also be taken to
> + * serialize access not only for the driver, but also for external hardware 
> and
> + * firmware agents.
>   *
>   * Context: Takes gt->mcr_lock.  uncore->lock should *not* be held when this
>   *  function is called, although it may be acquired after this
> @@ -359,12 +358,40 @@ static u32 rw_with_mcr_steering(struct intel_gt *gt,
>  void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags)
>  {
>   unsigned long __flags;
> + int err = 0;
>  
>   lockdep_assert_not_held(>uncore->lock);
>  
> + /*
> +  * Starting with MTL, we need to coordinate not only with other
> +  * driver threads, but also with hardware/firmware agents.  A dedicated
> +  * locking register is used.
> +  */
> + if (GRAPHICS_VER(gt->i915) >= IP_VER(12, 70))
> + err = wait_for(intel_uncore_read_fw(gt->uncore,
> + MTL_STEER_SEMAPHORE) == 
> 0x1, 100);
> +
> + /*
> +  * Even on platforms with a hardware lock, we'll continue to grab
> +  * a software spinlock too for lockdep purposes.  If the hardware lock
> +  * was already acquired, there should never be contention on the
> +  * software lock.
> +  */
>   spin_lock_irqsave(>mcr_lock, __flags);
>  
>   *flags = __flags;
> +
> + /*
> +  * In theory we should never fail to acquire the HW semaphore; this
> +  * would indicate some hardware/firmware is misbehaving and not
> +  * releasing it properly.
> +  */
> + if (err == -ETIMEDOUT) {
> + drm_err_ratelimited(>i915->drm,
> + "GT%u hardware MCR steering semaphore timed 
> out",
> + gt->info.id);
> + add_taint_for_CI(gt->i915, TAINT_WARN);  /* CI is now 
> unreliable */
> + }
>  }
>  
>  /**
> @@ -379,6 +406,9 @@ void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long 
> *flags)
>  void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned long flags)
>  {
>   spin_unlock_irqrestore(>mcr_lock, flags);
> +
> + if (GRAPHICS_VER(gt->i915) >= IP_VER(12, 70))
> + intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 784152548472..1618d46cb8c7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -67,6 +67,7 @@
>  #define GMD_ID_MEDIA _MMIO(MTL_MEDIA_GSI_BASE + 
> 0xd8c)
>  
>  #define MCFG_MCR_SELECTOR_MMIO(0xfd0)
> +#define MTL_STEER_SEMAPHORE  _MMIO(0xfd0)
>  #define MTL_MCR_SELECTOR _MMIO(0xfd4)
>  #define SF_MCR_SELECTOR  _MMIO(0xfd8)
>  #define GEN8_MCR_SELECTOR_MMIO(0xfdc)
> -- 
> 2.38.1
>

[PATCH 2/2] vkms: Add support for multiple connectors

2022-12-02 Thread Marius Vlad

This patch adds support for creating multiple virtual connectors, in
case one would need it. Use module parameters to specify how many,
defaulting to just one, allocating from the start, a maximum of 4
possible outputs.

This is of particular importance when testing out the DRM backend in
compositors, but also to be able to independently handle multiple
outputs/connectors, like setting one to off/sleep on while the others
are on, and combinations that arise from that.

Signed-off-by: Marius Vlad 
---
 drivers/gpu/drm/vkms/vkms_crtc.c  |  3 +--
 drivers/gpu/drm/vkms/vkms_drv.c   | 26 ++
 drivers/gpu/drm/vkms/vkms_drv.h   |  8 +---
 drivers/gpu/drm/vkms/vkms_output.c|  5 ++---
 drivers/gpu/drm/vkms/vkms_writeback.c | 18 --
 5 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..0b6c40ac80b6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -89,8 +89,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
 {
struct drm_device *dev = crtc->dev;
unsigned int pipe = crtc->index;
-   struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
-   struct vkms_output *output = >output;
+   struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
struct drm_vblank_crtc *vblank = >vblank[pipe];
 
if (!READ_ONCE(vblank->enabled)) {
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 0ffe5f0e33f7..205546dafc70 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -51,13 +51,19 @@ static bool enable_overlay;
 module_param_named(enable_overlay, enable_overlay, bool, 0444);
 MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
 
+static int max_connectors = 1;
+module_param_named(max_connectors, max_connectors, int, 0444);
+MODULE_PARM_DESC(max_connectors, "Specify how many virtual connectors to 
create");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
 {
-   struct vkms_device *vkms = drm_device_to_vkms_device(dev);
+   int i;
+   struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 
-   destroy_workqueue(vkms->output.composer_workq);
+   for (i = 0; i < vkmsdev->config->max_connectors; i++)
+   destroy_workqueue(vkmsdev->output[i].composer_workq);
 }
 
 static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
@@ -98,6 +104,7 @@ static int vkms_config_show(struct seq_file *m, void *data)
seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
+   seq_printf(m, "connectors=%d\n", vkmsdev->config->max_connectors);
 
return 0;
 }
@@ -140,6 +147,7 @@ static const struct drm_mode_config_helper_funcs 
vkms_mode_config_helpers = {
 static int vkms_modeset_init(struct vkms_device *vkmsdev)
 {
struct drm_device *dev = >drm;
+   int i, ret = 0;
 
drm_mode_config_init(dev);
dev->mode_config.funcs = _mode_funcs;
@@ -155,7 +163,14 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
dev->mode_config.preferred_depth = 0;
dev->mode_config.helper_private = _mode_config_helpers;
 
-   return vkms_output_init(vkmsdev, 0);
+   for (i = 0; i < vkmsdev->config->max_connectors; i++) {
+   ret = vkms_output_init(vkmsdev, i);
+   if (ret)
+   return ret;
+   }
+
+   drm_mode_config_reset(dev);
+   return ret;
 }
 
 static int vkms_create(struct vkms_config *config)
@@ -191,7 +206,7 @@ static int vkms_create(struct vkms_config *config)
goto out_devres;
}
 
-   ret = drm_vblank_init(_device->drm, 1);
+   ret = drm_vblank_init(_device->drm, config->max_connectors);
if (ret) {
DRM_ERROR("Failed to vblank\n");
goto out_devres;
@@ -229,6 +244,9 @@ static int __init vkms_init(void)
config->cursor = enable_cursor;
config->writeback = enable_writeback;
config->overlay = enable_overlay;
+   config->max_connectors = max_connectors;
+   if (config->max_connectors > NUM_MAX_CONNECTORS)
+   config->max_connectors = NUM_MAX_CONNECTORS;
 
return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 0a67b8073f7e..fdea37db3b1d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -21,7 +21,8 @@
 #define XRES_MAX  8192
 #define YRES_MAX  8192
 
-#define NUM_OVERLAY_PLANES 8
+#define NUM_OVERLAY_PLANES 8
+#define NUM_MAX_CONNECTORS 4
 
 struct vkms_frame_info {
struct drm_framebuffer *fb;
@@ -113,6 +114,7 @@ struct vkms_config {
bool

[PATCH 1/2] vkms: Pass the correct bitmask for possible crtcs

2022-12-02 Thread Marius Vlad

In preparation of having multiple outputs/virtual connectors we need to
able to chose the correct encoder/connectors/crtc combination so pass also
the index as a bitmask as possible crtcs for the encoder.

Signed-off-by: Marius Vlad 
---
 drivers/gpu/drm/vkms/vkms_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_output.c 
b/drivers/gpu/drm/vkms/vkms_output.c
index 991857125bb4..ebb75ede65ab 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -96,7 +96,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
DRM_ERROR("Failed to init encoder\n");
goto err_encoder;
}
-   encoder->possible_crtcs = 1;
+   encoder->possible_crtcs = (1 << index);
 
ret = drm_connector_attach_encoder(connector, encoder);
if (ret) {
-- 
2.35.1

[PATCH 0/2] vkms: Add support for multiple connectors

2022-12-02 Thread Marius Vlad

With multiple outputs available we can perform management of outputs at
a more granular level, such that we're able to turn off or on, several
outputs at a time, or combinations that arise from doing that. 

The Weston project use VKMS when running its test suite in CI, and we
have now uses cases which would need to ability to set-up the outputs
DPMS/state individually, rather than globally -- which would affect all
outputs. This an attempt on fixing that by giving the possibility to
create more than one output, and thus allowing to run tests that could
exercise code paths in the compositor related to management of outputs.

Marius Vlad (2):
  vkms: Pass the correct bitmask for possible crtcs
  vkms: Add support for multiple connectors

 drivers/gpu/drm/vkms/vkms_crtc.c  |  3 +--
 drivers/gpu/drm/vkms/vkms_drv.c   | 26 ++
 drivers/gpu/drm/vkms/vkms_drv.h   |  8 +---
 drivers/gpu/drm/vkms/vkms_output.c|  7 +++
 drivers/gpu/drm/vkms/vkms_writeback.c | 18 --
 5 files changed, 39 insertions(+), 23 deletions(-)

-- 
2.35.1

Re: [PATCH v1 0/2] ASoC/tda998x: Fix reporting of nonexistent capture streams

2022-12-02 Thread Russell King (Oracle)

On Wed, Nov 30, 2022 at 06:46:42PM +, Mark Brown wrote:
> The recently added pcm-test selftest has pointed out that systems with
> the tda998x driver end up advertising that they support capture when in
> reality as far as I can see the tda998x devices are transmit only.  The
> DAIs registered through hdmi-codec are bidirectional, meaning that for
> I2S systems when combined with a typical bidrectional CPU DAI the
> overall capability of the PCM is bidirectional.  In most cases the I2S
> links will clock OK but no useful audio will be returned which isn't so
> bad but we should still not advertise the useless capability, and some
> systems may notice problems for example due to pinmux management.
> 
> This is happening due to the hdmi-codec helpers not providing any
> mechanism for indicating unidirectional audio so add one and use it in
> the tda998x driver.  It is likely other hdmi-codec users are also
> affected but I don't have those systems to hand.
> 
> Mark Brown (2):
>   ASoC: hdmi-codec: Allow playback and capture to be disabled
>   drm: tda99x: Don't advertise non-existent capture support
> 
>  drivers/gpu/drm/i2c/tda998x_drv.c |  2 ++
>  include/sound/hdmi-codec.h|  4 
>  sound/soc/codecs/hdmi-codec.c | 30 +-
>  3 files changed, 31 insertions(+), 5 deletions(-)

Looks sane.

Reviewed-by: Russell King (Oracle) 

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

[PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
respective drivers can use the vmap'ed GEM-buffer memory. Implement state
helpers, the {begin,end}_fb_access helpers and wire up everything.

With this commit, MIPI DBI drivers can access the GEM object's memory
that is provided by shadow-plane state. The actual changes to drivers
are implemented separately.

v2:
* use shadow-plane state directly (Noralf)

Signed-off-by: Thomas Zimmermann 
Tested-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes  # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 85 ++
 drivers/gpu/drm/tiny/ili9225.c |  5 ++
 drivers/gpu/drm/tiny/st7586.c  |  5 ++
 include/drm/drm_mipi_dbi.h | 16 ++-
 4 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index f58123327ed6..b808de61c5bc 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -433,6 +434,90 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe 
*pipe)
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
+/**
+ * mipi_dbi_pipe_begin_fb_access - MIPI DBI pipe begin-access helper
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct _simple_display_funcs.begin_fb_access.
+ *
+ * See drm_gem_begin_shadow_fb_access() for details and 
mipi_dbi_pipe_cleanup_fb()
+ * for cleanup.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
+ struct drm_plane_state *plane_state)
+{
+   return drm_gem_begin_shadow_fb_access(>plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_begin_fb_access);
+
+/**
+ * mipi_dbi_pipe_end_fb_access - MIPI DBI pipe end-access helper
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct _simple_display_funcs.end_fb_access.
+ *
+ * See mipi_dbi_pipe_begin_fb_access().
+ */
+void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
+struct drm_plane_state *plane_state)
+{
+   drm_gem_end_shadow_fb_access(>plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_end_fb_access);
+
+/**
+ * mipi_dbi_pipe_reset_plane - MIPI DBI plane-reset helper
+ * @pipe: Display pipe
+ *
+ * This function implements struct _simple_display_funcs.reset_plane
+ * for MIPI DBI planes.
+ */
+void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe)
+{
+   drm_gem_reset_shadow_plane(>plane);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_reset_plane);
+
+/**
+ * mipi_dbi_pipe_duplicate_plane_state - duplicates MIPI DBI plane state
+ * @pipe: Display pipe
+ *
+ * This function implements struct 
_simple_display_funcs.duplicate_plane_state
+ * for MIPI DBI planes.
+ *
+ * See drm_gem_duplicate_shadow_plane_state() for additional details.
+ *
+ * Returns:
+ * A pointer to a new plane state on success, or NULL otherwise.
+ */
+struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct 
drm_simple_display_pipe *pipe)
+{
+   return drm_gem_duplicate_shadow_plane_state(>plane);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_duplicate_plane_state);
+
+/**
+ * mipi_dbi_pipe_destroy_plane_state - cleans up MIPI DBI plane state
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct drm_simple_display_funcs.destroy_plane_state
+ * for MIPI DBI planes.
+ *
+ * See drm_gem_destroy_shadow_plane_state() for additional details.
+ */
+void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
+  struct drm_plane_state *plane_state)
+{
+   drm_gem_destroy_shadow_plane_state(>plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
+
 static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
 {
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index ae94c74d0163..a69aec8402bc 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -343,6 +343,11 @@ static const struct drm_simple_display_pipe_funcs 
ili9225_pipe_funcs = {
.enable = ili9225_pipe_enable,
.disable= ili9225_pipe_disable,
.update = ili9225_pipe_update,
+   .begin_fb_access = mipi_dbi_pipe_begin_fb_access,
+   .end_fb_access  = mipi_dbi_pipe_end_fb_access,
+   .reset_plane= mipi_dbi_pipe_reset_plane,
+   .duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
+   .destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
 };
 
 static const struct drm_display_mode ili9225_mode = {
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index e773b1f2fd5f..76b13cefc904

[PATCH v2 8/8] drm/mipi-dbi: Move drm_dev_{enter, exit}() out from fb_dirty functions

Call drm_dev_enter() and drm_dev_exit() in the outer-most callbacks
of the modesetting pipeline. If drm_dev_enter() fails, the driver can
thus avoid unnecessary work.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
Tested-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes  # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 13 +++--
 drivers/gpu/drm/tiny/ili9225.c | 13 +++--
 drivers/gpu/drm/tiny/st7586.c  | 13 +++--
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 0ef5e81ba5e1..eb27d000bcfb 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -259,13 +259,10 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, 
struct drm_framebuffer *fb,
unsigned int width = rect->x2 - rect->x1;
struct mipi_dbi *dbi = >dbi;
bool swap = dbi->swap_bytes;
-   int idx, ret = 0;
+   int ret = 0;
bool full;
void *tr;
 
-   if (!drm_dev_enter(fb->dev, ))
-   return;
-
full = width == fb->width && height == fb->height;
 
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, 
DRM_RECT_ARG(rect));
@@ -288,8 +285,6 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct 
drm_framebuffer *fb,
 err_msg:
if (ret)
drm_err_once(fb->dev, "Failed to update display %d\n", ret);
-
-   drm_dev_exit(idx);
 }
 
 /**
@@ -325,6 +320,7 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe 
*pipe,
struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct drm_rect rect;
+   int idx;
 
if (!pipe->crtc.state->active)
return;
@@ -332,8 +328,13 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe 
*pipe,
if (WARN_ON(!fb))
return;
 
+   if (!drm_dev_enter(fb->dev, ))
+   return;
+
if (drm_atomic_helper_damage_merged(old_state, state, ))
mipi_dbi_fb_dirty(_plane_state->data[0], fb, );
+
+   drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 0ba5177deca7..077c6ff5a2e1 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -87,13 +87,10 @@ static void ili9225_fb_dirty(struct iosys_map *src, struct 
drm_framebuffer *fb,
bool swap = dbi->swap_bytes;
u16 x_start, y_start;
u16 x1, x2, y1, y2;
-   int idx, ret = 0;
+   int ret = 0;
bool full;
void *tr;
 
-   if (!drm_dev_enter(fb->dev, ))
-   return;
-
full = width == fb->width && height == fb->height;
 
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, 
DRM_RECT_ARG(rect));
@@ -156,8 +153,6 @@ static void ili9225_fb_dirty(struct iosys_map *src, struct 
drm_framebuffer *fb,
 err_msg:
if (ret)
dev_err_once(fb->dev->dev, "Failed to update display %d\n", 
ret);
-
-   drm_dev_exit(idx);
 }
 
 static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -167,12 +162,18 @@ static void ili9225_pipe_update(struct 
drm_simple_display_pipe *pipe,
struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct drm_rect rect;
+   int idx;
 
if (!pipe->crtc.state->active)
return;
 
+   if (!drm_dev_enter(fb->dev, ))
+   return;
+
if (drm_atomic_helper_damage_merged(old_state, state, ))
ili9225_fb_dirty(_plane_state->data[0], fb, );
+
+   drm_dev_exit(idx);
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 53dca9272d4d..3cf4eec16a81 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -113,10 +113,7 @@ static void st7586_fb_dirty(struct iosys_map *src, struct 
drm_framebuffer *fb,
 {
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
struct mipi_dbi *dbi = >dbi;
-   int start, end, idx, ret = 0;
-
-   if (!drm_dev_enter(fb->dev, ))
-   return;
+   int start, end, ret = 0;
 
/* 3 pixels per byte, so grow clip to nearest multiple of 3 */
rect->x1 = rounddown(rect->x1, 3);
@@ -145,8 +142,6 @@ static void st7586_fb_dirty(struct iosys_map *src, struct 
drm_framebuffer *fb,
 err_msg:
if (ret)
dev_err_once(fb->dev->dev, "Failed to update display %d\n", 
ret);
-
-   drm_dev_exit(idx);
 }
 
 static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -156,12 +151,18 @@ static void st7586_pipe_update(struct 
drm_simple_display_pipe *pipe,
struct drm_shadow_plane_state

[PATCH v2 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers

Move the vmap/vunmap blocks from the inner fb_dirty helpers into the
MIPI DBI update helpers. The function calls can result in waiting and/or
processing overhead. Reduce the penalties by executing the functions once
in the outer-most function of the pipe update.

This change also prepares for MIPI DBI for shadow-plane helpers. With
shadow-plane helpers, transfer source buffers are mapped into kernel
address space automatically.

v2:
* keep each driver's existing buffer-mapping patter (Noralf)
* zero-initialize iosys_map arrays (Noralf)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
Tested-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes  # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 63 ++
 drivers/gpu/drm/tiny/ili9225.c | 25 ++
 drivers/gpu/drm/tiny/st7586.c  | 28 ++-
 include/drm/drm_mipi_dbi.h |  6 ++--
 4 files changed, 75 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index a6ac56580876..f58123327ed6 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -192,6 +192,7 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
 /**
  * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
  * @dst: The destination buffer
+ * @src: The source buffer
  * @fb: The source framebuffer
  * @clip: Clipping rectangle of the area to be copied
  * @swap: When true, swap MSB/LSB of 16-bit values
@@ -199,12 +200,10 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
  * Returns:
  * Zero on success, negative error code on failure.
  */
-int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
+int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer 
*fb,
  struct drm_rect *clip, bool swap)
 {
struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES];
struct iosys_map dst_map = IOSYS_MAP_INIT_VADDR(dst);
int ret;
 
@@ -212,19 +211,15 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer 
*fb,
if (ret)
return ret;
 
-   ret = drm_gem_fb_vmap(fb, map, data);
-   if (ret)
-   goto out_drm_gem_fb_end_cpu_access;
-
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
if (swap)
-   drm_fb_swab(_map, NULL, data, fb, clip, 
!gem->import_attach);
+   drm_fb_swab(_map, NULL, src, fb, clip, 
!gem->import_attach);
else
-   drm_fb_memcpy(_map, NULL, data, fb, clip);
+   drm_fb_memcpy(_map, NULL, src, fb, clip);
break;
case DRM_FORMAT_XRGB:
-   drm_fb_xrgb_to_rgb565(_map, NULL, data, fb, clip, swap);
+   drm_fb_xrgb_to_rgb565(_map, NULL, src, fb, clip, swap);
break;
default:
drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
@@ -232,8 +227,6 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
ret = -EINVAL;
}
 
-   drm_gem_fb_vunmap(fb, map);
-out_drm_gem_fb_end_cpu_access:
drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
return ret;
@@ -257,10 +250,9 @@ static void mipi_dbi_set_window_address(struct 
mipi_dbi_dev *dbidev,
 ys & 0xff, (ye >> 8) & 0xff, ye & 0xff);
 }
 
-static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect 
*rect)
+static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer 
*fb,
+ struct drm_rect *rect)
 {
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES];
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
unsigned int height = rect->y2 - rect->y1;
unsigned int width = rect->x2 - rect->x1;
@@ -270,16 +262,9 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, 
struct drm_rect *rect)
bool full;
void *tr;
 
-   if (WARN_ON(!fb))
-   return;
-
if (!drm_dev_enter(fb->dev, ))
return;
 
-   ret = drm_gem_fb_vmap(fb, map, data);
-   if (ret)
-   goto err_drm_dev_exit;
-
full = width == fb->width && height == fb->height;
 
DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, 
DRM_RECT_ARG(rect));
@@ -287,11 +272,11 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, 
struct drm_rect *rect)
if (!dbi->dc || !full || swap ||
fb->format->format == DRM_FORMAT_XRGB) {
tr = dbidev->tx_buf;
-   ret = mipi_dbi_buf_copy(dbidev->tx_buf, fb, rect, swap);
+   ret = mipi_dbi_buf_copy(tr, src, fb, rect, swap);
if (ret)
goto err_msg;

[PATCH v2 3/8] drm/st7586: Call MIPI DBI mode_valid helper

MIPI DBI drivers validate each mode against their native resolution.
Add this test to st7586.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
---
 drivers/gpu/drm/tiny/st7586.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index ce57fa9917e5..6bdd23e2a47c 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -263,6 +263,7 @@ static const u32 st7586_formats[] = {
 };
 
 static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
+   .mode_valid = mipi_dbi_pipe_mode_valid,
.enable = st7586_pipe_enable,
.disable= st7586_pipe_disable,
.update = st7586_pipe_update,
-- 
2.38.1

[PATCH v2 4/8] drm/mipi-dbi: Initialize default driver functions with macro

Introduce DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS to initialize MIPI-DBI
helpers to default values and convert drivers. The prepare_fb function
set by some drivers is called implicitly by simple-kms helpers, so leave
it out.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
Tested-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes  # drm/tiny/mi0283qt
---
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |  5 +
 drivers/gpu/drm/tiny/hx8357d.c   |  5 +
 drivers/gpu/drm/tiny/ili9163.c   |  5 +
 drivers/gpu/drm/tiny/ili9341.c   |  5 +
 drivers/gpu/drm/tiny/ili9486.c   |  5 +
 drivers/gpu/drm/tiny/mi0283qt.c  |  5 +
 drivers/gpu/drm/tiny/panel-mipi-dbi.c|  5 +
 drivers/gpu/drm/tiny/st7735r.c   |  5 +
 include/drm/drm_mipi_dbi.h   | 16 
 9 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index be088983aa7c..3fdf884b3257 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -577,10 +577,7 @@ static void ili9341_dbi_enable(struct 
drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9341_dbi_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = ili9341_dbi_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(ili9341_dbi_enable),
 };
 
 static const struct drm_display_mode ili9341_dbi_mode = {
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 9f634f720817..cdc4486e059b 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -181,10 +181,7 @@ static void yx240qv29_enable(struct 
drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = yx240qv29_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx350hv15_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index 835ed12792d5..bc4384d410fc 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -100,10 +100,7 @@ static void yx240qv29_enable(struct 
drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9163_pipe_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = yx240qv29_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 420f6005a956..47b61c3bf145 100644
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ b/drivers/gpu/drm/tiny/ili9341.c
@@ -137,10 +137,7 @@ static void yx240qv29_enable(struct 
drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = yx240qv29_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index 1bb847466b10..9f735d84d85d 100644
--- a/drivers/gpu/drm/tiny/ili9486.c
+++ b/drivers/gpu/drm/tiny/ili9486.c
@@ -150,10 +150,7 @@ static void waveshare_enable(struct 
drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs waveshare_pipe_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = waveshare_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(waveshare_enable),
 };
 
 static const struct drm_display_mode waveshare_mode = {
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index 47df2b5a3048..01ff43c8ac3f 100644
--- a/drivers/gpu/drm/tiny/mi0283qt.c
+++ b/drivers/gpu/drm/tiny/mi0283qt.c
@@ -141,10 +141,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe 
*pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
-   .mode_valid = mipi_dbi_pipe_mode_valid,
-   .enable = mi0283qt_enable,
-   .disable = mipi_dbi_pipe_disable,
-   .update = mipi_dbi_pipe_update,
+   DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(mi0283qt_enable),
 };
 
 static const struct drm_display_mode mi0283qt_mode = {
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c

[PATCH v2 7/8] drm/mipi-dbi: Use shadow-plane mappings

Use the buffer mappings provided by shadow-plane helpers. As the
mappings are established while the commit can still fail, errors
are now reported correctly to callers.

v2:
* use shadow-plane state directly (Noralf)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
Tested-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes  # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 25 +
 drivers/gpu/drm/tiny/ili9225.c | 16 
 drivers/gpu/drm/tiny/st7586.c  | 16 
 3 files changed, 13 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index b808de61c5bc..0ef5e81ba5e1 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -321,12 +321,10 @@ EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
 void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
  struct drm_plane_state *old_state)
 {
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
struct drm_plane_state *state = pipe->plane.state;
+   struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct drm_rect rect;
-   int ret;
 
if (!pipe->crtc.state->active)
return;
@@ -334,14 +332,8 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe 
*pipe,
if (WARN_ON(!fb))
return;
 
-   ret = drm_gem_fb_vmap(fb, map, data);
-   if (ret)
-   return;
-
if (drm_atomic_helper_damage_merged(old_state, state, ))
-   mipi_dbi_fb_dirty([0], fb, );
-
-   drm_gem_fb_vunmap(fb, map);
+   mipi_dbi_fb_dirty(_plane_state->data[0], fb, );
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
@@ -362,6 +354,7 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
   struct drm_crtc_state *crtc_state,
   struct drm_plane_state *plane_state)
 {
+   struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect rect = {
.x1 = 0,
@@ -369,22 +362,14 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
.y1 = 0,
.y2 = fb->height,
};
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
-   int idx, ret;
+   int idx;
 
if (!drm_dev_enter(>drm, ))
return;
 
-   ret = drm_gem_fb_vmap(fb, map, data);
-   if (ret)
-   goto err_drm_dev_exit;
-
-   mipi_dbi_fb_dirty([0], fb, );
+   mipi_dbi_fb_dirty(_plane_state->data[0], fb, );
backlight_enable(dbidev->backlight);
 
-   drm_gem_fb_vunmap(fb, map);
-err_drm_dev_exit:
drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_enable_flush);
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index a69aec8402bc..0ba5177deca7 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -164,19 +164,15 @@ static void ili9225_pipe_update(struct 
drm_simple_display_pipe *pipe,
struct drm_plane_state *old_state)
 {
struct drm_plane_state *state = pipe->plane.state;
+   struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
-   struct drm_gem_dma_object *dma_obj;
-   struct iosys_map src;
struct drm_rect rect;
 
if (!pipe->crtc.state->active)
return;
 
-   dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-   iosys_map_set_vaddr(, dma_obj->vaddr);
-
if (drm_atomic_helper_damage_merged(old_state, state, ))
-   ili9225_fb_dirty(, fb, );
+   ili9225_fb_dirty(_plane_state->data[0], fb, );
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -184,6 +180,7 @@ static void ili9225_pipe_enable(struct 
drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state)
 {
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+   struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
struct device *dev = pipe->crtc.dev->dev;
struct mipi_dbi *dbi = >dbi;
@@ -193,8 +190,6 @@ static void ili9225_pipe_enable(struct 
drm_simple_display_pipe *pipe,
.y1 = 0,
.y2 = fb->height,
};
-   struct drm_gem_dma_object *dma_obj;
-   struct iosys_map src;
int ret, idx;
u8 am_id;
 
@@ -285,10 +280,7 @@ static void ili9225_pipe_enable(struct 
drm_simple_display_pipe *pipe,

[PATCH v2 0/8] drm/mipi-dbi: Convert to shadow-plane helpers

Convert the MIPI-DBI-based drivers to shadow-plane helpers. The
drivers vmap/vunmap GEM buffer memory during the atomic commit.
Shadow-plane helpers automate this process.

Patches 1 to 4 prepare the MIPI code for the change and simplify
the rest of the patchset.

Patches 5 to 7 rework the vmap code in the MIPI-DBI drivers and add
shadow-plane helpers. Most of the affected drivers call MIPI-DBI
helpers and get the update automatically. Only ili9225 and st7586
require changes to their source code.

Patch 8 simplifies drm_dev_enter() and _exit(). It's not strictly
needed, but streamlines the driver code and make sense overall.

Testing is welcome, as I don't have any hardware to test these
changes myself.

v2:
* fix vmap()/vaddr confusion (Noralf)
* drop struct mipi_dbi_plane_state (Noralf)

Thomas Zimmermann (8):
  drm/simple-kms: Remove drm_gem_simple_display_pipe_prepare_fb()
  drm/ili9225: Call MIPI DBI mode_valid helper
  drm/st7586: Call MIPI DBI mode_valid helper
  drm/mipi-dbi: Initialize default driver functions with macro
  drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update
helpers
  drm/mipi-dbi: Support shadow-plane state
  drm/mipi-dbi: Use shadow-plane mappings
  drm/mipi-dbi: Move drm_dev_{enter,exit}() out from fb_dirty functions

 drivers/gpu/drm/drm_gem_atomic_helper.c  |  31 +---
 drivers/gpu/drm/drm_mipi_dbi.c   | 144 ++-
 drivers/gpu/drm/drm_simple_kms_helper.c  |   2 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |   6 +-
 drivers/gpu/drm/tiny/hx8357d.c   |   5 +-
 drivers/gpu/drm/tiny/ili9163.c   |   6 +-
 drivers/gpu/drm/tiny/ili9225.c   |  36 +++--
 drivers/gpu/drm/tiny/ili9341.c   |   5 +-
 drivers/gpu/drm/tiny/ili9486.c   |   5 +-
 drivers/gpu/drm/tiny/mi0283qt.c  |   5 +-
 drivers/gpu/drm/tiny/panel-mipi-dbi.c|   5 +-
 drivers/gpu/drm/tiny/st7586.c|  39 +++--
 drivers/gpu/drm/tiny/st7735r.c   |   5 +-
 include/drm/drm_gem_atomic_helper.h  |   2 -
 include/drm/drm_mipi_dbi.h   |  36 -
 include/drm/drm_plane.h  |   4 +-
 include/drm/drm_simple_kms_helper.h  |   4 +-
 17 files changed, 205 insertions(+), 135 deletions(-)


base-commit: ad232f8a0287b805a7a167eddad30fe33fbec9d5
-- 
2.38.1

[PATCH v2 1/8] drm/simple-kms: Remove drm_gem_simple_display_pipe_prepare_fb()

The helper drm_gem_simple_display_pipe_prepare_fb() is simple-KMS'
default implementation for prepare_fb. Remove the call from drivers
that set it explicitly. Then inline the helper into the only caller
within simple-kms helpers. No functional changes.

Simple-KMS drivers that implement the prepare_fb callback should call
drm_gem_plane_helper_prepare_fb() directly.

v2:
* fix typo in commit message

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_gem_atomic_helper.c  | 31 +---
 drivers/gpu/drm/drm_simple_kms_helper.c  |  2 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |  1 -
 drivers/gpu/drm/tiny/ili9163.c   |  1 -
 include/drm/drm_gem_atomic_helper.h  |  2 --
 include/drm/drm_plane.h  |  4 +--
 include/drm/drm_simple_kms_helper.h  |  4 +--
 7 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c 
b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e42800718f51..5d4b9cd077f7 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -26,11 +26,8 @@
  * call drm_gem_plane_helper_prepare_fb() from their implementation of
  * struct _plane_helper.prepare_fb . It sets the plane's fence from
  * the framebuffer so that the DRM core can synchronize access automatically.
- *
  * drm_gem_plane_helper_prepare_fb() can also be used directly as
- * implementation of prepare_fb. For drivers based on
- * struct drm_simple_display_pipe, drm_gem_simple_display_pipe_prepare_fb()
- * provides equivalent functionality.
+ * implementation of prepare_fb.
  *
  * .. code-block:: c
  *
@@ -41,11 +38,6 @@
  * . prepare_fb = drm_gem_plane_helper_prepare_fb,
  * };
  *
- * struct drm_simple_display_pipe_funcs driver_pipe_funcs = {
- * ...,
- * . prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
- * };
- *
  * A driver using a shadow buffer copies the content of the shadow buffers
  * into the HW's framebuffer memory during an atomic update. This requires
  * a mapping of the shadow buffer into kernel address space. The mappings
@@ -205,27 +197,6 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane 
*plane,
 }
 EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
 
-/**
- * drm_gem_simple_display_pipe_prepare_fb - prepare_fb helper for 
_simple_display_pipe
- * @pipe: Simple display pipe
- * @plane_state: Plane state
- *
- * This function uses drm_gem_plane_helper_prepare_fb() to extract the fences
- * from _gem_object.resv and attaches them to the plane state for the 
atomic
- * helper to wait on. This is necessary to correctly implement implicit
- * synchronization for any buffers shared as a struct _buf. Drivers can use
- * this as their _simple_display_pipe_funcs.prepare_fb callback.
- *
- * See drm_gem_plane_helper_prepare_fb() for a discussion of implicit and
- * explicit fencing in atomic modeset updates.
- */
-int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe 
*pipe,
-  struct drm_plane_state *plane_state)
-{
-   return drm_gem_plane_helper_prepare_fb(>plane, plane_state);
-}
-EXPORT_SYMBOL(drm_gem_simple_display_pipe_prepare_fb);
-
 /*
  * Shadow-buffered Planes
  */
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c 
b/drivers/gpu/drm/drm_simple_kms_helper.c
index 3ef420ec4534..270523ae36d4 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -267,7 +267,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane 
*plane,
 
WARN_ON_ONCE(pipe->funcs && pipe->funcs->cleanup_fb);
 
-   return drm_gem_simple_display_pipe_prepare_fb(pipe, state);
+   return drm_gem_plane_helper_prepare_fb(plane, state);
}
 
return pipe->funcs->prepare_fb(pipe, state);
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c 
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 384a724f2822..be088983aa7c 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -581,7 +581,6 @@ static const struct drm_simple_display_pipe_funcs 
ili9341_dbi_funcs = {
.enable = ili9341_dbi_enable,
.disable = mipi_dbi_pipe_disable,
.update = mipi_dbi_pipe_update,
-   .prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
 };
 
 static const struct drm_display_mode ili9341_dbi_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index ca0451f79962..835ed12792d5 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -104,7 +104,6 @@ static const struct drm_simple_display_pipe_funcs 
ili9163_pipe_funcs = {
.enable = yx240qv29_enable,
.disable = mipi_dbi_pipe_disable,
.update = mipi_dbi_pipe_update,
-   .prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
 };

[PATCH v2 2/8] drm/ili9225: Call MIPI DBI mode_valid helper

MIPI DBI drivers validate each mode against their native resolution.
Add this test to ili9225.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Noralf Trønnes 
---
 drivers/gpu/drm/tiny/ili9225.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 815bab285823..f05a2d25866c 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -326,6 +326,7 @@ static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 
*cmd, u8 *par,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
+   .mode_valid = mipi_dbi_pipe_mode_valid,
.enable = ili9225_pipe_enable,
.disable= ili9225_pipe_disable,
.update = ili9225_pipe_update,
-- 
2.38.1

Re: [Intel-gfx] [PATCH v6 3/5] drm/i915: Introduce guard pages to i915_vma

2022-12-02 Thread Tvrtko Ursulin




On 02/12/2022 11:11, Andi Shyti wrote:

Hi Tvrtko,

On Fri, Dec 02, 2022 at 10:20:11AM +, Tvrtko Ursulin wrote:


On 01/12/2022 20:39, Andi Shyti wrote:

From: Chris Wilson 

Introduce the concept of padding the i915_vma with guard pages before
and after. The major consequence is that all ordinary uses of i915_vma
must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
directly, as the drm_mm_node will include the guard pages that surround
our object.

The biggest connundrum is how exactly to mix requesting a fixed address
with guard pages, particularly through the existing uABI. The user does
not know about guard pages, so such must be transparent to the user, and
so the execobj.offset must be that of the object itself excluding the
guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
The caveat is that some placements will be impossible with guard pages,
as wrap arounds need to be avoided, and the vma itself will require a
larger node. We must not report EINVAL but ENOSPC as these are unavailable
locations within the GTT rather than conflicting user requirements.

In the next patch, we start using guard pages for scanout objects. While
these are limited to GGTT vma, on a few platforms these vma (or at least
an alias of the vma) is shared with userspace, so we may leak the
existence of such guards if we are not careful to ensure that the
execobj.offset is transparent and excludes the guards. (On such platforms
like ivb, without full-ppgtt, userspace has to use relocations so the
presence of more untouchable regions within its GTT such be of no further
issue.)

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
Hi Tvrtko,

I removed your r-b in this version because I restored the original value
of the guard being aligned with the vma size alignment. Turns out that
CI failed with the latest version because the guard was becoming too big
(we would have hit the GEM_BUG_ON)[*].

The reason why now the guard is aligned with the vma alignment is that
the area is already aligned and if we use as a starting address start +
guard, guard needs to be aligned, otherwise we screw up all the memory
alignment.

Let me know if it makes sense to you.


Reviewed-by: Tvrtko Ursulin 

Conditional to promise of a prioritised follow up improvement, if it turns
out GGTT wastage due a bit over zealous guard size comes to bite.


Sure! I'll be alert!

There are some unrelated failures from CI, just to be sure I sent
last night a trybot run.


Trybot looked okay, and I just pressed re-test for the intel-gfx series 
so lets see that too.


Regards,

Tvrtko

Re: [PATCH 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers

2022-12-02 Thread Noralf Trønnes




Den 02.12.2022 12.50, skrev Thomas Zimmermann:
> 
>>>
>>> You use drm_gem_fb_vmap() in the other places but here you access the
>>> object directly (and in the next hunk), but again not so important since
>>> it goes away in a later patch.
>>
>> I'll update this patch to use drm_gem_fb_vmap() consistently.
> 
> And after looking at the impact and churn, I rather go with the existing
> code that initializes from the GEM DMA object.
> 
> Noralf, is there a reason why most of MIPI DBI uses DMA helpers? In
> terms of flexibility and resource consumption, wouldn't SHMEM helpers be
> a better fit?
> 

The SHMEM helper didn't exist at the time. The SPI subsystem doesn't
have an interface for scatter/gather transfers and DMA is needed in
order to run at full speed. SPI does convert an is_vmalloc_addr() buffer
to an sg list of pages in spi_map_buf() so it solves the missing
interface that way.

I have never tried to pass a shmem buffer to spi_sync() so I don't know
if it works, but I guess that it will work.

Bare in mind that theses buffers are at most 400k in size so I'm not
sure there's much to gain in term of resources at least.

Noralf.

Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization

2022-12-02 Thread Marek Vasut


On 12/2/22 11:52, Marek Szyprowski wrote:

Hi,

Sorry for delay, I was on a sick leave last 2 weeks.

On 28.11.2022 15:43, Jagan Teki wrote:

,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut  wrote:

On 11/23/22 21:09, Jagan Teki wrote:

On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut  wrote:

On 11/17/22 14:04, Marek Szyprowski wrote:

On 17.11.2022 05:58, Marek Vasut wrote:

On 11/10/22 19:38, Jagan Teki wrote:

DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.

Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.

This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).

v8, v7, v6, v5:
* none

v4:
* update init handling to ensure host init done on first cmd transfer

v3:
* none

v2:
* check initialized state in samsung_dsim_init

v1:
* keep DSI init in host transfer

Signed-off-by: Marek Szyprowski 
Signed-off-by: Jagan Teki 
---
  drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
  include/drm/bridge/samsung-dsim.h |  5 +++--
  2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index bb1f45fd5a88..ec7e01ae02ea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
  disable_irq(dsi->irq);
  }
  -static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
flag)
  {
  const struct samsung_dsim_driver_data *driver_data =
dsi->driver_data;
  +if (dsi->state & flag)
+return 0;
+
  samsung_dsim_reset(dsi);
-samsung_dsim_enable_irq(dsi);
+
+if (!(dsi->state & DSIM_STATE_INITIALIZED))
+samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
  samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
samsung_dsim *dsi)
  samsung_dsim_set_phy_ctrl(dsi);
  samsung_dsim_init_link(dsi);
  +dsi->state |= flag;
+
  return 0;
  }
  @@ -1269,6 +1276,10 @@ static void
samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
  }
dsi->state |= DSIM_STATE_ENABLED;
+
+ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+if (ret)
+return;
  }
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t
samsung_dsim_host_transfer(struct mipi_dsi_host *host,
  if (!(dsi->state & DSIM_STATE_ENABLED))
  return -EINVAL;
  -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-ret = samsung_dsim_init(dsi);
-if (ret)
-return ret;
-dsi->state |= DSIM_STATE_INITIALIZED;
-}
+ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);

This triggers full controller reset and reprogramming upon first
command transfer, is such heavy handed reload really necessary ?

Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
DSI. If this is a real issue for you, then maybe the driver could do the
initialization conditionally, in prepare() callback in case of IMX and
on the first transfer in case of Exynos?

That's odd , it does actually break panel support for me, without this
double reset the panel works again. But I have to wonder, why would such
a full reset be necessary at all , even on the exynos ?

Is it breaking samsung_dsim_reset from host_transfer? maybe checking
whether a reset is required before calling it might fix the issue.  I
agree with double initialization is odd but it seems it is required on
some panels in Exynos, I think tweaking them and adjusting the panel
code might resolve this discrepancy.

Can someone provide further details on the exynos problem ?

If I'm correct this sequence is required in order to work the existing
panel/bridges on exynos. Adjusting these panel/bridge codes can
possibly fix the sequence further.

Marek Szyprowski, please add if you have anything.



Well, frankly speaking the double initialization is not a correct
sequence, but this is the only one that actually works on Exynos based
boards with DSI panels after moving the initialization to bridge's
.prepare() callback.


Somehow, I suspect this is

Re: [PATCH 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers

2022-12-02 Thread Noralf Trønnes




Den 02.12.2022 12.27, skrev Thomas Zimmermann:
> Hi
> 
> Am 25.11.22 um 18:39 schrieb Noralf Trønnes:
>>
>>
>> Den 21.11.2022 11.45, skrev Thomas Zimmermann:
>>> Move the vmap/vunmap blocks from the inner fb_dirty helpers into the
>>> MIPI DBI update helpers. The function calls can result in waiting and/or
>>> processing overhead. Reduce the penalties by executing the functions
>>> once
>>> in the outer-most function of the pipe update.
>>>
>>> This change also prepares for MIPI DBI for shadow-plane helpers. With
>>> shadow-plane helpers, transfer source buffers are mapped into kernel
>>> address space automatically.
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> ---

>>> @@ -303,9 +291,6 @@ static void mipi_dbi_fb_dirty(struct
>>> drm_framebuffer *fb, struct drm_rect *rect)
>>>   if (ret)
>>>   drm_err_once(fb->dev, "Failed to update display %d\n", ret);
>>>   -    drm_gem_fb_vunmap(fb, map);
>>> -
>>> -err_drm_dev_exit:
>>>   drm_dev_exit(idx);
>>>   }
>>>   @@ -338,14 +323,27 @@ EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
>>>   void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
>>>     struct drm_plane_state *old_state)
>>>   {
>>> +    struct iosys_map map[DRM_FORMAT_MAX_PLANES];
>>> +    struct iosys_map data[DRM_FORMAT_MAX_PLANES];
>>
>> These should have been zeroed to avoid UBSAN complaint, but you'll
>> remove these later so not so important.
> 
> Will be fixed.
> 
> But do you know how these warnings happen? I went through the helpers
> before and changed them to only access the format-info's relevant
> planes. No unintialized memory access should be reported.
> 

It happens with imported buffers, iosys_map_clear() looks at the
uninitialized boolean variable ->is_iomem:

drm_gem_fb_vmap -> ... -> dma_buf_vmap -> iosys_map_clear

static inline void iosys_map_clear(struct iosys_map *map)
{
if (map->is_iomem) {
map->vaddr_iomem = NULL;
map->is_iomem = false;
} else {
map->vaddr = NULL;
}
}

Noralf.

[RESEND PATCH linux-next 1/2] MAINTAINERS: Remove some obsolete drm drivers(tdfx, mga, i810, savage, r128, sis)

2022-12-02 Thread Cai Huoqing

Commit 399516ab0fee ("MAINTAINERS: Add a bunch of legacy (UMS) DRM drivers")
marked these drivers obsolete 7 years ago.
And the mesa UMD of these drm drivers already in deprecated list
in the link: https://docs.mesa3d.org/systems.html

3dfx Glide-->driver/gpu/drm/tdfx
Matrox-->driver/gpu/drm/mga
Intel i810-->driver/gpu/drm/i810
S3 Savage-->drivers/gpu/drm/savage/
ATI Rage 128->drivers/gpu/drm/r128/
Silicon Integrated Systems->drivers/gpu/drm/sis/

It's time to remove these.

Signed-off-by: Cai Huoqing 
---
 MAINTAINERS | 29 -
 1 file changed, 29 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3583c5f6889d..44c4f2d46cfa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6569,11 +6569,6 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
 F: drivers/gpu/drm/tiny/ili9486.c
 
-DRM DRIVER FOR INTEL I810 VIDEO CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/i810/
-F: include/uapi/drm/i810_drm.h
-
 DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
 M: Jagan Teki 
 S: Maintained
@@ -6602,11 +6597,6 @@ S:   Maintained
 F: 
Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
 F: drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
 
-DRM DRIVER FOR MATROX G200/G400 GRAPHICS CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/mga/
-F: include/uapi/drm/mga_drm.h
-
 DRM DRIVER FOR MGA G200 GRAPHICS CHIPS
 M: Dave Airlie 
 R: Thomas Zimmermann 
@@ -6725,11 +6715,6 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: drivers/gpu/drm/qxl/
 F: include/uapi/drm/qxl_drm.h
 
-DRM DRIVER FOR RAGE 128 VIDEO CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/r128/
-F: include/uapi/drm/r128_drm.h
-
 DRM DRIVER FOR RAYDIUM RM67191 PANELS
 M: Robert Chiras 
 S: Maintained
@@ -6757,11 +6742,6 @@ S:   Maintained
 F: 
Documentation/devicetree/bindings/display/panel/rocktech,jh057n00900.yaml
 F: drivers/gpu/drm/panel/panel-sitronix-st7703.c
 
-DRM DRIVER FOR SAVAGE VIDEO CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/savage/
-F: include/uapi/drm/savage_drm.h
-
 DRM DRIVER FOR FIRMWARE FRAMEBUFFERS
 M: Thomas Zimmermann 
 M: Javier Martinez Canillas 
@@ -6777,11 +6757,6 @@ F:   include/drm/drm_aperture.h
 F: include/linux/aperture.h
 F: include/video/nomodeset.h
 
-DRM DRIVER FOR SIS VIDEO CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/sis/
-F: include/uapi/drm/sis_drm.h
-
 DRM DRIVER FOR SITRONIX ST7586 PANELS
 M: David Lechner 
 S: Maintained
@@ -6809,10 +6784,6 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/ste,mcde.yaml
 F: drivers/gpu/drm/mcde/
 
-DRM DRIVER FOR TDFX VIDEO CARDS
-S: Orphan / Obsolete
-F: drivers/gpu/drm/tdfx/
-
 DRM DRIVER FOR TI DLPC3433 MIPI DSI TO DMD BRIDGE
 M: Jagan Teki 
 S: Maintained
-- 
2.25.1

[PATCH v6 7/8] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

This is a compute-only module marketed towards AI and vision
acceleration. This particular version can be found on the Amlogic A311D
SoC.

The feature bits are taken from the Khadas downstream kernel driver
6.4.4.3.310723AAA.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 44df273a5aae..66b8ad6c7d26 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -134,6 +134,37 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.minor_features10 = 0x90044250,
.minor_features11 = 0x0024,
},
+   {
+   .model = 0x8000,
+   .revision = 0x7120,
+   .product_id = 0x45080009,
+   .customer_id = 0x88,
+   .eco_id = 0,
+   .stream_count = 8,
+   .register_max = 64,
+   .thread_count = 256,
+   .shader_core_count = 1,
+   .vertex_cache_size = 16,
+   .vertex_output_buffer_size = 1024,
+   .pixel_pipes = 1,
+   .instruction_count = 512,
+   .num_constants = 320,
+   .buffer_size = 0,
+   .varyings_count = 16,
+   .features = 0xe0287cac,
+   .minor_features0 = 0xc1799eff,
+   .minor_features1 = 0xfefbfadb,
+   .minor_features2 = 0xeb9d6fbf,
+   .minor_features3 = 0xedfffced,
+   .minor_features4 = 0xd30dafc7,
+   .minor_features5 = 0x7b5ac333,
+   .minor_features6 = 0xfc8ee200,
+   .minor_features7 = 0x03fffa6f,
+   .minor_features8 = 0x00fe0ef0,
+   .minor_features9 = 0x0088003c,
+   .minor_features10 = 0x108048c0,
+   .minor_features11 = 0x0010,
+   },
 };
 
 bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
-- 
2.38.1

[PATCH v6 6/8] drm/etnaviv: Warn when probing on NPUs

Userspace is still not making full use of the hardware, so we don't know
yet if changes to the UAPI won't be needed. Warn about it.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..3cbc82bbf8d4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -765,6 +765,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
goto fail;
}
 
+   if (gpu->identity.nn_core_count > 0)
+   dev_warn(gpu->dev, "etnaviv has been instantiated on a NPU, "
+   "for which the UAPI is still 
experimental\n");
+
/* Exclude VG cores with FE2.0 */
if (gpu->identity.features & chipFeatures_PIPE_VG &&
gpu->identity.features & chipFeatures_FE20) {
-- 
2.38.1

[PATCH v6 5/8] drm/etnaviv: Add nn_core_count to chip feature struct

We will use these for differentiating between GPUs and NPUs, as the
downstream driver does.

Signed-off-by: Tomeu Vizoso 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h  | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..c8f3ad2031ce 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -50,6 +50,9 @@ struct etnaviv_chip_identity {
/* Number of shader cores. */
u32 shader_core_count;
 
+   /* Number of Neural Network cores. */
+   u32 nn_core_count;
+
/* Size of the vertex cache. */
u32 vertex_cache_size;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c 
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index f2fc645c7956..44df273a5aae 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -16,6 +16,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 128,
.shader_core_count = 1,
+   .nn_core_count = 0,
.vertex_cache_size = 8,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -47,6 +48,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -78,6 +80,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -109,6 +112,7 @@ static const struct etnaviv_chip_identity 
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 1024,
.shader_core_count = 4,
+   .nn_core_count = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 2,
-- 
2.38.1

[PATCH v6 0/8] Support for the NPU in Vim3

Hi,

This series adds support for the Verisilicon VIPNano-QI NPU in the A311D
as in the VIM3 board.

The IP is very closely based on previous Vivante GPUs, so the etnaviv
kernel driver works basically unchanged.

The userspace part of the driver is being reviewed at:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18986

v2: Move reference to RESET_NNA to npu node (Neil)
v3: Fix indentation mistake (Neil)
v4: Add warning when etnaviv probes on a NPU (Lucas)
v5: Reorder HWDB commit to be the last (Lucas)
v6: Add patch to move the power domain to the SoC-specific dtsi (Neil)

Regards,

Tomeu

Tomeu Vizoso (8):
  dt-bindings: reset: meson-g12a: Add missing NNA reset
  dt-bindings: power: Add G12A NNA power domain
  soc: amlogic: meson-pwrc: Add NNA power domain for A311D
  arm64: dts: Add DT node for the VIPNano-QI on the A311D
  drm/etnaviv: Add nn_core_count to chip feature struct
  drm/etnaviv: Warn when probing on NPUs
  drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055
  arm64: dts: Fix NPU power domain references in Amlogic G12-based SoCs

 .../boot/dts/amlogic/meson-g12-common.dtsi|  9 +
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  |  4 +++
 arch/arm64/boot/dts/amlogic/meson-g12b.dtsi   |  4 +++
 arch/arm64/boot/dts/amlogic/meson-sm1.dtsi|  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  3 ++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c| 35 +++
 drivers/soc/amlogic/meson-ee-pwrc.c   | 17 +
 include/dt-bindings/power/meson-g12a-power.h  |  1 +
 .../reset/amlogic,meson-g12a-reset.h  |  4 ++-
 10 files changed, 84 insertions(+), 1 deletion(-)

-- 
2.38.1

Re: [PATCH 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers





You use drm_gem_fb_vmap() in the other places but here you access the
object directly (and in the next hunk), but again not so important since
it goes away in a later patch.


I'll update this patch to use drm_gem_fb_vmap() consistently.


And after looking at the impact and churn, I rather go with the existing 
code that initializes from the GEM DMA object.


Noralf, is there a reason why most of MIPI DBI uses DMA helpers? In 
terms of flexibility and resource consumption, wouldn't SHMEM helpers be 
a better fit?


Best regards
Thomas





With the comments considered:

Reviewed-by: Noralf Trønnes 


Thanks.

Best regards
Thomas




  if (drm_atomic_helper_damage_merged(old_state, state, ))
-    st7586_fb_dirty(state->fb, );
+    st7586_fb_dirty(, fb, );
  }
  static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -176,6 +181,8 @@ static void st7586_pipe_enable(struct 
drm_simple_display_pipe *pipe,

  .y1 = 0,
  .y2 = fb->height,
  };
+    struct drm_gem_dma_object *dma_obj;
+    struct iosys_map src;
  int idx, ret;
  u8 addr_mode;
@@ -235,7 +242,10 @@ static void st7586_pipe_enable(struct 
drm_simple_display_pipe *pipe,

  msleep(100);
-    st7586_fb_dirty(fb, );
+    dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+    iosys_map_set_vaddr(, dma_obj->vaddr);
+
+    st7586_fb_dirty(, fb, );
  mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
  out_exit:
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 8c4ea7956d61d..36ac8495566b0 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -13,9 +13,10 @@
  #include 
  struct drm_rect;
-struct spi_device;
  struct gpio_desc;
+struct iosys_map;
  struct regulator;
+struct spi_device;
  /**
   * struct mipi_dbi - MIPI DBI interface
@@ -176,8 +177,9 @@ int mipi_dbi_command_read(struct mipi_dbi *dbi, 
u8 cmd, u8 *val);
  int mipi_dbi_command_buf(struct mipi_dbi *dbi, u8 cmd, u8 *data, 
size_t len);
  int mipi_dbi_command_stackbuf(struct mipi_dbi *dbi, u8 cmd, const 
u8 *data,

    size_t len);
-int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
+int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct 
drm_framebuffer *fb,

    struct drm_rect *clip, bool swap);
+
  /**
   * mipi_dbi_command - MIPI DCS command with optional parameter(s)
   * @dbi: MIPI DBI structure




--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v2 02/21] dt-bindings: display: tegra: vi: add 'vip' property and example

2022-12-02 Thread Krzysztof Kozlowski

On 02/12/2022 09:11, Luca Ceresoli wrote:
> Hello Rob,
> 
> On Thu, 1 Dec 2022 17:16:36 -0600
> Rob Herring  wrote:
> 
>> On Mon, Nov 28, 2022 at 04:23:17PM +0100, Luca Ceresoli wrote:
>>> The Tegra20 VI peripheral can receive parallel input from the VIP parallel
>>> input module. Add it to the allowed properties and augment the existing
>>> nvidia,tegra20-vi example to show a 'vip' property.
>>>
>>> Reviewed-by: Krzysztof Kozlowski 
>>> Signed-off-by: Luca Ceresoli 
>>>
>>> ---
>>>
>>> Changed in v2 (suggested by Krzysztof Kozlowski):
>>> - rename "i2c3" -> "ic2"
>>> - add review tag
>>> ---
>>>  .../display/tegra/nvidia,tegra20-vi.yaml  | 68 +++
>>>  MAINTAINERS   |  1 +
>>>  2 files changed, 69 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml 
>>> b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
>>> index 782a4b10150a..5b5583c2b562 100644
>>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
>>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
>>> @@ -74,6 +74,22 @@ properties:
>>>avdd-dsi-csi-supply:
>>>  description: DSI/CSI power supply. Must supply 1.2 V.
>>>  
>>> +  vip:
>>> +$ref: /schemas/display/tegra/nvidia,tegra20-vip.yaml
>>> +
>>> +  ports:
>>> +$ref: /schemas/graph.yaml#/properties/ports
>>> +
>>> +properties:
>>> +  port@0:
>>> +$ref: /schemas/graph.yaml#/properties/port
>>> +description:
>>> +  Input from the VIP (parallel input capture) module
>>> +
>>> +properties:
>>> +  endpoint:
>>> +$ref: /schemas/graph.yaml#/properties/endpoint  
>>
>> You can drop 'endpoint'. You only need port nodes if there's no extra 
>> properties in the endpoints.
> 
> Oh, nice, will remove in v3.
> 
> Krzysztof, can I keep your Reviewed-by after this change?

Yes.

Best regards,
Krzysztof

Re: [PATCH 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers


Hi

Am 25.11.22 um 18:39 schrieb Noralf Trønnes:



Den 21.11.2022 11.45, skrev Thomas Zimmermann:

Move the vmap/vunmap blocks from the inner fb_dirty helpers into the
MIPI DBI update helpers. The function calls can result in waiting and/or
processing overhead. Reduce the penalties by executing the functions once
in the outer-most function of the pipe update.

This change also prepares for MIPI DBI for shadow-plane helpers. With
shadow-plane helpers, transfer source buffers are mapped into kernel
address space automatically.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/drm_mipi_dbi.c | 60 +++---
  drivers/gpu/drm/tiny/ili9225.c | 31 ++
  drivers/gpu/drm/tiny/st7586.c  | 28 +++-
  include/drm/drm_mipi_dbi.h |  6 ++--
  4 files changed, 81 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index a6ac565808765..40e59a3a6481e 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -192,6 +192,7 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
  /**
   * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
   * @dst: The destination buffer
+ * @src: The source buffer
   * @fb: The source framebuffer
   * @clip: Clipping rectangle of the area to be copied
   * @swap: When true, swap MSB/LSB of 16-bit values
@@ -199,12 +200,13 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
   * Returns:
   * Zero on success, negative error code on failure.
   */
-int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
+int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer 
*fb,
  struct drm_rect *clip, bool swap)
  {
struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES];
+   struct iosys_map data[DRM_FORMAT_MAX_PLANES] = {
+   *src,
+   };


I would prefer that you used src directly when calling the drm_fb_*
functions, data can be anything and to me it isn't clear what that
contains when I see it (ofc now I know, but next year I've forgotten).
And is the array necessary, this helper will only ever support one color
plane after all?


Will be fixed.




struct iosys_map dst_map = IOSYS_MAP_INIT_VADDR(dst);
int ret;
  
@@ -212,10 +214,6 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,

if (ret)
return ret;
  
-	ret = drm_gem_fb_vmap(fb, map, data);

-   if (ret)
-   goto out_drm_gem_fb_end_cpu_access;
-
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
if (swap)
@@ -232,8 +230,6 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
ret = -EINVAL;
}
  
-	drm_gem_fb_vunmap(fb, map);

-out_drm_gem_fb_end_cpu_access:
drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
  
  	return ret;

@@ -257,10 +253,9 @@ static void mipi_dbi_set_window_address(struct 
mipi_dbi_dev *dbidev,
 ys & 0xff, (ye >> 8) & 0xff, ye & 0xff);
  }
  
-static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)

+static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer 
*fb,
+ struct drm_rect *rect)
  {
-   struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES];
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
unsigned int height = rect->y2 - rect->y1;
unsigned int width = rect->x2 - rect->x1;
@@ -270,16 +265,9 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, 
struct drm_rect *rect)
bool full;
void *tr;
  
-	if (WARN_ON(!fb))

-   return;
-
if (!drm_dev_enter(fb->dev, ))
return;
  
-	ret = drm_gem_fb_vmap(fb, map, data);

-   if (ret)
-   goto err_drm_dev_exit;
-
full = width == fb->width && height == fb->height;
  
  	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));

@@ -287,11 +275,11 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, 
struct drm_rect *rect)
if (!dbi->dc || !full || swap ||
fb->format->format == DRM_FORMAT_XRGB) {
tr = dbidev->tx_buf;
-   ret = mipi_dbi_buf_copy(dbidev->tx_buf, fb, rect, swap);
+   ret = mipi_dbi_buf_copy(tr, src, fb, rect, swap);
if (ret)
goto err_msg;
} else {
-   tr = data[0].vaddr; /* TODO: Use mapping abstraction properly */
+   tr = src->vaddr; /* TODO: Use mapping abstraction properly */
}
  
  	mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1,

@@ -303,9 +291,6 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, 
struct drm_rect *rect)
if (ret)

Re: [Intel-gfx] [PATCH v6 3/5] drm/i915: Introduce guard pages to i915_vma

2022-12-02 Thread Andi Shyti

Hi Tvrtko,

On Fri, Dec 02, 2022 at 10:20:11AM +, Tvrtko Ursulin wrote:
> 
> On 01/12/2022 20:39, Andi Shyti wrote:
> > From: Chris Wilson 
> > 
> > Introduce the concept of padding the i915_vma with guard pages before
> > and after. The major consequence is that all ordinary uses of i915_vma
> > must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
> > directly, as the drm_mm_node will include the guard pages that surround
> > our object.
> > 
> > The biggest connundrum is how exactly to mix requesting a fixed address
> > with guard pages, particularly through the existing uABI. The user does
> > not know about guard pages, so such must be transparent to the user, and
> > so the execobj.offset must be that of the object itself excluding the
> > guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
> > The caveat is that some placements will be impossible with guard pages,
> > as wrap arounds need to be avoided, and the vma itself will require a
> > larger node. We must not report EINVAL but ENOSPC as these are unavailable
> > locations within the GTT rather than conflicting user requirements.
> > 
> > In the next patch, we start using guard pages for scanout objects. While
> > these are limited to GGTT vma, on a few platforms these vma (or at least
> > an alias of the vma) is shared with userspace, so we may leak the
> > existence of such guards if we are not careful to ensure that the
> > execobj.offset is transparent and excludes the guards. (On such platforms
> > like ivb, without full-ppgtt, userspace has to use relocations so the
> > presence of more untouchable regions within its GTT such be of no further
> > issue.)
> > 
> > Signed-off-by: Chris Wilson 
> > Signed-off-by: Tejas Upadhyay 
> > Signed-off-by: Tvrtko Ursulin 
> > Signed-off-by: Andi Shyti 
> > ---
> > Hi Tvrtko,
> > 
> > I removed your r-b in this version because I restored the original value
> > of the guard being aligned with the vma size alignment. Turns out that
> > CI failed with the latest version because the guard was becoming too big
> > (we would have hit the GEM_BUG_ON)[*].
> > 
> > The reason why now the guard is aligned with the vma alignment is that
> > the area is already aligned and if we use as a starting address start +
> > guard, guard needs to be aligned, otherwise we screw up all the memory
> > alignment.
> > 
> > Let me know if it makes sense to you.
> 
> Reviewed-by: Tvrtko Ursulin 
> 
> Conditional to promise of a prioritised follow up improvement, if it turns
> out GGTT wastage due a bit over zealous guard size comes to bite.

Sure! I'll be alert!

There are some unrelated failures from CI, just to be sure I sent
last night a trybot run.

Thanks, Tvrtko!

Andi

Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization

2022-12-02 Thread Marek Szyprowski

Hi,

Sorry for delay, I was on a sick leave last 2 weeks.

On 28.11.2022 15:43, Jagan Teki wrote:
> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut  wrote:
>> On 11/23/22 21:09, Jagan Teki wrote:
>>> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut  wrote:
 On 11/17/22 14:04, Marek Szyprowski wrote:
> On 17.11.2022 05:58, Marek Vasut wrote:
>> On 11/10/22 19:38, Jagan Teki wrote:
>>> DSI host initialization handling in previous exynos dsi driver has
>>> some pitfalls. It initializes the host during host transfer() hook
>>> that is indeed not the desired call flow for I2C and any other DSI
>>> configured downstream bridges.
>>>
>>> Host transfer() is usually triggered for downstream DSI panels or
>>> bridges and I2C-configured-DSI bridges miss these host initialization
>>> as these downstream bridges use bridge operations hooks like pre_enable,
>>> and enable in order to initialize or set up the host.
>>>
>>> This patch is trying to handle the host init handler to satisfy all
>>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
>>> flag to ensure that host init is also done on first cmd transfer, this
>>> helps existing DSI panels work on exynos platform (form Marek
>>> Szyprowski).
>>>
>>> v8, v7, v6, v5:
>>> * none
>>>
>>> v4:
>>> * update init handling to ensure host init done on first cmd transfer
>>>
>>> v3:
>>> * none
>>>
>>> v2:
>>> * check initialized state in samsung_dsim_init
>>>
>>> v1:
>>> * keep DSI init in host transfer
>>>
>>> Signed-off-by: Marek Szyprowski 
>>> Signed-off-by: Jagan Teki 
>>> ---
>>>  drivers/gpu/drm/bridge/samsung-dsim.c | 25 
>>> +
>>>  include/drm/bridge/samsung-dsim.h |  5 +++--
>>>  2 files changed, 20 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index bb1f45fd5a88..ec7e01ae02ea 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
>>> samsung_dsim *dsi)
>>>  disable_irq(dsi->irq);
>>>  }
>>>  -static int samsung_dsim_init(struct samsung_dsim *dsi)
>>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int
>>> flag)
>>>  {
>>>  const struct samsung_dsim_driver_data *driver_data =
>>> dsi->driver_data;
>>>  +if (dsi->state & flag)
>>> +return 0;
>>> +
>>>  samsung_dsim_reset(dsi);
>>> -samsung_dsim_enable_irq(dsi);
>>> +
>>> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
>>> +samsung_dsim_enable_irq(dsi);
>>>if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>>>  samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
>>> samsung_dsim *dsi)
>>>  samsung_dsim_set_phy_ctrl(dsi);
>>>  samsung_dsim_init_link(dsi);
>>>  +dsi->state |= flag;
>>> +
>>>  return 0;
>>>  }
>>>  @@ -1269,6 +1276,10 @@ static void
>>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>>  }
>>>dsi->state |= DSIM_STATE_ENABLED;
>>> +
>>> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
>>> +if (ret)
>>> +return;
>>>  }
>>>static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
>>> @@ -1458,12 +1469,9 @@ static ssize_t
>>> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
>>>  if (!(dsi->state & DSIM_STATE_ENABLED))
>>>  return -EINVAL;
>>>  -if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>>> -ret = samsung_dsim_init(dsi);
>>> -if (ret)
>>> -return ret;
>>> -dsi->state |= DSIM_STATE_INITIALIZED;
>>> -}
>>> +ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
>> This triggers full controller reset and reprogramming upon first
>> command transfer, is such heavy handed reload really necessary ?
> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM
> DSI. If this is a real issue for you, then maybe the driver could do the
> initialization conditionally, in prepare() callback in case of IMX and
> on the first transfer in case of Exynos?
 That's odd , it does actually break panel support for me, without this
 double reset the panel works again. But I have to wonder, why would such
 a full reset be necessary at all , even on the exynos ?
>>> Is it breaking samsung_dsim_reset from host_transfer? maybe checking
>>> whether a reset is required before calling it

Re: [PATCH v2 12/12] arm64: dts: qcom: sm6115: Add smmu fallback to qcom generic compatible

2022-12-02 Thread Konrad Dybcio




On 30.11.2022 21:09, Adam Skladowski wrote:
> Add fallback to generic qcom mmu-500 implementation.
> 
> Signed-off-by: Adam Skladowski 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi 
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 38b903592a57..572bf04adf90 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -1233,7 +1233,7 @@ dispcc: clock-controller@5f0 {
>   };
>  
>   apps_smmu: iommu@c60 {
> - compatible = "qcom,sm6115-smmu-500", "arm,mmu-500";
> + compatible = "qcom,sm6115-smmu-500", "qcom,smmu-500", 
> "arm,mmu-500";
>   reg = <0x0c60 0x8>;
>   #iommu-cells = <2>;
>   #global-interrupts = <1>;

Re: [PATCH v2 10/12] arm64: dts: qcom: sm6115: Add i2c/spi nodes

2022-12-02 Thread Konrad Dybcio




On 30.11.2022 21:09, Adam Skladowski wrote:
> Add I2C/SPI nodes for SM6115.
> 
> Signed-off-by: Adam Skladowski 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 290 +++
>  1 file changed, 290 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi 
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index b30a5485671d..e676b9d117e3 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -365,6 +366,90 @@ tlmm: pinctrl@50 {
>   interrupt-controller;
>   #interrupt-cells = <2>;
>  
> + qup_i2c0_default: qup-i2c0-default-state {
> + pins = "gpio0", "gpio1";
> + function = "qup0";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_i2c1_default: qup-i2c1-default-state {
> + pins = "gpio4", "gpio5";
> + function = "qup1";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_i2c2_default: qup-i2c2-default-state {
> + pins = "gpio6", "gpio7";
> + function = "qup2";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_i2c3_default: qup-i2c3-default-state {
> + pins = "gpio8", "gpio9";
> + function = "qup3";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_i2c4_default: qup-i2c4-default-state {
> + pins = "gpio12", "gpio13";
> + function = "qup4";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_i2c5_default: qup-i2c5-default-state {
> + pins = "gpio14", "gpio15";
> + function = "qup5";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi0_default: qup-spi0-default-state {
> + pins = "gpio0", "gpio1","gpio2", "gpio3";
> + function = "qup0";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi1_default: qup-spi1-default-state {
> + pins = "gpio4", "gpio5", "gpio69", "gpio70";
> + function = "qup1";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi2_default: qup-spi2-default-state {
> + pins = "gpio6", "gpio7", "gpio71", "gpio80";
> + function = "qup2";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi3_default: qup-spi3-default-state {
> + pins = "gpio8", "gpio9", "gpio10", "gpio11";
> + function = "qup3";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi4_default: qup-spi4-default-state {
> + pins = "gpio12", "gpio13", "gpio96", "gpio97";
> + function = "qup4";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + qup_spi5_default: qup-spi5-default-state {
> + pins = "gpio14", "gpio15", "gpio16", "gpio17";
> + function = "qup5";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
>   sdc1_state_on: sdc1-on-state {
>   clk-pins {
>   pins = "sdc1_clk";
> @@ -701,6 +786,211 @@ gpi_dma0: dma-controller@4a0 {
>   status = "disabled";
>   };
>  
> + qupv3_id_0: geniqup@4ac {
> + compatible = "qcom,geni-se-qup";
> + reg = <0x04ac 0x2000>;
> +

Re: [PATCH] [RFC] drm/etnaviv: Disable softpin

Am Freitag, dem 02.12.2022 um 11:20 +0100 schrieb Marek Vasut:
> On 12/2/22 09:59, Lucas Stach wrote:
> > Am Freitag, dem 02.12.2022 um 00:21 +0100 schrieb Marek Vasut:
> > > Currently softpin suffers from assorted race conditions exposed by newer
> > > versions of mesa 22.2.y and 22.3.y . Those races are difficult to fix in
> > > older kernel versions due to massive amount of backports necessary to do
> > > so. Disable softpin by default until Linux 6.1.y is out, which contains
> > > the necessary fixes to make softpin work reliably.
> > > 
> > Sorry, but that's a NACK. The userspace driver depends on softpin for
> > GPUs with texture descriptors, so this introduces a hard functional
> > regression for those GPUs. I.e. they would go from "require race fixes
> > that are already on the way to upstream" to not working at all.
> 
> I expected that NAK.
> 
> But then, what options do we have here, except for a massive convoluted 
> backport, which might bring bugs of its own ?

There is no other option. The kernel driver in the LTS releases does
have a buggy softpin implementation. If someone cares about being able
to keep their system on the LTS kernel, then that someone either needs
to carry out the backports of the fixes with the necessary diligence or
sponsor someone to do it.

Regards,
Lucas

[PATCH v5 5/7] arm64: dts: renesas: white-hawk-cpu: Add DP output support

Add DT nodes needed for the mini DP connector. The DP is driven by
sn65dsi86, which in turn gets the pixel data from the SoC via DSI.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  | 94 +++
 1 file changed, 94 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi 
b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
index c10740aee9f6..8aab859aac7a 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0-white-hawk-cpu.dtsi
@@ -97,6 +97,15 @@ memory@6 {
reg = <0x6 0x 0x1 0x>;
};
 
+   reg_1p2v: regulator-1p2v {
+   compatible = "regulator-fixed";
+   regulator-name = "fixed-1.2V";
+   regulator-min-microvolt = <120>;
+   regulator-max-microvolt = <120>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+
reg_1p8v: regulator-1p8v {
compatible = "regulator-fixed";
regulator-name = "fixed-1.8V";
@@ -114,6 +123,24 @@ reg_3p3v: regulator-3p3v {
regulator-boot-on;
regulator-always-on;
};
+
+   mini-dp-con {
+   compatible = "dp-connector";
+   label = "CN5";
+   type = "mini";
+
+   port {
+   mini_dp_con_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+
+   sn65dsi86_refclk: clk-x6 {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <3840>;
+   };
 };
 
  {
@@ -134,6 +161,23 @@ phy0: ethernet-phy@0 {
};
 };
 
+ {
+   status = "okay";
+
+   ports {
+   port@1 {
+   dsi0_out: endpoint {
+   remote-endpoint = <_in>;
+   data-lanes = <1 2 3 4>;
+   };
+   };
+   };
+};
+
+ {
+   status = "okay";
+};
+
 _clk {
clock-frequency = <1666>;
 };
@@ -172,6 +216,51 @@ eeprom@50 {
};
 };
 
+ {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+
+   status = "okay";
+   clock-frequency = <40>;
+
+   bridge@2c {
+   compatible = "ti,sn65dsi86";
+   reg = <0x2c>;
+
+   clocks = <_refclk>;
+   clock-names = "refclk";
+
+   interrupt-parent = <_ex>;
+   interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+   enable-gpios = < 26 GPIO_ACTIVE_HIGH>;
+
+   vccio-supply = <_1p8v>;
+   vpll-supply = <_1p8v>;
+   vcca-supply = <_1p2v>;
+   vcc-supply = <_1p2v>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   sn65dsi86_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   sn65dsi86_out: endpoint {
+   remote-endpoint = <_dp_con_in>;
+   };
+   };
+   };
+   };
+};
+
  {
pinctrl-0 = <_pins>;
pinctrl-1 = <_pins>;
@@ -221,6 +310,11 @@ i2c0_pins: i2c0 {
function = "i2c0";
};
 
+   i2c1_pins: i2c1 {
+   groups = "i2c1";
+   function = "i2c1";
+   };
+
keys_pins: keys {
pins = "GP_5_0", "GP_5_1", "GP_5_2";
bias-pull-up;
-- 
2.34.1

[PATCH v5 2/7] dt-bindings: display: bridge: renesas, dsi-csi2-tx: Add r8a779g0

Extend the Renesas DSI display bindings to support the r8a779g0 V4H.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Acked-by: Krzysztof Kozlowski 
Reviewed-by: Laurent Pinchart 
---
 .../bindings/display/bridge/renesas,dsi-csi2-tx.yaml   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml 
b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
index afeeb967393d..d33026f85e19 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
@@ -11,13 +11,14 @@ maintainers:
 
 description: |
   This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
-  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
+  R-Car Gen4 SoCs. The encoder can operate in either DSI or CSI-2 mode, with up
   to four data lanes.
 
 properties:
   compatible:
 enum:
   - renesas,r8a779a0-dsi-csi2-tx# for V3U
+  - renesas,r8a779g0-dsi-csi2-tx# for V4H
 
   reg:
 maxItems: 1
-- 
2.34.1

RE: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code

2022-12-02 Thread nobuhiro1.iwamatsu

> -Original Message-
> From: Uwe Kleine-König 
> Sent: Thursday, December 1, 2022 12:22 AM
> To: Thierry Reding 
> Cc: Conor Dooley ; Linus Walleij
> ; Bartosz Golaszewski ; Douglas
> Anderson ; Pavel Machek ;
> Claudiu Beznea ; Nicolas Ferre
> ; Alexandre Belloni
> ; Ray Jui ; Scott
> Branden ; Broadcom internal kernel review list
> ; Benson Leung
> ; Guenter Roeck ; Shawn
> Guo ; Sascha Hauer ;
> Pengutronix Kernel Team ; Fabio Estevam
> ; NXP Linux Team ; Kevin
> Hilman ; Jerome Brunet ;
> Martin Blumenstingl ; Matthias
> Brugger ; Florian Fainelli ;
> Heiko Stuebner ; Palmer Dabbelt
> ; Paul Walmsley ;
> Michael Walle ; Orson Zhai ;
> Baolin Wang ; Chunyan Zhang
> ; Fabrice Gasnier ;
> Maxime Coquelin ; Alexandre Torgue
> ; Chen-Yu Tsai ; Samuel
> Holland ; Hammer Hsieh
> ; iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣ
> Ｔ) ; Sean Anderson
> ; Michal Simek ;
> Bjorn Andersson ; Stephen Boyd
> ; Matthias Kaehlcke ; Satya
> Priya ; linux-...@vger.kernel.org;
> linux-g...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> linux-l...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> chrome-platf...@lists.linux.dev; linux-amlo...@lists.infradead.org;
> linux-media...@lists.infradead.org; linux-rpi-ker...@lists.infradead.org;
> linux-rockc...@lists.infradead.org; linux-ri...@lists.infradead.org;
> linux-st...@st-md-mailman.stormreply.com; linux-su...@lists.linux.dev
> Subject: [PATCH v2 01/11] pwm: Make .get_state() callback return an error
> code
> 
> .get_state() might fail in some cases. To make it possible that a driver 
> signals
> such a failure change the prototype of .get_state() to return an error code.
> 
> This patch was created using coccinelle and the following semantic patch:
> 
> @p1@
> identifier getstatefunc;
> identifier driver;
> @@
>  struct pwm_ops driver = {
> ...,
> .get_state = getstatefunc
> ,...
>  };
> 
> @p2@
> identifier p1.getstatefunc;
> identifier chip, pwm, state;
> @@
> -void
> +int
>  getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct
> pwm_state *state)  {
>...
> -  return;
> +  return 0;
>...
>  }
> 
> plus the actual change of the prototype in include/linux/pwm.h (plus some
> manual fixing of indentions and empty lines).
> 
> So for now all drivers return success unconditionally. They are adapted in the
> following patches to make the changes easier reviewable.
> 
> Signed-off-by: Uwe Kleine-König 



> a/drivers/pwm/pwm-visconti.c b/drivers/pwm/pwm-visconti.c index
> 927c4cbb1daf..e3fb79b3e2a7 100644
> --- a/drivers/pwm/pwm-visconti.c
> +++ b/drivers/pwm/pwm-visconti.c
> @@ -103,8 +103,8 @@ static int visconti_pwm_apply(struct pwm_chip *chip,
> struct pwm_device *pwm,
>   return 0;
>  }
> 
> -static void visconti_pwm_get_state(struct pwm_chip *chip, struct
> pwm_device *pwm,
> -struct pwm_state *state)
> +static int visconti_pwm_get_state(struct pwm_chip *chip, struct
> pwm_device *pwm,
> +   struct pwm_state *state)
>  {
>   struct visconti_pwm_chip *priv = visconti_pwm_from_chip(chip);
>   u32 period, duty, pwmc0, pwmc0_clk;
> @@ -122,6 +122,8 @@ static void visconti_pwm_get_state(struct pwm_chip
> *chip, struct pwm_device *pwm
>   state->polarity = PWM_POLARITY_NORMAL;
> 
>   state->enabled = true;
> +
> + return 0;
>  }
> 
>  static const struct pwm_ops visconti_pwm_ops = { diff --git



for the Visconti pwd:
Reviewed-by: Nobuhiro Iwamatsu

[PATCH v5 6/7] drm: rcar-du: Add r8a779g0 support

Add support for DU on r8a779g0, which is identical to DU on r8a779a0.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 22 ++
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  2 +-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d003e8d9e7a2..46c60a2d710d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -524,6 +524,27 @@ static const struct rcar_du_device_info 
rcar_du_r8a779a0_info = {
.dsi_clk_mask =  BIT(1) | BIT(0),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a779g0_info = {
+   .gen = 4,
+   .features = RCAR_DU_FEATURE_CRTC_IRQ
+ | RCAR_DU_FEATURE_VSP1_SOURCE
+ | RCAR_DU_FEATURE_NO_BLENDING,
+   .channels_mask = BIT(1) | BIT(0),
+   .routes = {
+   /* R8A779G0 has two MIPI DSI outputs. */
+   [RCAR_DU_OUTPUT_DSI0] = {
+   .possible_crtcs = BIT(0),
+   .port = 0,
+   },
+   [RCAR_DU_OUTPUT_DSI1] = {
+   .possible_crtcs = BIT(1),
+   .port = 1,
+   },
+   },
+   .num_rpf = 5,
+   .dsi_clk_mask =  BIT(1) | BIT(0),
+};
+
 static const struct of_device_id rcar_du_of_table[] = {
{ .compatible = "renesas,du-r8a7742", .data = _du_r8a7790_info },
{ .compatible = "renesas,du-r8a7743", .data = _du_r8a7743_info },
@@ -549,6 +570,7 @@ static const struct of_device_id rcar_du_of_table[] = {
{ .compatible = "renesas,du-r8a77990", .data = _du_r8a7799x_info },
{ .compatible = "renesas,du-r8a77995", .data = _du_r8a7799x_info },
{ .compatible = "renesas,du-r8a779a0", .data = _du_r8a779a0_info },
+   { .compatible = "renesas,du-r8a779g0", .data = _du_r8a779g0_info },
{ }
 };
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c 
b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 1fe8581577ed..6da01760ede5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -107,7 +107,7 @@ static void rcar_du_group_setup_didsr(struct rcar_du_group 
*rgrp)
 */
rcrtc = rcdu->crtcs;
num_crtcs = rcdu->num_crtcs;
-   } else if (rcdu->info->gen == 3 && rgrp->num_crtcs > 1) {
+   } else if (rcdu->info->gen >= 3 && rgrp->num_crtcs > 1) {
/*
 * On Gen3 dot clocks are setup through per-group registers,
 * only available when the group has two channels.
-- 
2.34.1

[PATCH v5 1/7] dt-bindings: display: renesas, du: Provide bindings for r8a779g0

Extend the Renesas DU display bindings to support the r8a779g0 V4H.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Acked-by: Krzysztof Kozlowski 
Reviewed-by: Laurent Pinchart 
---
 Documentation/devicetree/bindings/display/renesas,du.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml 
b/Documentation/devicetree/bindings/display/renesas,du.yaml
index b3e588022082..d4830f52c512 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -40,6 +40,7 @@ properties:
   - renesas,du-r8a77990 # for R-Car E3 compatible DU
   - renesas,du-r8a77995 # for R-Car D3 compatible DU
   - renesas,du-r8a779a0 # for R-Car V3U compatible DU
+  - renesas,du-r8a779g0 # for R-Car V4H compatible DU
 
   reg:
 maxItems: 1
@@ -762,6 +763,7 @@ allOf:
   contains:
 enum:
   - renesas,du-r8a779a0
+  - renesas,du-r8a779g0
 then:
   properties:
 clocks:
-- 
2.34.1

Re: [PATCH v5 1/2] drm/fourcc: Add Synaptics VideoSmart tiled modifiers

2022-12-02 Thread Randy Li




Sent from my iPad

> On Dec 1, 2022, at 5:39 PM, Daniel Vetter  wrote:
> CAUTION: Email originated externally, do not click links or open attachments 
> unless you recognize the sender and know the content is safe.
> 
> 
>> On Thu, Dec 01, 2022 at 12:49:16AM +0800, Randy Li wrote:
>> 
>> 
>> Sent from my iPad
>> 
 On Nov 30, 2022, at 7:30 PM, Daniel Vetter  wrote:
>>> CAUTION: Email originated externally, do not click links or open 
>>> attachments unless you recognize the sender and know the content is safe.
 On Wed, Nov 30, 2022 at 05:21:48PM +0800, Hsia-Jun Li wrote:
 From: "Hsia-Jun(Randy) Li" 
 Those modifiers only record the parameters would effort pixel
 layout or memory layout. Whether physical memory page mapping
 is used is not a part of format.
 Signed-off-by: Hsia-Jun(Randy) Li 
 ---
 include/uapi/drm/drm_fourcc.h | 76 +++
 1 file changed, 76 insertions(+)
 diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
 index bc056f2d537d..e0905f573f43 100644
 --- a/include/uapi/drm/drm_fourcc.h
 +++ b/include/uapi/drm/drm_fourcc.h
 @@ -407,6 +407,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
 #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
 #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
 +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
 /* add more to the end as needed */
 @@ -1507,6 +1508,81 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 
 modifier)
 #define AMD_FMT_MOD_CLEAR(field) \
 (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
 +/*
 + * Synaptics VideoSmart modifiers
 + *
 + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
 + * within a tile. GOT size and layout varies based on platform and
 + * performance concern.
 + *
 + * Besides, an 8 length 4 bytes arrary (32 bytes) would be need to store
 + * some compression parameters for a compression metadata plane.
 + *
 + * Further information can be found in
 + * Documentation/gpu/synaptics.rst
 + *
 + *   Macro
 + * Bits  Param Description
 + *   - 
 -
 + *
 + *  7:0  f Scan direction description.
 + *
 + *   0 = Invalid
 + *   1 = V4, the scan would always start from vertical for 4 
 pixel
 + *   then move back to the start pixel of the next 
 horizontal
 + *   direction.
 + *   2 = Reserved for future use.
 + *
 + * 15:8  m The times of pattern repeat in the right angle direction 
 from
 + * the first scan direction.
 + *
 + * 19:16 p The padding bits after the whole scan, could be zero.
 + *
 + * 20:20 g GOT packing flag.
 + *
 + * 23:21 - Reserved for future use.  Must be zero.
>>> Can you pls fold all the future use reservations into the top end?
>> You see we could put more related flag in each of reserved area.
>> Here is for the group of tiles flag.
>> Bit 35 to 32 could be used for describing the dimension of the group of 
>> tiles.
> 
> Oh also on the dimension thing, this is the tile size and has nothing to
> do with the overall buffer size, right?
I don’t think you could have a insufficient tile, that applies to the group of 
tile.
> Because the overall buffer size is
> meant to be carried in separate metadata (like the drm_framebuffer
> structure or ADDFB2 ioctl data). drm fourcc/modifier assume that height,
> width, offset and stride are specified per plane already (unless the
> auxiary plane has a fixed layout and is not tracked as a separate plane
> for this format).
One thing I noticed here, there is no way to tell the buffer size that user 
should request/allocate from the drm API. It needs to be calculated in the 
userspace unless you would use the custom ioctl.
> 
>>> Also I
>>> think it'd be good to at least reserve maybe the top 8 bits or so for a
>>> synaptics specific format indicator, so that it's easier to extend this in
>>> the future ...
>> I think the  bit 56 to 63 are used for storing the vendor id. That is why I 
>> didn’t include them below. Or you mean the bit 7 to 0?
>> Do yo
> 
> Yeah there's 8 bit vendor id, but you could reserve another 8 bit at the
> top (so 48:55 or something like that) to enumerate within the synaptics
> space. Just to future proof the schema, because experience says that hw
> engineers absolutely do love to change this stuff eventually.
I left the whole 37:55.
> -Daniel
> 
>>> -Daniel
 + *
 + * 27:24 h log2(horizontal) of pixels, in GOTs.
 + *
 + * 31:28 v log2(vertical) of pixels, in GOTs.
 + *
 + * 35:32 - Reserved for future use.  Must be zero.
 + *
 + * 36:36 c Compression flag.
 + *
 + * 55:37 -

Re: [PATCH v2 3/7] clk: renesas: r8a779g0: Add display related clocks


Hi Geert, Laurent,

On 30/11/2022 21:18, Geert Uytterhoeven wrote:

Hi Tomi,

On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
 wrote:

Add clocks related to display which are needed to get the DSI output
working.

Extracted from Renesas BSP tree.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 


Thanks for your patch!


--- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
@@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] 
__initconst = {
 DEF_FIXED("viobusd2",   R8A779G0_CLK_VIOBUSD2,  CLK_VIO,2, 1),
 DEF_FIXED("vcbus",  R8A779G0_CLK_VCBUS, CLK_VC, 1, 1),
 DEF_FIXED("vcbusd2",R8A779G0_CLK_VCBUSD2,   CLK_VC, 2, 1),
+   DEF_FIXED("dsiref", R8A779G0_CLK_DSIREF,CLK_PLL5_DIV4,  48, 1),
+   DEF_DIV6P1("dsiext",R8A779G0_CLK_DSIEXT,CLK_PLL5_DIV4,  0x884),

 DEF_GEN4_SDH("sd0h",R8A779G0_CLK_SD0H,  CLK_SDSRC, 
0x870),
 DEF_GEN4_SD("sd0",  R8A779G0_CLK_SD0,   R8A779G0_CLK_SD0H, 
0x870),
@@ -161,6 +163,14 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] 
__initconst = {
 DEF_MOD("avb0", 211,R8A779G0_CLK_S0D4_HSC),
 DEF_MOD("avb1", 212,R8A779G0_CLK_S0D4_HSC),
 DEF_MOD("avb2", 213,R8A779G0_CLK_S0D4_HSC),
+


Weird horizontal and vertical spacing below...


+   DEF_MOD("dis0", 411,R8A779G0_CLK_S0D3),


I doubt this parent clock is correct.
Based on Table 8.1.4e ("Lists of CPG clocks generated from PLL5"),
this should be one of the VIOBUS clocks.
VIOBUSD2 has the same rate as S0D3, so I'd use that one.


+   DEF_MOD("dsitxlink0",   415,R8A779G0_CLK_DSIREF),
+   DEF_MOD("dsitxlink1",   416,R8A779G0_CLK_DSIREF),


Now that you started questioning about the clocks, I started to wonder 
about the DSI clocks. They don't quite make sense to me, but here also I 
just assumed it's "fine" as I copied it and it works.


The VIOBUS & VIOBUSD2 are marked to as going to the DSI. But we don't 
actually mark any of the DSI clocks as coming from those sources.


DSIREF is quite clear, it's the source for DSI PLL.

DSIEXT goes to the DSI PHY and is also marked to be used for LP-TX.

In the DT we have now:

clocks = < CPG_MOD 415>,
 < CPG_CORE R8A779G0_CLK_DSIEXT>,
 < CPG_CORE R8A779G0_CLK_DSIREF>;
clock-names = "fck", "dsi", "pll";

The "dsi" clock name is a bit vague, but maybe it's "not fclk, not pll, 
but still needed for dsi"? =)


Is it ok to refer to DSIEXT & DSIREF like that, or should they be in the 
r8a779g0_mod_clks list? Or is that list for fclks only?


So the fclk in the dts is mod clock 415 (416 for the second dsi), which 
is dsitxlink0 or dsitxlink1. Well, those names don't quite make sense if 
it's a fclk.


I would rename those clocks to "dsi0" and "dsi1", and source them from 
R8A779G0_CLK_VIOBUSD2, similarly to the other video clocks.


Does the above make sense?

 Tomi

Re: Screen corruption using radeon kernel driver

2022-12-02 Thread Mikhail Krylov

On Thu, Dec 01, 2022 at 02:00:58PM +, Robin Murphy wrote:
> On 2022-11-30 19:59, Mikhail Krylov wrote:
> > On Wed, Nov 30, 2022 at 11:07:32AM -0500, Alex Deucher wrote:
> > > On Wed, Nov 30, 2022 at 10:42 AM Robin Murphy  
> > > wrote:
> > > > 
> > > > On 2022-11-30 14:28, Alex Deucher wrote:
> > > > > On Wed, Nov 30, 2022 at 7:54 AM Robin Murphy  
> > > > > wrote:
> > > > > > 
> > > > > > On 2022-11-29 17:11, Mikhail Krylov wrote:
> > > > > > > On Tue, Nov 29, 2022 at 11:05:28AM -0500, Alex Deucher wrote:
> > > > > > > > On Tue, Nov 29, 2022 at 10:59 AM Mikhail Krylov 
> > > > > > > >  wrote:
> > > > > > > > > 
> > > > > > > > > On Tue, Nov 29, 2022 at 09:44:19AM -0500, Alex Deucher wrote:
> > > > > > > > > > On Mon, Nov 28, 2022 at 3:48 PM Mikhail Krylov 
> > > > > > > > > >  wrote:
> > > > > > > > > > > 
> > > > > > > > > > > On Mon, Nov 28, 2022 at 09:50:50AM -0500, Alex Deucher 
> > > > > > > > > > > wrote:
> > > > > > > > > > > 
> > > > > > > > > > > > > > [excessive quoting removed]
> > > > > > > > > > > 
> > > > > > > > > > > > > So, is there any progress on this issue? I do 
> > > > > > > > > > > > > understand it's not a high
> > > > > > > > > > > > > priority one, and today I've checked it on 6.0 
> > > > > > > > > > > > > kernel, and
> > > > > > > > > > > > > unfortunately, it still persists...
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I'm considering writing a patch that will allow user 
> > > > > > > > > > > > > to override
> > > > > > > > > > > > > need_dma32/dma_bits setting with a module parameter. 
> > > > > > > > > > > > > I'll have some time
> > > > > > > > > > > > > after the New Year for that.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Is it at all possible that such a patch will be 
> > > > > > > > > > > > > merged into kernel?
> > > > > > > > > > > > > 
> > > > > > > > > > > > On Mon, Nov 28, 2022 at 9:31 AM Mikhail Krylov 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > Unless someone familiar with HIMEM can figure out what 
> > > > > > > > > > > > is going wrong
> > > > > > > > > > > > we should just revert the patch.
> > > > > > > > > > > > 
> > > > > > > > > > > > Alex
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Okay, I was suggesting that mostly because
> > > > > > > > > > > 
> > > > > > > > > > > a) it works for me with dma_bits = 40 (I understand 
> > > > > > > > > > > that's what it is
> > > > > > > > > > > without the original patch applied);
> > > > > > > > > > > 
> > > > > > > > > > > b) there's a hint of uncertainity on this line
> > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_device.c#n1359
> > > > > > > > > > > saying that for AGP dma_bits = 32 is the safest option, 
> > > > > > > > > > > so apparently there are
> > > > > > > > > > > setups, unlike mine, where dma_bits = 32 is better than 
> > > > > > > > > > > 40.
> > > > > > > > > > > 
> > > > > > > > > > > But I'm in no position to argue, just wanted to make 
> > > > > > > > > > > myself clear.
> > > > > > > > > > > I'm okay with rebuilding the kernel for my machine until 
> > > > > > > > > > > the original
> > > > > > > > > > > patch is reverted or any other fix is applied.
> > > > > > > > > > 
> > > > > > > > > > What GPU do you have and is it AGP?  If it is AGP, does 
> > > > > > > > > > setting
> > > > > > > > > > radeon.agpmode=-1 also fix it?
> > > > > > > > > > 
> > > > > > > > > > Alex
> > > > > > > > > 
> > > > > > > > > That is ATI Radeon X1950, and, unfortunately, 
> > > > > > > > > radeon.agpmode=-1 doesn't
> > > > > > > > > help, it just makes 3D acceleration in games such as 
> > > > > > > > > OpenArena stop
> > > > > > > > > working.
> > > > > > > > 
> > > > > > > > Just to confirm, is the board AGP or PCIe?
> > > > > > > > 
> > > > > > > > Alex
> > > > > > > 
> > > > > > > It is AGP. That's an old machine.
> > > > > > 
> > > > > > Can you check whether dma_addressing_limited() is actually 
> > > > > > returning the
> > > > > > expected result at the point of radeon_ttm_init()? Disabling 
> > > > > > highmem is
> > > > > > presumably just hiding whatever problem exists, by throwing away all
> > > > > >>32-bit RAM such that use_dma32 doesn't matter.
> > > > > 
> > > > > The device in question only supports a 32 bit DMA mask so
> > > > > dma_addressing_limited() should return true.  Bounce buffers are not
> > > > > really usable on GPUs because they map so much memory.  If
> > > > > dma_addressing_limited() returns false, that would explain it.
> > > > 
> > > > Right, it appears to be the only part of the offending commit that
> > > > *could* reasonably make any difference, so I'm primarily wondering if
> > > > dma_get_required_mask() somehow gets confused.
> > > 
> > > Mikhail,
> > > 
> > > Can you see that dma_addressing_limited() and dma_get_required_mask()
> > > return in this case?
> > > 
> > > Alex
> > > 
> > > 
> > > > 
> > > > Thanks,
> > > > Robin.
> > 
> >

[PATCH v5 7/7] drm: rcar-du: dsi: Add r8A779g0 support

Add DSI support for r8a779g0. The main differences to r8a779a0 are in
the PLL and PHTW setups.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c  | 497 ++-
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h |   6 +-
 2 files changed, 375 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c 
b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index a7f2b7f66a17..e10e4d4b89a2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -28,6 +29,31 @@
 #include "rcar_mipi_dsi.h"
 #include "rcar_mipi_dsi_regs.h"
 
+#define MHZ(v) ((u32)((v) * 100U))
+
+enum rcar_mipi_dsi_hw_model {
+   RCAR_DSI_V3U,
+   RCAR_DSI_V4H,
+};
+
+struct rcar_mipi_dsi_device_info {
+   enum rcar_mipi_dsi_hw_model model;
+
+   const struct dsi_clk_config *clk_cfg;
+
+   u8 clockset2_m_offset;
+
+   u8 n_min;
+   u8 n_max;
+   u8 n_mul;
+   unsigned long fpfd_min;
+   unsigned long fpfd_max;
+   u16 m_min;
+   u16 m_max;
+   unsigned long fout_min;
+   unsigned long fout_max;
+};
+
 struct rcar_mipi_dsi {
struct device *dev;
const struct rcar_mipi_dsi_device_info *info;
@@ -50,6 +76,17 @@ struct rcar_mipi_dsi {
unsigned int lanes;
 };
 
+struct dsi_setup_info {
+   unsigned long hsfreq;
+   u16 hsfreqrange;
+
+   unsigned long fout;
+   u16 m;
+   u16 n;
+   u16 vclk_divider;
+   const struct dsi_clk_config *clkset;
+};
+
 static inline struct rcar_mipi_dsi *
 bridge_to_rcar_mipi_dsi(struct drm_bridge *bridge)
 {
@@ -62,65 +99,78 @@ host_to_rcar_mipi_dsi(struct mipi_dsi_host *host)
return container_of(host, struct rcar_mipi_dsi, host);
 }
 
-static const u32 phtw[] = {
-   0x01020114, 0x01600115, /* General testing */
-   0x01030116, 0x0102011d, /* General testing */
-   0x011101a4, 0x018601a4, /* 1Gbps testing */
-   0x014201a0, 0x010001a3, /* 1Gbps testing */
-   0x0101011f, /* 1Gbps testing */
-};
-
-static const u32 phtw2[] = {
-   0x010c0130, 0x010c0140, /* General testing */
-   0x010c0150, 0x010c0180, /* General testing */
-   0x010c0190,
-   0x010a0160, 0x010a0170,
-   0x01800164, 0x01800174, /* 1Gbps testing */
-};
-
 static const u32 hsfreqrange_table[][2] = {
-   { 8000U,   0x00 }, { 9000U,   0x10 }, { 1U,  0x20 },
-   { 11000U,  0x30 }, { 12000U,  0x01 }, { 13000U,  0x11 },
-   { 14000U,  0x21 }, { 15000U,  0x31 }, { 16000U,  0x02 },
-   { 17000U,  0x12 }, { 18000U,  0x22 }, { 19000U,  0x32 },
-   { 20500U,  0x03 }, { 22000U,  0x13 }, { 23500U,  0x23 },
-   { 25000U,  0x33 }, { 27500U,  0x04 }, { 3U,  0x14 },
-   { 32500U,  0x25 }, { 35000U,  0x35 }, { 4U,  0x05 },
-   { 45000U,  0x16 }, { 5U,  0x26 }, { 55000U,  0x37 },
-   { 6U,  0x07 }, { 65000U,  0x18 }, { 7U,  0x28 },
-   { 75000U,  0x39 }, { 8U,  0x09 }, { 85000U,  0x19 },
-   { 9U,  0x29 }, { 95000U,  0x3a }, { 10U, 0x0a },
-   { 105000U, 0x1a }, { 11U, 0x2a }, { 115000U, 0x3b },
-   { 12U, 0x0b }, { 125000U, 0x1b }, { 13U, 0x2b },
-   { 135000U, 0x3c }, { 14U, 0x0c }, { 145000U, 0x1c },
-   { 15U, 0x2c }, { 155000U, 0x3d }, { 16U, 0x0d },
-   { 165000U, 0x1d }, { 17U, 0x2e }, { 175000U, 0x3e },
-   { 18U, 0x0e }, { 185000U, 0x1e }, { 19U, 0x2f },
-   { 195000U, 0x3f }, { 20U, 0x0f }, { 205000U, 0x40 },
-   { 21U, 0x41 }, { 215000U, 0x42 }, { 22U, 0x43 },
-   { 225000U, 0x44 }, { 23U, 0x45 }, { 235000U, 0x46 },
-   { 24U, 0x47 }, { 245000U, 0x48 }, { 25U, 0x49 },
+   {   MHZ(80), 0x00 }, {   MHZ(90), 0x10 }, {  MHZ(100), 0x20 },
+   {  MHZ(110), 0x30 }, {  MHZ(120), 0x01 }, {  MHZ(130), 0x11 },
+   {  MHZ(140), 0x21 }, {  MHZ(150), 0x31 }, {  MHZ(160), 0x02 },
+   {  MHZ(170), 0x12 }, {  MHZ(180), 0x22 }, {  MHZ(190), 0x32 },
+   {  MHZ(205), 0x03 }, {  MHZ(220), 0x13 }, {  MHZ(235), 0x23 },
+   {  MHZ(250), 0x33 }, {  MHZ(275), 0x04 }, {  MHZ(300), 0x14 },
+   {  MHZ(325), 0x25 }, {  MHZ(350), 0x35 }, {  MHZ(400), 0x05 },
+   {  MHZ(450), 0x16 }, {  MHZ(500), 0x26 }, {  MHZ(550), 0x37 },
+   {  MHZ(600), 0x07 }, {  MHZ(650), 0x18 }, {  MHZ(700), 0x28 },
+   {  MHZ(750), 0x39 }, {  MHZ(800), 0x09 }, {  MHZ(850), 0x19 },
+   {  MHZ(900), 0x29 }, {  MHZ(950), 0x3a }, { MHZ(1000), 0x0a },
+   { MHZ(1050), 0x1a }, { MHZ(1100), 0x2a }, { MHZ(1150), 0x3b },
+   { MHZ(1200), 0x0b }, { MHZ(1250), 0x1b },

[PATCH v5 0/7] Renesas V4H DSI & DP output support

From: Tomi Valkeinen 

Hi,

These add support for DSI on V4H SoC (r8a779g0) and DP for Whitehawk
board.

I'm sending the full series for clarity.

Changes to v4:

- Changes in "clk: renesas: r8a779g0: Add display related clocks":
* Fix formatting
* Use R8A779G0_CLK_VIOBUSD2 as the source for all video related
  clocks.

 Tomi

Tomi Valkeinen (7):
  dt-bindings: display: renesas,du: Provide bindings for r8a779g0
  dt-bindings: display: bridge: renesas,dsi-csi2-tx: Add r8a779g0
  clk: renesas: r8a779g0: Add display related clocks
  arm64: dts: renesas: r8a779g0: Add display related nodes
  arm64: dts: renesas: white-hawk-cpu: Add DP output support
  drm: rcar-du: Add r8a779g0 support
  drm: rcar-du: dsi: Add r8A779g0 support

 .../display/bridge/renesas,dsi-csi2-tx.yaml   |   3 +-
 .../bindings/display/renesas,du.yaml  |   2 +
 .../dts/renesas/r8a779g0-white-hawk-cpu.dtsi  |  94 
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 130 +
 drivers/clk/renesas/r8a779g0-cpg-mssr.c   |   9 +
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |  22 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c   |   2 +-
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c   | 497 +-
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi_regs.h  |   6 +-
 9 files changed, 635 insertions(+), 130 deletions(-)

-- 
2.34.1

Re: [PATCH v2 3/7] clk: renesas: r8a779g0: Add display related clocks


Hi,

On 30/11/2022 21:18, Geert Uytterhoeven wrote:

Hi Tomi,

On Wed, Nov 23, 2022 at 8:00 AM Tomi Valkeinen
 wrote:

Add clocks related to display which are needed to get the DSI output
working.

Extracted from Renesas BSP tree.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 


Thanks for your patch!


--- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
@@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] 
__initconst = {
 DEF_FIXED("viobusd2",   R8A779G0_CLK_VIOBUSD2,  CLK_VIO,2, 1),
 DEF_FIXED("vcbus",  R8A779G0_CLK_VCBUS, CLK_VC, 1, 1),
 DEF_FIXED("vcbusd2",R8A779G0_CLK_VCBUSD2,   CLK_VC, 2, 1),
+   DEF_FIXED("dsiref", R8A779G0_CLK_DSIREF,CLK_PLL5_DIV4,  48, 1),
+   DEF_DIV6P1("dsiext",R8A779G0_CLK_DSIEXT,CLK_PLL5_DIV4,  0x884),

 DEF_GEN4_SDH("sd0h",R8A779G0_CLK_SD0H,  CLK_SDSRC, 
0x870),
 DEF_GEN4_SD("sd0",  R8A779G0_CLK_SD0,   R8A779G0_CLK_SD0H, 
0x870),
@@ -161,6 +163,14 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] 
__initconst = {
 DEF_MOD("avb0", 211,R8A779G0_CLK_S0D4_HSC),
 DEF_MOD("avb1", 212,R8A779G0_CLK_S0D4_HSC),
 DEF_MOD("avb2", 213,R8A779G0_CLK_S0D4_HSC),
+


Weird horizontal and vertical spacing below...


Yep. I added those to keep the lines more visible for me while working 
on this, but forgot to remove.





+   DEF_MOD("dis0", 411,R8A779G0_CLK_S0D3),


I doubt this parent clock is correct.
Based on Table 8.1.4e ("Lists of CPG clocks generated from PLL5"),
this should be one of the VIOBUS clocks.
VIOBUSD2 has the same rate as S0D3, so I'd use that one.


I'm pretty clueless about Renesas clocks, and I can't find a nice 
clock-tree picture from the docs, but looking at the table, what you say 
makes sense.


Both VIOBUS and VIOBUSD2 are marked to go to the video IPs, but with a 
bit of browsing, I can't find any more info about the clocking. Afaik, 
we don't care about the dis0 rate in the driver, so... Basically any 
clock will work here =). I'll pick VIOBUSD2 as you suggest (why would 
there be a /2 clock if it's not used...).



+   DEF_MOD("dsitxlink0",   415,R8A779G0_CLK_DSIREF),
+   DEF_MOD("dsitxlink1",   416,R8A779G0_CLK_DSIREF),
+
+   DEF_MOD("fcpvd0",   508,R8A779G0_CLK_S0D3),
+   DEF_MOD("fcpvd1",   509,R8A779G0_CLK_S0D3),


Likewise.


Ack.


+
 DEF_MOD("hscif0",   514,R8A779G0_CLK_SASYNCPERD1),
 DEF_MOD("hscif1",   515,R8A779G0_CLK_SASYNCPERD1),
 DEF_MOD("hscif2",   516,R8A779G0_CLK_SASYNCPERD1),
@@ -193,6 +203,10 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] 
__initconst = {
 DEF_MOD("tmu3", 716,R8A779G0_CLK_SASYNCPERD2),
 DEF_MOD("tmu4", 717,R8A779G0_CLK_SASYNCPERD2),
 DEF_MOD("tpu0", 718,R8A779G0_CLK_SASYNCPERD4),
+
+   DEF_MOD("vspd0",830,R8A779G0_CLK_S0D1_VIO),
+   DEF_MOD("vspd1",831,R8A779G0_CLK_S0D1_VIO),


While S0D1_VIO is a VIO clock, it is clocked from PLL1, which supports
spread-spectrum, unlike PLL5.
Again, based on Table 8.1.4e ("Lists of CPG clocks generated from
PLL5"), this should be one of the VIOBUS clocks.


Yep.


Not that all of this matters a lot: all of these parents are always-on,
and I think "dis0" is the only clock where we care about the actual
clock rate?
No, of the clocks added above, in the drivers we only care about the 
dsiref rate. That's used for the DSI PLL, and that PLL is used as the 
DU's pclk.


 Tomi

[PATCH v5 4/7] arm64: dts: renesas: r8a779g0: Add display related nodes

Add DT nodes for components needed to get the DSI output working:
- FCPv
- VSPd
- DU
- DSI

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
Reviewed-by: Geert Uytterhoeven 
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 130 ++
 1 file changed, 130 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi 
b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 45d8d927ad26..4577208963b3 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -1203,6 +1203,136 @@ gic: interrupt-controller@f100 {
  (GIC_CPU_MASK_SIMPLE(4) | 
IRQ_TYPE_LEVEL_HIGH)>;
};
 
+   fcpvd0: fcp@fea1 {
+   compatible = "renesas,fcpv";
+   reg = <0 0xfea1 0 0x200>;
+   clocks = < CPG_MOD 508>;
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   resets = < 508>;
+   };
+
+   fcpvd1: fcp@fea11000 {
+   compatible = "renesas,fcpv";
+   reg = <0 0xfea11000 0 0x200>;
+   clocks = < CPG_MOD 509>;
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   resets = < 509>;
+   };
+
+   vspd0: vsp@fea2 {
+   compatible = "renesas,vsp2";
+   reg = <0 0xfea2 0 0x7000>;
+   interrupts = ;
+   clocks = < CPG_MOD 830>;
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   resets = < 830>;
+
+   renesas,fcp = <>;
+   };
+
+   vspd1: vsp@fea28000 {
+   compatible = "renesas,vsp2";
+   reg = <0 0xfea28000 0 0x7000>;
+   interrupts = ;
+   clocks = < CPG_MOD 831>;
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   resets = < 831>;
+
+   renesas,fcp = <>;
+   };
+
+   du: display@feb0 {
+   compatible = "renesas,du-r8a779g0";
+   reg = <0 0xfeb0 0 0x4>;
+   interrupts = ,
+;
+   clocks = < CPG_MOD 411>;
+   clock-names = "du.0";
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   resets = < 411>;
+   reset-names = "du.0";
+   renesas,vsps = < 0>, < 0>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   du_out_dsi0: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   du_out_dsi1: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   dsi0: dsi-encoder@fed8 {
+   compatible = "renesas,r8a779g0-dsi-csi2-tx";
+   reg = <0 0xfed8 0 0x1>;
+   power-domains = < R8A779G0_PD_ALWAYS_ON>;
+   clocks = < CPG_MOD 415>,
+< CPG_CORE R8A779G0_CLK_DSIEXT>,
+< CPG_CORE R8A779G0_CLK_DSIREF>;
+   clock-names = "fck", "dsi", "pll";
+   resets = < 415>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   dsi0_in: endpoint {
+   remote-endpoint = 
<_out_dsi0>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   };
+   };
+   };
+
+   dsi1: dsi-encoder@fed9 {
+   compatible = "renesas,r8a779g0-dsi-csi2-tx";
+   reg = <0 0xfed9 0 0x1>;
+   power-domains = <

[PATCH v5 3/7] clk: renesas: r8a779g0: Add display related clocks

Add clocks related to display which are needed to get the DSI output
working.

Extracted from Renesas BSP tree.

Signed-off-by: Tomi Valkeinen 
Reviewed-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 drivers/clk/renesas/r8a779g0-cpg-mssr.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/clk/renesas/r8a779g0-cpg-mssr.c 
b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
index c6337a408e5e..d898ca391e6f 100644
--- a/drivers/clk/renesas/r8a779g0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779g0-cpg-mssr.c
@@ -145,6 +145,8 @@ static const struct cpg_core_clk r8a779g0_core_clks[] 
__initconst = {
DEF_FIXED("viobusd2",   R8A779G0_CLK_VIOBUSD2,  CLK_VIO,2, 1),
DEF_FIXED("vcbus",  R8A779G0_CLK_VCBUS, CLK_VC, 1, 1),
DEF_FIXED("vcbusd2",R8A779G0_CLK_VCBUSD2,   CLK_VC, 2, 1),
+   DEF_FIXED("dsiref", R8A779G0_CLK_DSIREF,CLK_PLL5_DIV4,  48, 1),
+   DEF_DIV6P1("dsiext",R8A779G0_CLK_DSIEXT,CLK_PLL5_DIV4,  0x884),
 
DEF_GEN4_SDH("sd0h",R8A779G0_CLK_SD0H,  CLK_SDSRC, 
0x870),
DEF_GEN4_SD("sd0",  R8A779G0_CLK_SD0,   R8A779G0_CLK_SD0H, 
0x870),
@@ -161,6 +163,11 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] 
__initconst = {
DEF_MOD("avb0", 211,R8A779G0_CLK_S0D4_HSC),
DEF_MOD("avb1", 212,R8A779G0_CLK_S0D4_HSC),
DEF_MOD("avb2", 213,R8A779G0_CLK_S0D4_HSC),
+   DEF_MOD("dis0", 411,R8A779G0_CLK_VIOBUSD2),
+   DEF_MOD("dsitxlink0",   415,R8A779G0_CLK_VIOBUSD2),
+   DEF_MOD("dsitxlink1",   416,R8A779G0_CLK_VIOBUSD2),
+   DEF_MOD("fcpvd0",   508,R8A779G0_CLK_VIOBUSD2),
+   DEF_MOD("fcpvd1",   509,R8A779G0_CLK_VIOBUSD2),
DEF_MOD("hscif0",   514,R8A779G0_CLK_SASYNCPERD1),
DEF_MOD("hscif1",   515,R8A779G0_CLK_SASYNCPERD1),
DEF_MOD("hscif2",   516,R8A779G0_CLK_SASYNCPERD1),
@@ -193,6 +200,8 @@ static const struct mssr_mod_clk r8a779g0_mod_clks[] 
__initconst = {
DEF_MOD("tmu3", 716,R8A779G0_CLK_SASYNCPERD2),
DEF_MOD("tmu4", 717,R8A779G0_CLK_SASYNCPERD2),
DEF_MOD("tpu0", 718,R8A779G0_CLK_SASYNCPERD4),
+   DEF_MOD("vspd0",830,R8A779G0_CLK_VIOBUSD2),
+   DEF_MOD("vspd1",831,R8A779G0_CLK_VIOBUSD2),
DEF_MOD("wdt1:wdt0",907,R8A779G0_CLK_R),
DEF_MOD("cmt0", 910,R8A779G0_CLK_R),
DEF_MOD("cmt1", 911,R8A779G0_CLK_R),
-- 
2.34.1

Re: [PATCH] [RFC] drm/etnaviv: Disable softpin

2022-12-02 Thread Marek Vasut


On 12/2/22 09:59, Lucas Stach wrote:

Am Freitag, dem 02.12.2022 um 00:21 +0100 schrieb Marek Vasut:

Currently softpin suffers from assorted race conditions exposed by newer
versions of mesa 22.2.y and 22.3.y . Those races are difficult to fix in
older kernel versions due to massive amount of backports necessary to do
so. Disable softpin by default until Linux 6.1.y is out, which contains
the necessary fixes to make softpin work reliably.


Sorry, but that's a NACK. The userspace driver depends on softpin for
GPUs with texture descriptors, so this introduces a hard functional
regression for those GPUs. I.e. they would go from "require race fixes
that are already on the way to upstream" to not working at all.


I expected that NAK.

But then, what options do we have here, except for a massive convoluted 
backport, which might bring bugs of its own ?

Re: [Intel-gfx] [PATCH v6 3/5] drm/i915: Introduce guard pages to i915_vma

2022-12-02 Thread Tvrtko Ursulin




On 01/12/2022 20:39, Andi Shyti wrote:

From: Chris Wilson 

Introduce the concept of padding the i915_vma with guard pages before
and after. The major consequence is that all ordinary uses of i915_vma
must use i915_vma_offset/i915_vma_size and not i915_vma.node.start/size
directly, as the drm_mm_node will include the guard pages that surround
our object.

The biggest connundrum is how exactly to mix requesting a fixed address
with guard pages, particularly through the existing uABI. The user does
not know about guard pages, so such must be transparent to the user, and
so the execobj.offset must be that of the object itself excluding the
guard. So a PIN_OFFSET_FIXED must then be exclusive of the guard pages.
The caveat is that some placements will be impossible with guard pages,
as wrap arounds need to be avoided, and the vma itself will require a
larger node. We must not report EINVAL but ENOSPC as these are unavailable
locations within the GTT rather than conflicting user requirements.

In the next patch, we start using guard pages for scanout objects. While
these are limited to GGTT vma, on a few platforms these vma (or at least
an alias of the vma) is shared with userspace, so we may leak the
existence of such guards if we are not careful to ensure that the
execobj.offset is transparent and excludes the guards. (On such platforms
like ivb, without full-ppgtt, userspace has to use relocations so the
presence of more untouchable regions within its GTT such be of no further
issue.)

Signed-off-by: Chris Wilson 
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Tvrtko Ursulin 
Signed-off-by: Andi Shyti 
---
Hi Tvrtko,

I removed your r-b in this version because I restored the original value
of the guard being aligned with the vma size alignment. Turns out that
CI failed with the latest version because the guard was becoming too big
(we would have hit the GEM_BUG_ON)[*].

The reason why now the guard is aligned with the vma alignment is that
the area is already aligned and if we use as a starting address start +
guard, guard needs to be aligned, otherwise we screw up all the memory
alignment.

Let me know if it makes sense to you.


Reviewed-by: Tvrtko Ursulin 

Conditional to promise of a prioritised follow up improvement, if it 
turns out GGTT wastage due a bit over zealous guard size comes to bite.


Regards,

Tvrtko



Thanks,
Andi

Changelog
=
v5 -> v6:
  - restore the original alignment of guard so that it's aligned
coherently with the vma area's alignment.
v4 -> v5:
  - remove again the GEM_BUG_ON()
  - fix an oversight where the rounding was called without assigning the
value to the guard.
v1 -> v4:
  - refer to the cover letter.

[*] 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110720v5/fi-rkl-11600/igt@i915_module_l...@load.html#dmesg-warnings300

  drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 +---
  drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +-
  drivers/gpu/drm/i915/i915_vma.c  | 43 
  drivers/gpu/drm/i915/i915_vma.h  |  5 +--
  drivers/gpu/drm/i915/i915_vma_resource.c |  4 +--
  drivers/gpu/drm/i915/i915_vma_resource.h |  7 +++-
  drivers/gpu/drm/i915/i915_vma_types.h|  1 +
  7 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 7644738b9cdbe..784d4a8c43ba9 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -296,8 +296,11 @@ static void gen8_ggtt_insert_entries(struct 
i915_address_space *vm,
 */
  
  	gte = (gen8_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
+   end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;
+   while (gte < end)
+   gen8_set_pte(gte++, vm->scratch[0]->encode);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
  
  	for_each_sgt_daddr(addr, iter, vma_res->bi.pages)

gen8_set_pte(gte++, pte_encode | addr);
@@ -347,9 +350,12 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
dma_addr_t addr;
  
  	gte = (gen6_pte_t __iomem *)ggtt->gsm;

-   gte += vma_res->start / I915_GTT_PAGE_SIZE;
-   end = gte + vma_res->node_size / I915_GTT_PAGE_SIZE;
+   gte += (vma_res->start - vma_res->guard) / I915_GTT_PAGE_SIZE;
  
+	end = gte + vma_res->guard / I915_GTT_PAGE_SIZE;

+   while (gte < end)
+   iowrite32(vm->scratch[0]->encode, gte++);
+   end += (vma_res->node_size + vma_res->guard) / I915_GTT_PAGE_SIZE;
for_each_sgt_daddr(addr, iter, vma_res->bi.pages)
iowrite32(vm->pte_encode(addr, level, flags), gte++);
GEM_BUG_ON(gte > end);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8c2f57eb5ddaa..2434197830523

Re: [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray

2022-12-02 Thread Philipp Zabel

On Thu, Dec 01, 2022 at 06:48:46PM +0100, Lucas Stach wrote:
> This simplifies the driver code a bit, as XArray already provides
> internal locking. IDRs are implemented using XArrays anyways, so
> this drops one level of unneeded abstraction.
> 
> Signed-off-by: Lucas Stach 

Reviewed-by: Philipp Zabel 

regards
Philipp

Re: [PATCH 1/2] drm/etnaviv: split fence lock

2022-12-02 Thread Philipp Zabel

On Thu, Dec 01, 2022 at 06:48:45PM +0100, Lucas Stach wrote:
> The fence lock currently protects two distinct things. It protects the fence
> IDR from concurrent inserts and removes and also keeps drm_sched_job_arm and
> drm_sched_entity_push_job in one atomic section to guarantee the fence seqno
> monotonicity. Split the lock into those two functions.
> 
> Signed-off-by: Lucas Stach 

Reviewed-by: Philipp Zabel 

regards
Philipp

Re: [PATCH v3 2/2] drm/etnaviv: print MMU exception cause

2022-12-02 Thread Philipp Zabel

On Fri, Dec 02, 2022 at 10:19:29AM +0100, Lucas Stach wrote:
> From: Christian Gmeiner 
> 
> The MMU tells us the fault status. While the raw register value is
> already printed, it's a bit more user friendly to translate the
> fault reasons into human readable format.
> 
> Signed-off-by: Christian Gmeiner 
> Signed-off-by: Lucas Stach 

Reviewed-by: Philipp Zabel 

regards
Philipp

[PATCH v3 2/2] drm/etnaviv: print MMU exception cause

From: Christian Gmeiner 

The MMU tells us the fault status. While the raw register value is
already printed, it's a bit more user friendly to translate the
fault reasons into human readable format.

Signed-off-by: Christian Gmeiner 
Signed-off-by: Lucas Stach 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..f79203b774d9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work)
 
 static void dump_mmu_fault(struct etnaviv_gpu *gpu)
 {
+   static const char *fault_reasons[] = {
+   "slave not present",
+   "page not present",
+   "write violation",
+   "out of bounds",
+   "read security violation",
+   "write security violation",
+   };
+
u32 status_reg, status;
int i;
 
@@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status);
 
for (i = 0; i < 4; i++) {
+   const char *reason = "unknown";
u32 address_reg;
+   u32 mmu_status;
 
-   if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4
+   mmu_status = (status >> (i * 4)) & 
VIVS_MMUv2_STATUS_EXCEPTION0__MASK;
+   if (!mmu_status)
continue;
 
+   if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons))
+   reason = fault_reasons[mmu_status - 1];
+
if (gpu->sec_mode == ETNA_SEC_NONE)
address_reg = VIVS_MMUv2_EXCEPTION_ADDR(i);
else
address_reg = VIVS_MMUv2_SEC_EXCEPTION_ADDR;
 
-   dev_err_ratelimited(gpu->dev, "MMU %d fault addr 0x%08x\n", i,
-   gpu_read(gpu, address_reg));
+   dev_err_ratelimited(gpu->dev,
+   "MMU %d fault (%s) addr 0x%08x\n",
+   i, reason, gpu_read(gpu, address_reg));
}
 }
 
-- 
2.30.2

[PATCH v3 1/2] drm/etnaviv: update hardware headers from rnndb

Update the state HI header from the rnndb commit
640a009e7e66 ("rnndb: fix AXI1_TOTAL_REQUEST_COUNT").

Signed-off-by: Lucas Stach 
---
 drivers/gpu/drm/etnaviv/state_hi.xml.h | 86 +-
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/state_hi.xml.h 
b/drivers/gpu/drm/etnaviv/state_hi.xml.h
index deaaa99fa654..94d5f33b1fd6 100644
--- a/drivers/gpu/drm/etnaviv/state_hi.xml.h
+++ b/drivers/gpu/drm/etnaviv/state_hi.xml.h
@@ -8,17 +8,17 @@ This file was generated by the rules-ng-ng headergen tool in 
this git repository
 git clone git://0x04.net/rules-ng-ng
 
 The rules-ng-ng source files this header was generated from are:
-- state.xml (  2 bytes, from 2019-12-20 21:20:35)
-- common.xml(  35468 bytes, from 2018-02-10 13:09:26)
-- common_3d.xml (  15058 bytes, from 2019-12-28 20:02:03)
-- state_hi.xml  (  30552 bytes, from 2019-12-28 20:02:48)
-- copyright.xml (   1597 bytes, from 2018-02-10 13:09:26)
-- state_2d.xml  (  51552 bytes, from 2018-02-10 13:09:26)
-- state_3d.xml  (  83098 bytes, from 2019-12-28 20:02:03)
-- state_blt.xml (  14252 bytes, from 2019-10-20 19:59:15)
-- state_vg.xml  (   5975 bytes, from 2018-02-10 13:09:26)
-
-Copyright (C) 2012-2019 by the following authors:
+- state.xml (  27198 bytes, from 2022-04-22 10:35:24)
+- common.xml(  35468 bytes, from 2020-10-28 12:56:03)
+- common_3d.xml (  15058 bytes, from 2020-10-28 12:56:03)
+- state_hi.xml  (  34804 bytes, from 2022-12-02 09:06:28)
+- copyright.xml (   1597 bytes, from 2020-10-28 12:56:03)
+- state_2d.xml  (  51552 bytes, from 2020-10-28 12:56:03)
+- state_3d.xml  (  84445 bytes, from 2022-11-15 15:59:38)
+- state_blt.xml (  14424 bytes, from 2022-11-07 11:18:41)
+- state_vg.xml  (   5975 bytes, from 2020-10-28 12:56:03)
+
+Copyright (C) 2012-2022 by the following authors:
 - Wladimir J. van der Laan 
 - Christian Gmeiner 
 - Lucas Stach 
@@ -321,16 +321,16 @@ DEALINGS IN THE SOFTWARE.
 #define VIVS_MMUv2_CONFIGURATION_ADDRESS(x)(((x) << 
VIVS_MMUv2_CONFIGURATION_ADDRESS__SHIFT) & 
VIVS_MMUv2_CONFIGURATION_ADDRESS__MASK)
 
 #define VIVS_MMUv2_STATUS  0x0188
-#define VIVS_MMUv2_STATUS_EXCEPTION0__MASK 0x0003
+#define VIVS_MMUv2_STATUS_EXCEPTION0__MASK 0x000f
 #define VIVS_MMUv2_STATUS_EXCEPTION0__SHIFT0
 #define VIVS_MMUv2_STATUS_EXCEPTION0(x)(((x) 
<< VIVS_MMUv2_STATUS_EXCEPTION0__SHIFT) & VIVS_MMUv2_STATUS_EXCEPTION0__MASK)
-#define VIVS_MMUv2_STATUS_EXCEPTION1__MASK 0x0030
+#define VIVS_MMUv2_STATUS_EXCEPTION1__MASK 0x00f0
 #define VIVS_MMUv2_STATUS_EXCEPTION1__SHIFT4
 #define VIVS_MMUv2_STATUS_EXCEPTION1(x)(((x) 
<< VIVS_MMUv2_STATUS_EXCEPTION1__SHIFT) & VIVS_MMUv2_STATUS_EXCEPTION1__MASK)
-#define VIVS_MMUv2_STATUS_EXCEPTION2__MASK 0x0300
+#define VIVS_MMUv2_STATUS_EXCEPTION2__MASK 0x0f00
 #define VIVS_MMUv2_STATUS_EXCEPTION2__SHIFT8
 #define VIVS_MMUv2_STATUS_EXCEPTION2(x)(((x) 
<< VIVS_MMUv2_STATUS_EXCEPTION2__SHIFT) & VIVS_MMUv2_STATUS_EXCEPTION2__MASK)
-#define VIVS_MMUv2_STATUS_EXCEPTION3__MASK 0x3000
+#define VIVS_MMUv2_STATUS_EXCEPTION3__MASK 0xf000
 #define VIVS_MMUv2_STATUS_EXCEPTION3__SHIFT12
 #define VIVS_MMUv2_STATUS_EXCEPTION3(x)(((x) 
<< VIVS_MMUv2_STATUS_EXCEPTION3__SHIFT) & VIVS_MMUv2_STATUS_EXCEPTION3__MASK)
 
@@ -465,7 +465,13 @@ DEALINGS IN THE SOFTWARE.
 #define VIVS_MC_PROFILE_CONFIG0
0x0470
 #define VIVS_MC_PROFILE_CONFIG0_FE__MASK   0x00ff
 #define VIVS_MC_PROFILE_CONFIG0_FE__SHIFT  0
+#define VIVS_MC_PROFILE_CONFIG0_FE_DRAW_COUNT  0x000a
+#define VIVS_MC_PROFILE_CONFIG0_FE_OUT_VERTEX_COUNT0x000b
+#define VIVS_MC_PROFILE_CONFIG0_FE_CACHE_MISS_COUNT0x000c
 #define VIVS_MC_PROFILE_CONFIG0_FE_RESET   0x000f
+#define VIVS_MC_PROFILE_CONFIG0_FE_CACHE_LK_COUNT  0x0010
+#define VIVS_MC_PROFILE_CONFIG0_FE_STALL_COUNT 0x0011
+#define VIVS_MC_PROFILE_CONFIG0_FE_PROCESS_COUNT   0x0012
 #define VIVS_MC_PROFILE_CONFIG0_DE__MASK   0xff00
 #define VIVS_MC_PROFILE_CONFIG0_DE__SHIFT  8
 #define VIVS_MC_PROFILE_CONFIG0_DE_RESET   0x0f00
@@ -499,11 +505,14 @@ DEALINGS IN THE SOFTWARE.
 #define VIVS_MC_PROFILE_CONFIG1_PA_DEPTH_CLIPPED_COUNTER   0x0006
 #define VIVS_MC_PROFILE_CONFIG1_PA_TRIVIAL_REJECTED_COUNTER0x0007
 #define VIVS_MC_PROFILE_CONFIG1_PA_CULLED_COUNTER  0x0008
+#define

Re: [Intel-gfx] [PATCH 0/2] drm/i915: Remove frontbuffer tracking from gem.

2022-12-02 Thread Tvrtko Ursulin




On 01/12/2022 22:03, Zanoni, Paulo R wrote:

Hi

I was given a link to https://patchwork.freedesktop.org/series/111494/
but can't seem to find it on the mailing list, so I'll reply here.

On Thu, 2022-08-25 at 08:46 +0200, Maarten Lankhorst wrote:

Frontbuffer tracking in gem is used in old drivers, but nowadays everyone
calls dirtyfb explicitly. Remove frontbuffer tracking from gem, and
isolate it to display only.


Ok, but then how can the Kernel know if the user space running is an
"old driver" or a new one? Assuming everybody is following the new
policy is at least risky.

(crazy idea: have an ioctl for user space to tell whether it knows how
to DirtyFB and another where it can even say it will never be touching
frontbuffers at all)

Also, when I wrote igt/kms_frontbuffer_tracking, it was agreed that
whatever was there was a representation of the "rules" of the
frontbfuffer writing contract/API. And I see on the link above that the
basic tests of kms_frontbuffer_tracking fail. If
kms_frontbuffer_tracking requires change due to i915.ko having changed
the frontbuffer writing rules, then we should do it, and possibly have
those rules written somewhere.

But then, having the Kernel change expectations like that is not
exactly what I learned we should be doing, because it's the recipe to
break user space.

I'm confused, please clarify us a little more. More sentences in the
commit messages would be absolutely great.


Right, +1 - we need to be sure there aren't some binaries, running in a 
distro somewhere, or whatever, which rely on this and then we are not 
allowed to break them. As minimum at least we need an audit and versions 
to know how old libraries/programs we are talking about here ie. when 
did everyone stop relying on implicit tracking.


Regards,

Tvrtko

Re: [RFC 3/3] drm: Update file owner during use

2022-12-02 Thread Christian König


Am 01.12.22 um 12:09 schrieb Tvrtko Ursulin:


On 30/11/2022 14:18, Daniel Vetter wrote:

On Wed, Nov 30, 2022 at 01:34:07PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

With the typical model where the display server opends the file 
descriptor

and then hands it over to the client we were showing stale data in
debugfs.

Fix it by updating the drm_file->pid on ioctl access from a different
process.

The field is also made RCU protected to allow for lockless readers. 
Update

side is protected with dev->filelist_mutex.

Signed-off-by: Tvrtko Ursulin 
Cc: "Christian König" 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  6 +++--
  drivers/gpu/drm/drm_auth.c  |  3 ++-
  drivers/gpu/drm/drm_debugfs.c   | 10 
  drivers/gpu/drm/drm_file.c  | 32 
-

  drivers/gpu/drm/drm_ioctl.c |  3 +++
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  5 +++-
  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c |  6 +++--
  include/drm/drm_file.h  | 13 --
  8 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 30e24da1f398..385deb044058 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -959,6 +959,7 @@ static int amdgpu_debugfs_gem_info_show(struct 
seq_file *m, void *unused)

  list_for_each_entry(file, >filelist, lhead) {
  struct task_struct *task;
  struct drm_gem_object *gobj;
+    struct pid *pid;
  int id;
    /*
@@ -968,8 +969,9 @@ static int amdgpu_debugfs_gem_info_show(struct 
seq_file *m, void *unused)
   * Therefore, we need to protect this ->comm access using 
RCU.

   */
  rcu_read_lock();
-    task = pid_task(file->pid, PIDTYPE_TGID);
-    seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
+    pid = rcu_dereference(file->pid);
+    task = pid_task(pid, PIDTYPE_TGID);
+    seq_printf(m, "pid %8d command %s:\n", pid_nr(pid),
 task ? task->comm : "");
  rcu_read_unlock();
  diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index cf92a9ae8034..2ed2585ded37 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device 
*dev, struct drm_file *fpriv)

  static int
  drm_master_check_perm(struct drm_device *dev, struct drm_file 
*file_priv)

  {
-    if (file_priv->pid == task_pid(current) && file_priv->was_master)
+    if (file_priv->was_master &&
+    rcu_access_pointer(file_priv->pid) == task_pid(current))


This scares me, and also makes me wonder whether we really want to
conflate the original owner with the rendering owner. And also, 
whether we
really want to keep updating that, because for some of the "bind an 
fd to

a pid" use-cases like svm we really do not want to ever again allow a
change.

So sligthly different idea:
- we have a separate render pid/drm_file owner frome the open() owner 
that

   we track in drm_auth.c
- that one is set the first time a driver specific ioctl is called 
(which

   for the "pass me the fd" dri3 mode should never be the compositor)
- we start out with nothing and only set it once, which further 
simplifies

   the model (still need the mutex for concurrent first ioctl ofc)


Simpler solution sounds plausible and mostly works for me. Certainly 
is attractive to simplify things. And as the disclaimer I put in the 
cover letter - I wasn't really sure at all if passing a master fd is a 
thing or not. Happy to implement your version if that will be the 
decision.


The only downside I can think of right now with having two owners is 
if someone is "naughty" and actually uses the fd for rendering from 
two sides. That wouldn't conceptually work for what I am doing in the 
cgroup controller, where I need to attribute GPU usage to a process, 
which is a lookup from struct pid -> list of drm_files -> etc. So in 
the two owners scheme I would just need to ignore the "open owner" and 
rely that "render ownder" truly is the only one doing the rendering. 
Or maybe I'd need to add support for multiple owners as well.. would 
be a bit annoying probably.


Hm now that I think about more.. the one shot nature of this scheme 
would have another downside. One could just send the fd back to itself 
via a throway forked helper, which only does one ioctl before sending 
it back, and then the "render owner" is forever lost. The proposal as 
I had it would be immune to this problem at least.



Eventually we could then use that to enforce static binding to a pid,
which is what we want for svm style models, i.e. if the pid changes, the
fd does an -EACCESS or similar.

Thoughts?


This use case I am not familiar with at all so can't comment. Only 
intuitively I would ask - why is it something that needs to be solved 
at the DRM level? Because

Re: [PATCH] [RFC] drm/etnaviv: Disable softpin