date:20221031


On 01/11/2022 01:54, Rob Clark wrote:

From: Rob Clark 

The _HI reg is always following the _LO reg, so no need to pass these
offsets seprately.

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  3 +--
  drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 27 -
  drivers/gpu/drm/msm/adreno/a5xx_preempt.c   |  4 +--
  drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 24 ++
  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |  3 +--
  drivers/gpu/drm/msm/msm_gpu.h   | 12 -
  6 files changed, 27 insertions(+), 46 deletions(-)


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry

Re: [PATCH] drm/msm: Fix return type of mdp4_lvds_connector_mode_valid

2022-10-31 Thread Abhinav Kumar





On 10/31/2022 5:24 PM, Dmitry Baryshkov wrote:

On 01/11/2022 01:10, Nathan Chancellor wrote:

On Tue, Sep 13, 2022 at 01:55:48PM -0700, Nathan Huckleberry wrote:

The mode_valid field in drm_connector_helper_funcs is expected to be of
type:
enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
  struct drm_display_mode *mode);

The mismatched return type breaks forward edge kCFI since the underlying
function definition does not match the function hook definition.

The return type of mdp4_lvds_connector_mode_valid should be changed from
int to enum drm_mode_status.

Reported-by: Dan Carpenter 
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Cc: l...@lists.linux.dev
Signed-off-by: Nathan Huckleberry 
---
  drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c

index 7288041dd86a..7444b75c4215 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -56,8 +56,9 @@ static int mdp4_lvds_connector_get_modes(struct 
drm_connector *connector)

  return ret;
  }
-static int mdp4_lvds_connector_mode_valid(struct drm_connector 
*connector,

- struct drm_display_mode *mode)
+static enum drm_mode_status
+mdp4_lvds_connector_mode_valid(struct drm_connector *connector,
+   struct drm_display_mode *mode)
  {
  struct mdp4_lvds_connector *mdp4_lvds_connector =
  to_mdp4_lvds_connector(connector);
--
2.37.2.789.g6183377224-goog




Did this patch get lost somewhere? I do not see it picked up. It is
needed to avoid a new WIP warning from clang for catching these CFI
failures:

drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c:89:16: error: 
incompatible function pointer types initializing 'enum drm_mode_status 
(*)(struct drm_connector *, struct drm_display_mode *)' with an 
expression of type 'int (struct drm_connector *, struct 
drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]

 .mode_valid = mdp4_lvds_connector_mode_valid,
   ^~
1 error generated.


It will be picked into 6.1-rc and then propagate through the stable 
kernel updates.


Anyway:

Fixes: 3e87599b68e7 ("drm/msm/mdp4: add LVDS panel support")
Reviewed-by: Dmitry Baryshkov 


Yes, it was already merged to the -fixes tree and a PR with it was sent 
out for 6.1-rc


https://gitlab.freedesktop.org/drm/msm/-/commit/0b33a33bd15d5bab73b87152b220a8d0153a4587

Re: [PATCH] drm/msm: Fix return type of mdp4_lvds_connector_mode_valid


On 01/11/2022 01:10, Nathan Chancellor wrote:

On Tue, Sep 13, 2022 at 01:55:48PM -0700, Nathan Huckleberry wrote:

The mode_valid field in drm_connector_helper_funcs is expected to be of
type:
enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
  struct drm_display_mode *mode);

The mismatched return type breaks forward edge kCFI since the underlying
function definition does not match the function hook definition.

The return type of mdp4_lvds_connector_mode_valid should be changed from
int to enum drm_mode_status.

Reported-by: Dan Carpenter 
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Cc: l...@lists.linux.dev
Signed-off-by: Nathan Huckleberry 
---
  drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
index 7288041dd86a..7444b75c4215 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
@@ -56,8 +56,9 @@ static int mdp4_lvds_connector_get_modes(struct drm_connector 
*connector)
return ret;
  }
  
-static int mdp4_lvds_connector_mode_valid(struct drm_connector *connector,

-struct drm_display_mode *mode)
+static enum drm_mode_status
+mdp4_lvds_connector_mode_valid(struct drm_connector *connector,
+  struct drm_display_mode *mode)
  {
struct mdp4_lvds_connector *mdp4_lvds_connector =
to_mdp4_lvds_connector(connector);
--
2.37.2.789.g6183377224-goog




Did this patch get lost somewhere? I do not see it picked up. It is
needed to avoid a new WIP warning from clang for catching these CFI
failures:

drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c:89:16: error: incompatible 
function pointer types initializing 'enum drm_mode_status (*)(struct 
drm_connector *, struct drm_display_mode *)' with an expression of type 'int 
(struct drm_connector *, struct drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]
 .mode_valid = mdp4_lvds_connector_mode_valid,
   ^~
1 error generated.


It will be picked into 6.1-rc and then propagate through the stable 
kernel updates.


Anyway:

Fixes: 3e87599b68e7 ("drm/msm/mdp4: add LVDS panel support")
Reviewed-by: Dmitry Baryshkov 


--
With best wishes
Dmitry

Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation

2022-10-31 Thread Jason Baron





On 10/31/22 6:11 PM, jim.cro...@gmail.com wrote:

On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
 wrote:

On Sun, Oct 30, 2022 at 08:42:52AM -0600, jim.cro...@gmail.com wrote:

On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
 wrote:

On Thu, Oct 27, 2022 at 01:55:39PM -0600, jim.cro...@gmail.com wrote:

On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
 wrote:

On Thu, Oct 27, 2022 at 09:37:52AM -0600, jim.cro...@gmail.com wrote:

On Thu, Oct 27, 2022 at 9:08 AM Jason Baron  wrote:



On 10/21/22 05:18, Jani Nikula wrote:

On Thu, 20 Oct 2022, Ville Syrjälä  wrote:

On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:

On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:

hi Greg, Dan, Jason, DRM-folk,

heres follow-up to V6:
   rebased on driver-core/driver-core-next for -v6 applied bits (thanks)
   rework drm_debug_enabled{_raw,_instrumented,} per Dan.

It excludes:
   nouveau parts (immature)
   tracefs parts (I missed --to=Steve on v6)
   split _ddebug_site and de-duplicate experiment (way unready)

IOW, its the remaining commits of V6 on which Dan gave his Reviewed-by.

If these are good to apply, I'll rebase and repost the rest separately.

All now queued up, thanks.

This stuff broke i915 debugs. When I first load i915 no debug prints are
produced. If I then go fiddle around in /sys/module/drm/parameters/debug
the debug prints start to suddenly work.

Wait what? I always assumed the default behaviour would stay the same,
which is usually how we roll. It's a regression in my books. We've got a
CI farm that's not very helpful in terms of dmesg logging right now
because of this.

BR,
Jani.



That doesn't sound good - so you are saying that prior to this change some
of the drm debugs were default enabled. But now you have to manually enable
them?

Thanks,

-Jason


Im just seeing this now.
Any new details ?

No. We just disabled it as BROKEN for now. I was just today thinking
about sending that patch out if no solutin is forthcoming soon since
we need this working before 6.1 is released.

Pretty sure you should see the problem immediately with any driver
(at least if it's built as a module, didn't try builtin). Or at least
can't think what would make i915 any more special.


So, I should note -
99% of my time & energy on this dyndbg + drm patchset
has been done using virtme,
so my world-view (and dev-hack-test env) has been smaller, simpler
maybe its been fatally simplistic.

ive just rebuilt v6.0  (before the trouble)
and run it thru my virtual home box,
I didnt see any unfamiliar drm-debug output
that I might have inadvertently altered somehow

I have some real HW I can put a reference kernel on,0
to look for the missing output, but its all gonna take some time,
esp to tighten up my dev-test-env

in the meantime, there is:

config DRM_USE_DYNAMIC_DEBUG
bool "use dynamic debug to implement drm.debug"
default y
depends on DRM
depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
depends on JUMP_LABEL
help
   Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
   Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
   bytes per callsite, the .data costs can be substantial, and
   are therefore configurable.

Does changing the default fix things for i915 dmesg ?

I think we want to mark it BROKEN in addition to make sure no one

Ok, I get the distinction now.
youre spelling that
   depends on BROKEN

I have a notional explanation, and a conflating commit:

can you eliminate
git log -p ccc2b496324c13e917ef05f563626f4e7826bef1

as the cause ?

Reverting that doesn't help.


thanks for eliminating it.


I do need to clarify, I dont know exactly what debug/logging output
is missing such that CI is failing

CI isn't failing. But any logs it produces are 100% useless,
as are any user reported logs.

The debugs that are missing are anything not coming directly
from drm.ko.

The stuff that I see being printed by i915.ko are drm_info()
and the drm_printer stuff from i915_welcome_messages(). That
also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
least still work correctly.

I suspect that the problem is just that the debug calls
aren't getting patched in when a module loads. And fiddling
with the modparam after the fact does trigger that somehow.


ok, heres the 'tape' of a virtme boot,
then modprobe going wrong.

[1.785873] dyndbg:   2 debug prints in module intel_rapl_msr
[2.040598] virtme-init: udev is done
virtme-init: console is ttyS0


load drm driver

bash-5.2# modprobe i915


drm module is loaded 1st

[6.549451] dyndbg: add-module: drm.302 sites
[6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
[6.550647] dyndbg:  0: 0 DRM_UT_CORE
[6.551097] dyndbg:  1: 1 DRM_UT_DRIVER
[6.551531] dyndbg:  2: 2 DRM_UT_KMS
[6.551931] dyndbg:  3: 3 DRM_UT_PRIME
[6.552402] dyndbg:  4: 4 DRM_UT_ATOMIC
[6.552799] dyndbg:  5: 5 DRM_UT_VBL
[6.553270] dyndbg:  6: 6 DRM_UT_STATE
[6.553634] dyndbg:  7: 7 DRM_UT_LEASE
[6.554043] dyndbg:  8: 8

Re: [PATCH 1/2] drm/msm/dsi: add a helper method to compute the dsi byte clk


On 28/10/2022 01:22, Abhinav Kumar wrote:



On 10/27/2022 10:35 AM, Dmitry Baryshkov wrote:

On 22/09/2022 03:49, Abhinav Kumar wrote:

Re-arrange the dsi_calc_pclk method to two helpers, one to
compute the DSI byte clk and the other to compute the pclk.

This makes the separation of the two clean and also allows
clients to compute and use the dsi byte clk separately.

Signed-off-by: Abhinav Kumar 
---
  drivers/gpu/drm/msm/dsi/dsi.h  |  2 ++
  drivers/gpu/drm/msm/dsi/dsi_host.c | 27 +++
  2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h 
b/drivers/gpu/drm/msm/dsi/dsi.h

index 2a96b4fe7839..60ba8e67f550 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -118,6 +118,8 @@ int dsi_link_clk_enable_6g(struct msm_dsi_host 
*msm_host);

  int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host);
  void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host);
  void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host);
+unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool 
is_bonded_dsi,

+    const struct drm_display_mode *mode);
  int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size);
  int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size);
  void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c

index 57a4c0fa614b..32b35d4ac1d3 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -569,9 +569,8 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host 
*msm_host)

  clk_disable_unprepare(msm_host->byte_clk);
  }
-static unsigned long dsi_get_pclk_rate(struct msm_dsi_host 
*msm_host, bool is_bonded_dsi)
+static unsigned long dsi_get_pclk_rate(const struct drm_display_mode 
*mode, bool is_bonded_dsi)

  {
-    struct drm_display_mode *mode = msm_host->mode;
  unsigned long pclk_rate;
  pclk_rate = mode->clock * 1000;
@@ -588,12 +587,18 @@ static unsigned long dsi_get_pclk_rate(struct 
msm_dsi_host *msm_host, bool is_bo

  return pclk_rate;
  }
-static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool 
is_bonded_dsi)
+unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool 
is_bonded_dsi,

+    const struct drm_display_mode *mode)
  {
+    struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
  u8 lanes = msm_host->lanes;
  u32 bpp = dsi_get_bpp(msm_host->format);
-    unsigned long pclk_rate = dsi_get_pclk_rate(msm_host, 
is_bonded_dsi);

-    u64 pclk_bpp = (u64)pclk_rate * bpp;
+    unsigned long pclk_rate;
+    u64 pclk_bpp;
+
+    pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi);
+
+    pclk_bpp = (u64)pclk_rate * bpp;
  if (lanes == 0) {
  pr_err("%s: forcing mdss_dsi lanes to 1\n", __func__);
@@ -606,8 +611,14 @@ static void dsi_calc_pclk(struct msm_dsi_host 
*msm_host, bool is_bonded_dsi)

  else
  do_div(pclk_bpp, (8 * lanes));
-    msm_host->pixel_clk_rate = pclk_rate;
-    msm_host->byte_clk_rate = pclk_bpp;
+    return pclk_bpp;
+}
+
+static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool 
is_bonded_dsi)

+{
+    msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, 
is_bonded_dsi);
+    msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, 
is_bonded_dsi,

+    msm_host->mode);


This way you are calling dsi_get_pclk_rate twice(), which is slightly 
inefficient. You can call it once (here) and then pass the resulting 
pclk_rate as an argument to dsi_byte_clk_get_rate().


So the goal was to have two independent APIs to calculate byte and pixel 
clk.


If we pass the output of one as the input to the other we are making 
them dependent.


Thats why i kept it separate.


Calling one function from another clearly points that they are not 
independent. And surely pixel and byte clocks can not be fully 
independent. I see your point about getting only the byte clock. But I 
think it would be easier to explicitly pass the pixel rate rather than 
calculating it again under the hood.







  DBG("pclk=%lu, bclk=%lu", msm_host->pixel_clk_rate,
  msm_host->byte_clk_rate);
@@ -635,7 +646,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host 
*msm_host, bool is_bonded_dsi)

  dsi_calc_pclk(msm_host, is_bonded_dsi);
-    pclk_bpp = (u64)dsi_get_pclk_rate(msm_host, is_bonded_dsi) * bpp;
+    pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) 
* bpp;


So... We have calculated all rates, stored the pclk_rate in 
msm_host->pixel_clk_rate. And now we are going to calculate it again. 
As you are touching this line of code, I'd suggest to just use 
msm_host->pixel_clk_rate instead of a function call.


Ack, I will fix this.




  do_div(pclk_bpp, 8);
  msm_host->src_clk_rate = pclk_bpp;




--
With best wishes
Dmitry

Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3


On 01/11/2022 03:08, Doug Anderson wrote:

Hi,

On Mon, Oct 31, 2022 at 2:11 PM Kuogee Hsieh  wrote:


Hi Dmitry,


Link rate is advertised by sink, but adjusted (reduced the link rate)
by host during link training.

Therefore should be fine if host did not support HBR3 rate.

It will reduce to lower link rate during link training procedures.

kuogee

On 10/31/2022 11:46 AM, Dmitry Baryshkov wrote:

On 31/10/2022 20:27, Kuogee Hsieh wrote:

An HBR3-capable device shall also support TPS4. Since TPS4 feature
had been implemented already, it is not necessary to limit link
rate at HBR2 (5.4G). This patch remove this limitation to support
HBR3 (8.1G) link rate.


The DP driver supports several platforms including sdm845 and can
support, if I'm not mistaken, platforms up to msm8998/sdm630/660.
Could you please confirm that all these SoCs have support for HBR3?

With that fact being confirmed:

Reviewed-by: Dmitry Baryshkov 




Signed-off-by: Kuogee Hsieh 
---
   drivers/gpu/drm/msm/dp/dp_panel.c | 4 
   1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149ceb..3344f5a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel
*dp_panel)
   if (link_info->num_lanes > dp_panel->max_dp_lanes)
   link_info->num_lanes = dp_panel->max_dp_lanes;
   -/* Limit support upto HBR2 until HBR3 support is added */
-if (link_info->rate >=
(drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
-link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
-
   drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
   drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
   drm_dbg_dp(panel->drm_dev, "lane_count=%d\n",
link_info->num_lanes);


Stephen might remember better, but I could have sworn that the problem
was that there might be something in the middle that couldn't support
the higher link rate. In other words, I think we have:

SoC <--> TypeC Port Controller <--> Display

The SoC might support HBR3 and the display might support HBR3, but the
TCPC (Type C Port Controller) might not. I think that the TCPC is a
silent/passive component so it can't really let anyone know about its
limitations.

In theory I guess you could rely on link training to just happen to
fail if you drive the link too fast for the TCPC to handle. Does this
actually work reliably?

I think the other option that was discussed in the past was to add
something in the device tree for this. Either you could somehow model
the TCPC in DRM and thus know that a given model of TCPC limits the
link rate or you could hack in a property in the DP controller to
limit it.


Latest pmic_glink proposal from Bjorn include adding the drm_bridge for 
the TCPC. Such bridge can in theory limit supported modes and rates.


--
With best wishes
Dmitry

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane

Hi,

On 01/11/2022 01:24, Jessica Zhang wrote:

On 10/29/2022 5:08 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, Jessica Zhang wrote:

Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
drm_plane. In addition, add support for setting and getting the values
of these properties.

COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
represents the format of the color fill. Userspace can set enable solid
fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
framebuffer to NULL.

Signed-off-by: Jessica Zhang >
Planes report supported formats using the drm_mode_getplane(). You'd
also need to tell userspace, which formats are supported for color
fill. I don't think one supports e.g. YV12.

Hey Dmitry,

Good point. Forgot to add this in the patch [3/3] commit message, but
FWIW MSM DPU devices only support the RGB format variants [1].

[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L697

Ack. So you'd need to tell this to userspace.

A bit of generic comment for the discussion (this is an RFC anyway).
Using color_fill/color_fill_format properties sounds simple, but this
might be not generic enough. Limiting color_fill to 32 bits would
prevent anybody from using floating point formats (e.g.
DRM_FORMAT_XRGB16161616F, 64-bit value). Yes, this can be solved with
e.g. using 64-bit for the color_fill value, but then this doesn't
sound extensible too much.

Hm... I can definitely change color_fill to use u64 instead. As for
making it more extensible, do you have any suggestions?

No. Not u64. It is a blob. Basically because when designing API you can
not guarantee that all fill values would fit into u64. Also see below.

So, a question for other hardware maintainers. Do we have hardware
that supports such 'color filled' planes? Do we want to support format
modifiers for filling color/data? Because what I have in mind is
closer to the blob structure, which can then be used for filling the
plane:

struct color_fill_blob {
u32 pixel_format;
u64 modifiers4];
u32 pixel_data_size; // fixme: is this necessary?
u8 pixel_data[];
};

Just a question about this blob struct -- what is the purpose of
pixel_data?

At least for devices using the DPU driver, the only data needed to
enable solid fill is color_fill and color_fill_format. I'd also be
interested in knowing if there are other drivers support a similar
feature and what is needed for them.

Yes. You are thinking from the DPU point of view. ARGB only. However as
we are adding generic API, we should not limit ourselves to it. Other
deivces might support other formats of fill data. For example using
YUY2/UYVY for filling the plane. And such YUV data is not a colour
anymore. It is a pixel data, just as I named it.

Another hardware might support some fill patterns. Or e.g. passing a
compressed texels/macrotiles. So, pixel data. Note, I've added format
modifiers. Maybe `u64 modifiers[4]` is an overkill, as we have just a
single data plane. Maybe just `u64 modifier` would be enough.

And then... This sounds a lot like a custom framebuffer.

So, maybe what should we do instead is to add new
DRM_MODE_FB_COLOR_FILL flag to the framebuffers, which would e.g. mean
that the FB gets stamped all over the plane. This would also save us
from changing if (!fb) checks all over the drm core.

JFYI we did originally consider using a custom 1x1 FB to for color fill
[1], but decided to go with a plane property instead. IIRC the
conclusion was that having color fill as a plane property is a cleaner
solution.

As for creating a new blob struct to hold all color fill info, I'm open
to implementing that over having 2 separate properties.

[1] https://oftc.irclog.whitequark.org/dri-devel/2022-09-23#31409842

Let me cite the conclusion form the IRC chat: `22:20
abhinav__: kinda.. the proposal was that userspace creates a blob
property with the solid fill color, and then attaches the blob-prop id
to the plane's FB_ID`.

It's not a pair of properties. It is a blob, because it is not that
limited as the pair of range properties is.

I will reread the log later, but just my 2c. Attaching the blob property
as the FB_ID might confuse userspace. I'd be slightly biased to being
more conservative here. However as the final proposal was to attach the
blob ID, let's do it this way.

Another approach might be using a format modifier instead of the FB flag.
I like the idea of having a format modifier denoting if the driver
supports color_fill for that specific format. This would allow userspace
to know which formats are supported by solid fill planes.

Yes, exactly. It would come in a natural way.

[Rumbling: and then it's natural to have the fill data in FB. Dull mode
off.]

--
With best wishes
Dmitry

Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3

2022-10-31 Thread Doug Anderson

Hi,

On Mon, Oct 31, 2022 at 2:11 PM Kuogee Hsieh  wrote:
>
> Hi Dmitry,
>
>
> Link rate is advertised by sink, but adjusted (reduced the link rate)
> by host during link training.
>
> Therefore should be fine if host did not support HBR3 rate.
>
> It will reduce to lower link rate during link training procedures.
>
> kuogee
>
> On 10/31/2022 11:46 AM, Dmitry Baryshkov wrote:
> > On 31/10/2022 20:27, Kuogee Hsieh wrote:
> >> An HBR3-capable device shall also support TPS4. Since TPS4 feature
> >> had been implemented already, it is not necessary to limit link
> >> rate at HBR2 (5.4G). This patch remove this limitation to support
> >> HBR3 (8.1G) link rate.
> >
> > The DP driver supports several platforms including sdm845 and can
> > support, if I'm not mistaken, platforms up to msm8998/sdm630/660.
> > Could you please confirm that all these SoCs have support for HBR3?
> >
> > With that fact being confirmed:
> >
> > Reviewed-by: Dmitry Baryshkov 
> >
> >
> >>
> >> Signed-off-by: Kuogee Hsieh 
> >> ---
> >>   drivers/gpu/drm/msm/dp/dp_panel.c | 4 
> >>   1 file changed, 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> >> b/drivers/gpu/drm/msm/dp/dp_panel.c
> >> index 5149ceb..3344f5a 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> >> @@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel
> >> *dp_panel)
> >>   if (link_info->num_lanes > dp_panel->max_dp_lanes)
> >>   link_info->num_lanes = dp_panel->max_dp_lanes;
> >>   -/* Limit support upto HBR2 until HBR3 support is added */
> >> -if (link_info->rate >=
> >> (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
> >> -link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
> >> -
> >>   drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
> >>   drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
> >>   drm_dbg_dp(panel->drm_dev, "lane_count=%d\n",
> >> link_info->num_lanes);

Stephen might remember better, but I could have sworn that the problem
was that there might be something in the middle that couldn't support
the higher link rate. In other words, I think we have:

SoC <--> TypeC Port Controller <--> Display

The SoC might support HBR3 and the display might support HBR3, but the
TCPC (Type C Port Controller) might not. I think that the TCPC is a
silent/passive component so it can't really let anyone know about its
limitations.

In theory I guess you could rely on link training to just happen to
fail if you drive the link too fast for the TCPC to handle. Does this
actually work reliably?

I think the other option that was discussed in the past was to add
something in the device tree for this. Either you could somehow model
the TCPC in DRM and thus know that a given model of TCPC limits the
link rate or you could hack in a property in the DP controller to
limit it.

-Doug

Re: [PATCH 00/10] Connect VFIO to IOMMUFD

2022-10-31 Thread Jason Gunthorpe

On Mon, Oct 31, 2022 at 08:25:39PM +0800, Yi Liu wrote:
> > There is something wrong with the test suite that it isn't covering
> > the above, I'm going to look into that today.
> 
> sounds to be the cause. I didn't see any significant change in vfio_main.c
> that may fail gvt. So should the iommufd changes. Then we will re-run the
> test after your update.:-)

I updated the github with all the changes made so far, it is worth
trying again!

Thanks,
Jason

[PATCH 1/2] drm/msm/adreno: Simplify read64/write64 helpers

2022-10-31 Thread Rob Clark

From: Rob Clark 

The _HI reg is always following the _LO reg, so no need to pass these
offsets seprately.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  3 +--
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 27 -
 drivers/gpu/drm/msm/adreno/a5xx_preempt.c   |  4 +--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 24 ++
 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |  3 +--
 drivers/gpu/drm/msm/msm_gpu.h   | 12 -
 6 files changed, 27 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 7cb8d9849c07..a10feb8a4194 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -606,8 +606,7 @@ static int a4xx_pm_suspend(struct msm_gpu *gpu) {
 
 static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 {
-   *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO,
-   REG_A4XX_RBBM_PERFCTR_CP_0_HI);
+   *value = gpu_read64(gpu, REG_A4XX_RBBM_PERFCTR_CP_0_LO);
 
return 0;
 }
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 3dcec7acb384..ba22d3c918bc 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -605,11 +605,9 @@ static int a5xx_ucode_init(struct msm_gpu *gpu)
a5xx_ucode_check_version(a5xx_gpu, a5xx_gpu->pfp_bo);
}
 
-   gpu_write64(gpu, REG_A5XX_CP_ME_INSTR_BASE_LO,
-   REG_A5XX_CP_ME_INSTR_BASE_HI, a5xx_gpu->pm4_iova);
+   gpu_write64(gpu, REG_A5XX_CP_ME_INSTR_BASE_LO, a5xx_gpu->pm4_iova);
 
-   gpu_write64(gpu, REG_A5XX_CP_PFP_INSTR_BASE_LO,
-   REG_A5XX_CP_PFP_INSTR_BASE_HI, a5xx_gpu->pfp_iova);
+   gpu_write64(gpu, REG_A5XX_CP_PFP_INSTR_BASE_LO, a5xx_gpu->pfp_iova);
 
return 0;
 }
@@ -868,8 +866,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
 * memory rendering at this point in time and we don't want to block off
 * part of the virtual memory space.
 */
-   gpu_write64(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_LO,
-   REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_HI, 0x);
+   gpu_write64(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_BASE_LO, 0x);
gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_TRUSTED_SIZE, 0x);
 
/* Put the GPU into 64 bit by default */
@@ -908,8 +905,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
return ret;
 
/* Set the ringbuffer address */
-   gpu_write64(gpu, REG_A5XX_CP_RB_BASE, REG_A5XX_CP_RB_BASE_HI,
-   gpu->rb[0]->iova);
+   gpu_write64(gpu, REG_A5XX_CP_RB_BASE, gpu->rb[0]->iova);
 
/*
 * If the microcode supports the WHERE_AM_I opcode then we can use that
@@ -936,7 +932,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
}
 
gpu_write64(gpu, REG_A5XX_CP_RB_RPTR_ADDR,
-   REG_A5XX_CP_RB_RPTR_ADDR_HI, shadowptr(a5xx_gpu, 
gpu->rb[0]));
+   shadowptr(a5xx_gpu, gpu->rb[0]));
} else if (gpu->nr_rings > 1) {
/* Disable preemption if WHERE_AM_I isn't available */
a5xx_preempt_fini(gpu);
@@ -1239,9 +1235,9 @@ static void a5xx_fault_detect_irq(struct msm_gpu *gpu)
gpu_read(gpu, REG_A5XX_RBBM_STATUS),
gpu_read(gpu, REG_A5XX_CP_RB_RPTR),
gpu_read(gpu, REG_A5XX_CP_RB_WPTR),
-   gpu_read64(gpu, REG_A5XX_CP_IB1_BASE, REG_A5XX_CP_IB1_BASE_HI),
+   gpu_read64(gpu, REG_A5XX_CP_IB1_BASE),
gpu_read(gpu, REG_A5XX_CP_IB1_BUFSZ),
-   gpu_read64(gpu, REG_A5XX_CP_IB2_BASE, REG_A5XX_CP_IB2_BASE_HI),
+   gpu_read64(gpu, REG_A5XX_CP_IB2_BASE),
gpu_read(gpu, REG_A5XX_CP_IB2_BUFSZ));
 
/* Turn off the hangcheck timer to keep it from bothering us */
@@ -1427,8 +1423,7 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
 
 static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 {
-   *value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO,
-   REG_A5XX_RBBM_ALWAYSON_COUNTER_HI);
+   *value = gpu_read64(gpu, REG_A5XX_RBBM_ALWAYSON_COUNTER_LO);
 
return 0;
 }
@@ -1465,8 +1460,7 @@ static int a5xx_crashdumper_run(struct msm_gpu *gpu,
if (IS_ERR_OR_NULL(dumper->ptr))
return -EINVAL;
 
-   gpu_write64(gpu, REG_A5XX_CP_CRASH_SCRIPT_BASE_LO,
-   REG_A5XX_CP_CRASH_SCRIPT_BASE_HI, dumper->iova);
+   gpu_write64(gpu, REG_A5XX_CP_CRASH_SCRIPT_BASE_LO, dumper->iova);
 
gpu_write(gpu, REG_A5XX_CP_CRASH_DUMP_CNTL, 1);
 
@@ -1666,8 +1660,7 @@ static u64 a5xx_gpu_busy(struct msm_gpu *gpu, unsigned 
long *out_sample_rate)
 {
u64 busy_cycles;
 
-   busy_cycles = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
-   REG_A5XX_RBBM_PERF

[PATCH 2/2] drm/msm: Hangcheck progress detection

2022-10-31 Thread Rob Clark

From: Rob Clark 

If the hangcheck timer expires, check if the fw's position in the
cmdstream has advanced (changed) since last timer expiration, and
allow it up to three additional "extensions" to it's alotted time.
The intention is to continue to catch "shader stuck in a loop" type
hangs quickly, but allow more time for things that are actually
making forward progress.

Because we need to sample the CP state twice to detect if there has
not been progress, this also cuts the the timer's duration in half.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 34 +++
 drivers/gpu/drm/msm/msm_drv.h |  8 ++-
 drivers/gpu/drm/msm/msm_gpu.c | 20 +++-
 drivers/gpu/drm/msm/msm_gpu.h |  5 +++-
 drivers/gpu/drm/msm/msm_ringbuffer.h  | 24 +++
 5 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 1ff605c18ee6..3b8fb7a11dff 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1843,6 +1843,39 @@ static uint32_t a6xx_get_rptr(struct msm_gpu *gpu, 
struct msm_ringbuffer *ring)
return ring->memptrs->rptr = gpu_read(gpu, REG_A6XX_CP_RB_RPTR);
 }
 
+static bool a6xx_progress(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
+{
+   struct msm_cp_state cp_state = {
+   .ib1_base = gpu_read64(gpu, REG_A6XX_CP_IB1_BASE),
+   .ib2_base = gpu_read64(gpu, REG_A6XX_CP_IB2_BASE),
+   .ib1_rem  = gpu_read(gpu, REG_A6XX_CP_IB1_REM_SIZE),
+   .ib2_rem  = gpu_read(gpu, REG_A6XX_CP_IB2_REM_SIZE),
+   };
+   bool progress;
+
+   /*
+* Adjust the remaining data to account for what has already been
+* fetched from memory, but not yet consumed by the SQE.
+*
+* This is not *technically* correct, the amount buffered could
+* exceed the IB size due to hw prefetching ahead, but:
+*
+* (1) We aren't trying to find the exact position, just whether
+* progress has been made
+* (2) The CP_REG_TO_MEM at the end of a submit should be enough
+* to prevent prefetching into an unrelated submit.  (And
+* either way, at some point the ROQ will be full.)
+*/
+   cp_state.ib1_rem += gpu_read(gpu, REG_A6XX_CP_CSQ_IB1_STAT) >> 16;
+   cp_state.ib2_rem += gpu_read(gpu, REG_A6XX_CP_CSQ_IB1_STAT) >> 16;
+
+   progress = !!memcmp(&cp_state, &ring->last_cp_state, sizeof(cp_state));
+
+   ring->last_cp_state = cp_state;
+
+   return progress;
+}
+
 static u32 a618_get_speed_bin(u32 fuse)
 {
if (fuse == 0)
@@ -1961,6 +1994,7 @@ static const struct adreno_gpu_funcs funcs = {
.create_address_space = a6xx_create_address_space,
.create_private_address_space = 
a6xx_create_private_address_space,
.get_rptr = a6xx_get_rptr,
+   .progress = a6xx_progress,
},
.get_timestamp = a6xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index efcd7260f428..970a1a0ab34f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -226,7 +226,13 @@ struct msm_drm_private {
 
struct drm_atomic_state *pm_state;
 
-   /* For hang detection, in ms */
+   /**
+* hangcheck_period: For hang detection, in ms
+*
+* Note that in practice, a submit/job will get at least two hangcheck
+* periods, due to checking for progress being implemented as simply
+* "have the CP position registers changed since last time?"
+*/
unsigned int hangcheck_period;
 
/**
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 3dffee54a951..136f5977b0bf 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -500,6 +500,21 @@ static void hangcheck_timer_reset(struct msm_gpu *gpu)
round_jiffies_up(jiffies + 
msecs_to_jiffies(priv->hangcheck_period)));
 }
 
+static bool made_progress(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
+{
+   if (ring->hangcheck_progress_retries >= 
DRM_MSM_HANGCHECK_PROGRESS_RETRIES)
+   return false;
+
+   if (!gpu->funcs->progress)
+   return false;
+
+   if (!gpu->funcs->progress(gpu, ring))
+   return false;
+
+   ring->hangcheck_progress_retries++;
+   return true;
+}
+
 static void hangcheck_handler(struct timer_list *t)
 {
struct msm_gpu *gpu = from_timer(gpu, t, hangcheck_timer);
@@ -511,9 +526,12 @@ static void hangcheck_handler(struct timer_list *t)
if (fence != ring->hangcheck_fence) {
/* some progress has been made.. ya! */
ring->hangcheck_fence = fence;
-   } else if (fence_before(fence, ring->fctx->last_fence)) {
+   ring->ha

[PATCH 0/2] drm/msm: Improved hang detection

2022-10-31 Thread Rob Clark

From: Rob Clark 

Try to detect when submit jobs are making forward progress and give them
a bit more time.

Rob Clark (2):
  drm/msm/adreno: Simplify read64/write64 helpers
  drm/msm: Hangcheck progress detection

 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  3 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 27 --
 drivers/gpu/drm/msm/adreno/a5xx_preempt.c   |  4 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 58 +++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |  3 +-
 drivers/gpu/drm/msm/msm_drv.h   |  8 ++-
 drivers/gpu/drm/msm/msm_gpu.c   | 20 ++-
 drivers/gpu/drm/msm/msm_gpu.h   | 17 +++---
 drivers/gpu/drm/msm/msm_ringbuffer.h| 24 +
 9 files changed, 115 insertions(+), 49 deletions(-)

-- 
2.37.3

Re: [PATCH 10/10] iommufd: Allow iommufd to supply /dev/vfio/vfio

2022-10-31 Thread Alex Williamson

On Fri, 28 Oct 2022 15:44:36 -0300
Jason Gunthorpe  wrote:

> On Wed, Oct 26, 2022 at 03:31:33PM -0600, Alex Williamson wrote:
> > On Tue, 25 Oct 2022 15:50:45 -0300
> > Jason Gunthorpe  wrote:
> >   
> > > If the VFIO container is compiled out, give a kconfig option for iommufd
> > > to provide the miscdev node with the same name and permissions as vfio
> > > uses.
> > > 
> > > The compatibility node supports the same ioctls as VFIO and automatically
> > > enables the VFIO compatible pinned page accounting mode.  
> > 
> > I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is
> > provided by something other than the vfio container code.  If we intend
> > to include this before P2P is resolved, that breadcrumb   
> 
> I don't belive I can get P2P done soon enough. I plan to do it after
> this is merged. Right now these two series are taking all my time.
> 
> > (dmesg I'm guessing) might also list any known limitations of the
> > compatibility to save time with debugging.  Thanks,  
> 
> Yes, that makes sense.
> 
> Do you want a dmesg at module load time, on every open, or a sysfs
> something? What seems like it would make it into a bug report?

I think dmesg at module load time should probably be ok, every open
seems like harassment and sysfs would require updated support in
various bug reporting tools.  Users are often terrible about reporting
full dmesg in bugs, but they do often filter it for "IOMMU" or "VFIO",
so keep that in mind when crafting the log message.  Thanks,

Alex

Re: [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c

2022-10-31 Thread Alex Williamson

On Fri, 28 Oct 2022 15:40:09 -0300
Jason Gunthorpe  wrote:

> On Wed, Oct 26, 2022 at 03:24:42PM -0600, Alex Williamson wrote:
> > On Tue, 25 Oct 2022 15:17:10 -0300
> > Jason Gunthorpe  wrote:
> >   
> > > This legacy module knob has become uAPI, when set on the vfio_iommu_type1
> > > it disables some security protections in the iommu drivers. Move the
> > > storage for this knob to vfio_main.c so that iommufd can access it too.  
> > 
> > I don't really understand this, we're changing the behavior of the
> > iommufd_device_attach() operation based on the modules options of
> > vfio_iommu_type1,   
> 
> The specific reason it was done is that we had a misconfigured test VM
> in the farm that needed it, and that VM has since been fixed. But it
> did highlight we should try to preserve this in some way.
> 
> > which may not be loaded or even compiled into the
> > kernel.  Our compatibility story falls apart when VFIO_CONTAINER is not
> > set, iommufd sneaks in to usurp /dev/vfio/vfio, and the user's module
> > options for type1 go unprocessed.  
> 
> There are two aspects here, trying to preseve the
> allow_unsafe_interrupts knob as it is already as some ABI in the best
> way we can.
> 
> And the second is how do we make this work in the new world where
> there may be no type 1 module at all. This patch is not trying to
> address that topic. I am expecting a range of small adjustments before
> VFIO_CONTAINER=n becomes really fully viable.
> 
> > I hate to suggest that type1 becomes a module that does nothing more
> > than maintain consistency of this variable when the full type1 isn't
> > available, but is that what we need to do?  
> 
> It is one idea, it depends how literal you want to be on "module
> parameters are ABI". IMHO it is a weak form of ABI and the need of
> this paramter in particular is not that common in modern times, AFAIK.
> 
> So perhaps we just also expose it through vfio.ko and expect people to
> migrate. That would give a window were both options are available.

That might be best.  Ultimately this is an opt-out of a feature that
has security implications, so I'd rather error on the side of requiring
the user to re-assert that opt-out.  It seems the potential good in
eliminating stale or unnecessary options outweighs any weak claims of
preserving an ABI for a module that's no longer in service.

However, I'd question whether vfio is the right place for that new
module option.  As proposed, vfio is only passing it through to
iommufd, where an error related to lack of the hardware feature is
masked behind an -EPERM by the time it gets back to vfio, making any
sort of advisory to the user about the module option convoluted.  It
seems like iommufd should own the option to opt-out universally, not
just through the vfio use case.  Thanks,

Alex

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane

On 10/29/2022 5:08 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, Jessica Zhang wrote:

Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
drm_plane. In addition, add support for setting and getting the values
of these properties.

Hey Dmitry,

Good point. Forgot to add this in the patch [3/3] commit message, but
FWIW MSM DPU devices only support the RGB format variants [1].

[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L697

Hm... I can definitely change color_fill to use u64 instead. As for
making it more extensible, do you have any suggestions?

So, a question for other hardware maintainers. Do we have hardware that
supports such 'color filled' planes? Do we want to support format
modifiers for filling color/data? Because what I have in mind is closer
to the blob structure, which can then be used for filling the plane:

struct color_fill_blob {
u32 pixel_format;
u64 modifiers4];
u32 pixel_data_size; // fixme: is this necessary?
u8 pixel_data[];
};

Just a question about this blob struct -- what is the purpose of pixel_data?

And then... This sounds a lot like a custom framebuffer.

So, maybe what should we do instead is to add new DRM_MODE_FB_COLOR_FILL
flag to the framebuffers, which would e.g. mean that the FB gets stamped
all over the plane. This would also save us from changing if (!fb)
checks all over the drm core.

As for creating a new blob struct to hold all color fill info, I'm open
to implementing that over having 2 separate properties.

[1] https://oftc.irclog.whitequark.org/dri-devel/2022-09-23#31409842

Thanks,

Jessica Zhang

What do you think?

--
With best wishes
Dmitry

Re: [PATCH] drm: xlnx: Fix return type of zynqmp_dp_connector_mode_valid

2022-10-31 Thread Nathan Chancellor

Hi all,

On Tue, Sep 13, 2022 at 01:56:00PM -0700, Nathan Huckleberry wrote:
> The mode_valid field in drm_connector_helper_funcs is expected to be of
> type
> enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
>  struct drm_display_mode *mode);
> 
> The mismatched return type breaks forward edge kCFI since the underlying
> function definition does not match the function hook definition.
> 
> The return type of zynqmp_dp_connector_mode_valid should be changed from
> int to enum drm_mode_status.
> 
> Reported-by: Dan Carpenter 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> Cc: l...@lists.linux.dev
> Signed-off-by: Nathan Huckleberry 
> ---
>  drivers/gpu/drm/xlnx/zynqmp_dp.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c 
> b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index d14612b34796..f571b08d23d3 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1352,8 +1352,9 @@ zynqmp_dp_connector_best_encoder(struct drm_connector 
> *connector)
>   return &dp->encoder;
>  }
>  
> -static int zynqmp_dp_connector_mode_valid(struct drm_connector *connector,
> -   struct drm_display_mode *mode)
> +static enum drm_mode_status
> +zynqmp_dp_connector_mode_valid(struct drm_connector *connector,
> +struct drm_display_mode *mode)
>  {
>   struct zynqmp_dp *dp = connector_to_dp(connector);
>   u8 max_lanes = dp->link_config.max_lanes;
> -- 
> 2.37.2.789.g6183377224-goog
> 
> 

Did this patch get lost somewhere? I do not see it picked up. It is
needed to avoid a new WIP warning from clang for catching these CFI
failures:

drivers/gpu/drm/xlnx/zynqmp_dp.c:1396:16: error: incompatible function pointer 
types initializing 'enum drm_mode_status (*)(struct drm_connector *, struct 
drm_display_mode *)' with an expression of type 'int (struct drm_connector *, 
struct drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]
.mode_valid = zynqmp_dp_connector_mode_valid,
  ^~
1 error generated.

Cheers,
Nathan

Re: [RFC PATCH 3/3] drm/msm/dpu: Use color_fill property for DPU planes





On 10/29/2022 4:40 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, Jessica Zhang wrote:

Initialize and use the color_fill properties for planes in DPU driver. In
addition, relax framebuffer requirements within atomic commit path and
add checks for NULL framebuffers.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  7 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++-
  2 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index 13ce321283ff..157698b4f234 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -441,7 +441,12 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  sspp_idx - SSPP_VIG0,
  state->fb ? state->fb->base.id : -1);
-    format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+    if (pstate->base.fb)
+    format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));

+    else if (state->color_fill && !state->color_fill_format)
+    format = dpu_get_dpu_format(DRM_FORMAT_ABGR);


As I wrote in the review of the earlier patch, this disallows using 
black as the plane fill colour. Not to mention that using ABGR should be 
explicit rather than implicit.


Hey Dmitry,

Acked.

Thanks,

Jessica Zhang




+    else
+    format = dpu_get_dpu_format(state->color_fill_format);
  if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
  bg_alpha_enable = true;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

index 658005f609f4..f3be37e97b64 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -103,7 +103,6 @@ struct dpu_plane {
  enum dpu_sspp pipe;
  struct dpu_hw_pipe *pipe_hw;
-    uint32_t color_fill;
  bool is_error;
  bool is_rt_pipe;
  const struct dpu_mdss_cfg *catalog;
@@ -697,7 +696,10 @@ static int _dpu_plane_color_fill(struct dpu_plane 
*pdpu,

   * select fill format to match user property expectation,
   * h/w only supports RGB variants
   */
-    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    if (plane->state->color_fill && !plane->state->color_fill_format)
+    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    else
+    fmt = dpu_get_dpu_format(plane->state->color_fill_format);
  /* update sspp */
  if (fmt && pdpu->pipe_hw->ops.setup_solidfill) {
@@ -720,6 +722,10 @@ static int _dpu_plane_color_fill(struct dpu_plane 
*pdpu,

  fmt, DPU_SSPP_SOLID_FILL,
  pstate->multirect_index);
+    /* skip remaining processing on color fill */
+    if (!plane->state->fb)
+    return 0;
+
  if (pdpu->pipe_hw->ops.setup_rects)
  pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw,
  &pipe_cfg,
@@ -999,12 +1005,21 @@ static int dpu_plane_atomic_check(struct 
drm_plane *plane,

  dst = drm_plane_state_dest(new_plane_state);
-    fb_rect.x2 = new_plane_state->fb->width;
-    fb_rect.y2 = new_plane_state->fb->height;
+    if (new_plane_state->fb) {
+    fb_rect.x2 = new_plane_state->fb->width;
+    fb_rect.y2 = new_plane_state->fb->height;
+    }
  max_linewidth = pdpu->catalog->caps->max_linewidth;
-    fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+    if (new_plane_state->fb) {
+    fmt = 
to_dpu_format(msm_framebuffer_format(new_plane_state->fb));

+    } else if (new_plane_state->color_fill) {
+    if (new_plane_state->color_fill_format)
+    fmt = 
dpu_get_dpu_format(new_plane_state->color_fill_format);

+    else
+    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    }
  min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -1016,7 +1031,7 @@ static int dpu_plane_atomic_check(struct 
drm_plane *plane,

  return -EINVAL;
  /* check src bounds */
-    } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
+    } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, 
&fb_rect, min_src_size)) {

  DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
  DRM_RECT_ARG(&src));
  return -E2BIG;
@@ -1084,9 +1099,9 @@ void dpu_plane_flush(struct drm_plane *plane)
  if (pdpu->is_error)
  /* force white frame with 100% alpha pipe output on error */
  _dpu_plane_color_fill(pdpu, 0xFF, 0xFF);
-    else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
+    else if (!(plane->state->fb) && plane->state->color_fill)
  /* force 100% alpha */
-    _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
+    _dpu_plane_color_fill(pdpu, plane->state->color_fill, 0xFF);
  else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) {
  const struct dpu_forma

Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation

2022-10-31 Thread jim . cromie

On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
 wrote:
>
> On Sun, Oct 30, 2022 at 08:42:52AM -0600, jim.cro...@gmail.com wrote:
> > On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
> >  wrote:
> > >
> > > On Thu, Oct 27, 2022 at 01:55:39PM -0600, jim.cro...@gmail.com wrote:
> > > > On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
> > > >  wrote:
> > > > >
> > > > > On Thu, Oct 27, 2022 at 09:37:52AM -0600, jim.cro...@gmail.com wrote:
> > > > > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron  
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > > > > On Thu, 20 Oct 2022, Ville Syrjälä 
> > > > > > > >  wrote:
> > > > > > > >> On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:
> > > > > > > >>> On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:
> > > > > > >  hi Greg, Dan, Jason, DRM-folk,
> > > > > > > 
> > > > > > >  heres follow-up to V6:
> > > > > > >    rebased on driver-core/driver-core-next for -v6 applied 
> > > > > > >  bits (thanks)
> > > > > > >    rework drm_debug_enabled{_raw,_instrumented,} per Dan.
> > > > > > > 
> > > > > > >  It excludes:
> > > > > > >    nouveau parts (immature)
> > > > > > >    tracefs parts (I missed --to=Steve on v6)
> > > > > > >    split _ddebug_site and de-duplicate experiment (way 
> > > > > > >  unready)
> > > > > > > 
> > > > > > >  IOW, its the remaining commits of V6 on which Dan gave his 
> > > > > > >  Reviewed-by.
> > > > > > > 
> > > > > > >  If these are good to apply, I'll rebase and repost the rest 
> > > > > > >  separately.
> > > > > > > >>>
> > > > > > > >>> All now queued up, thanks.
> > > > > > > >>
> > > > > > > >> This stuff broke i915 debugs. When I first load i915 no debug 
> > > > > > > >> prints are
> > > > > > > >> produced. If I then go fiddle around in 
> > > > > > > >> /sys/module/drm/parameters/debug
> > > > > > > >> the debug prints start to suddenly work.
> > > > > > > >
> > > > > > > > Wait what? I always assumed the default behaviour would stay 
> > > > > > > > the same,
> > > > > > > > which is usually how we roll. It's a regression in my books. 
> > > > > > > > We've got a
> > > > > > > > CI farm that's not very helpful in terms of dmesg logging right 
> > > > > > > > now
> > > > > > > > because of this.
> > > > > > > >
> > > > > > > > BR,
> > > > > > > > Jani.
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > > That doesn't sound good - so you are saying that prior to this 
> > > > > > > change some
> > > > > > > of the drm debugs were default enabled. But now you have to 
> > > > > > > manually enable
> > > > > > > them?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > -Jason
> > > > > >
> > > > > >
> > > > > > Im just seeing this now.
> > > > > > Any new details ?
> > > > >
> > > > > No. We just disabled it as BROKEN for now. I was just today thinking
> > > > > about sending that patch out if no solutin is forthcoming soon since
> > > > > we need this working before 6.1 is released.
> > > > >
> > > > > Pretty sure you should see the problem immediately with any driver
> > > > > (at least if it's built as a module, didn't try builtin). Or at least
> > > > > can't think what would make i915 any more special.
> > > > >
> > > >
> > > > So, I should note -
> > > > 99% of my time & energy on this dyndbg + drm patchset
> > > > has been done using virtme,
> > > > so my world-view (and dev-hack-test env) has been smaller, simpler
> > > > maybe its been fatally simplistic.
> > > >
> > > > ive just rebuilt v6.0  (before the trouble)
> > > > and run it thru my virtual home box,
> > > > I didnt see any unfamiliar drm-debug output
> > > > that I might have inadvertently altered somehow
> > > >
> > > > I have some real HW I can put a reference kernel on,0
> > > > to look for the missing output, but its all gonna take some time,
> > > > esp to tighten up my dev-test-env
> > > >
> > > > in the meantime, there is:
> > > >
> > > > config DRM_USE_DYNAMIC_DEBUG
> > > > bool "use dynamic debug to implement drm.debug"
> > > > default y
> > > > depends on DRM
> > > > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > > > depends on JUMP_LABEL
> > > > help
> > > >   Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > > >   Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > > >   bytes per callsite, the .data costs can be substantial, and
> > > >   are therefore configurable.
> > > >
> > > > Does changing the default fix things for i915 dmesg ?
> > >
> > > I think we want to mark it BROKEN in addition to make sure no one
> >
> > Ok, I get the distinction now.
> > youre spelling that
> >   depends on BROKEN
> >
> > I have a notional explanation, and a conflating commit:
> >
> > can you eliminate
> > git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
> >
> > as the cause ?
>
> Reverting that doesn't help.
>

thanks for eliminating it.

> >
> > I do need to

Re: [PATCH] drm/msm: Fix return type of mdp4_lvds_connector_mode_valid

2022-10-31 Thread Nathan Chancellor

Hi all,

On Tue, Sep 13, 2022 at 01:55:48PM -0700, Nathan Huckleberry wrote:
> The mode_valid field in drm_connector_helper_funcs is expected to be of
> type:
> enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
>  struct drm_display_mode *mode);
> 
> The mismatched return type breaks forward edge kCFI since the underlying
> function definition does not match the function hook definition.
> 
> The return type of mdp4_lvds_connector_mode_valid should be changed from
> int to enum drm_mode_status.
> 
> Reported-by: Dan Carpenter 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> Cc: l...@lists.linux.dev
> Signed-off-by: Nathan Huckleberry 
> ---
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c 
> b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
> index 7288041dd86a..7444b75c4215 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
> @@ -56,8 +56,9 @@ static int mdp4_lvds_connector_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int mdp4_lvds_connector_mode_valid(struct drm_connector *connector,
> -  struct drm_display_mode *mode)
> +static enum drm_mode_status
> +mdp4_lvds_connector_mode_valid(struct drm_connector *connector,
> +struct drm_display_mode *mode)
>  {
>   struct mdp4_lvds_connector *mdp4_lvds_connector =
>   to_mdp4_lvds_connector(connector);
> -- 
> 2.37.2.789.g6183377224-goog
> 
> 

Did this patch get lost somewhere? I do not see it picked up. It is
needed to avoid a new WIP warning from clang for catching these CFI
failures:

drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c:89:16: error: incompatible 
function pointer types initializing 'enum drm_mode_status (*)(struct 
drm_connector *, struct drm_display_mode *)' with an expression of type 'int 
(struct drm_connector *, struct drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]
.mode_valid = mdp4_lvds_connector_mode_valid,
  ^~
1 error generated.

Cheers,
Nathan

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane





On 10/29/2022 4:23 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, Jessica Zhang wrote:

Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
drm_plane. In addition, add support for setting and getting the values
of these properties.

COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
represents the format of the color fill. Userspace can set enable solid
fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
framebuffer to NULL.


I suppose that COLOR_FILL should override framebuffer rather than 
requiring that FB is set to NULL. In other words, if color_filL_format 
is non-zero, it would make sense to ignore the FB. Then one can use the 
color_fill_format property to quickly switch between filled plane and 
FB-backed one.


Hey Dmitry,

I think this is a good idea -- acked.





Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++
  drivers/gpu/drm/drm_blend.c   | 38 +++
  include/drm/drm_blend.h   |  2 ++
  include/drm/drm_plane.h   | 28 +++
  4 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c

index 79730fa1dd8e..e1664463fca4 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -544,6 +544,10 @@ static int drm_atomic_plane_set_property(struct 
drm_plane *plane,

  state->src_w = val;
  } else if (property == config->prop_src_h) {
  state->src_h = val;
+    } else if (property == plane->color_fill_format_property) {
+    state->color_fill_format = val;
+    } else if (property == plane->color_fill_property) {
+    state->color_fill = val;
  } else if (property == plane->alpha_property) {
  state->alpha = val;
  } else if (property == plane->blend_mode_property) {
@@ -616,6 +620,10 @@ drm_atomic_plane_get_property(struct drm_plane 
*plane,

  *val = state->src_w;
  } else if (property == config->prop_src_h) {
  *val = state->src_h;
+    } else if (property == plane->color_fill_format_property) {
+    *val = state->color_fill_format;
+    } else if (property == plane->color_fill_property) {
+    *val = state->color_fill;
  } else if (property == plane->alpha_property) {
  *val = state->alpha;
  } else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index b4c8cab7158c..b8c2b263fa51 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -616,3 +616,41 @@ int drm_plane_create_blend_mode_property(struct 
drm_plane *plane,

  return 0;
  }
  EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+int drm_plane_create_color_fill_property(struct drm_plane *plane)
+{
+    struct drm_property *prop;
+
+    prop = drm_property_create_range(plane->dev, 0, "color_fill",
+ 0, 0x);
+    if (!prop)
+    return -ENOMEM;
+
+    drm_object_attach_property(&plane->base, prop, 0);
+    plane->color_fill_property = prop;
+
+    if (plane->state)
+    plane->state->color_fill = 0;
+
+    return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_color_fill_property);
+
+int drm_plane_create_color_fill_format_property(struct drm_plane *plane)
+{
+    struct drm_property *prop;
+
+    prop = drm_property_create_range(plane->dev, 0, "color_fill_format",
+ 0, 0x);
+    if (!prop)
+    return -ENOMEM;
+
+    drm_object_attach_property(&plane->base, prop, 0);
+    plane->color_fill_format_property = prop;
+
+    if (plane->state)
+    plane->state->color_fill_format = 0;


Don't you also need to reset these properties in 
__drm_atomic_helper_plane_state_reset() ?


Ah, yes I believe so -- acked.

Thanks,

Jessica Zhang




+
+    return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_color_fill_format_property);
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 88bdfec3bd88..3e96f5e83cce 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -58,4 +58,6 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
    struct drm_atomic_state *state);
  int drm_plane_create_blend_mode_property(struct drm_plane *plane,
   unsigned int supported_modes);
+int drm_plane_create_color_fill_property(struct drm_plane *plane);
+int drm_plane_create_color_fill_format_property(struct drm_plane 
*plane);

  #endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 89ea54652e87..dcbfdb0e1f71 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -116,6 +116,20 @@ struct drm_plane_state {
  /** @src_h: height of visible portion of plane (in 16.16) */
  uint32_t src_h, src_w;
+    /**
+ * @color_fill_format:
+ * Format of the color fill value.
+ */
+    uint32_t color_fill_format;
+
+

[PATCH] drm/i915/guc: Remove excessive line feeds in state dumps

2022-10-31 Thread John . C . Harrison

From: John Harrison 

Some of the GuC state dump messages were adding extra line feeds. When
printing via a DRM printer to dmesg, for example, that messes up the
log formatting as it loses any prefixing from the printer. Given that
the extra line feeds are just in the middle of random bits of GuC
state, there isn't any real need for them. So just remove them
completely.

Signed-off-by: John Harrison 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 ++--
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 27b09ba1d295f..1bcd61bb50f89 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -871,14 +871,14 @@ void intel_guc_load_status(struct intel_guc *guc, struct 
drm_printer *p)
u32 status = intel_uncore_read(uncore, GUC_STATUS);
u32 i;
 
-   drm_printf(p, "\nGuC status 0x%08x:\n", status);
+   drm_printf(p, "GuC status 0x%08x:\n", status);
drm_printf(p, "\tBootrom status = 0x%x\n",
   (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
drm_printf(p, "\tuKernel status = 0x%x\n",
   (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
drm_printf(p, "\tMIA Core status = 0x%x\n",
   (status & GS_MIA_MASK) >> GS_MIA_SHIFT);
-   drm_puts(p, "\nScratch registers:\n");
+   drm_puts(p, "Scratch registers:\n");
for (i = 0; i < 16; i++) {
drm_printf(p, "\t%2d: \t0x%x\n",
   i, intel_uncore_read(uncore, 
SOFT_SCRATCH(i)));
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 4ccb29f9ac55c..4dbdac8002e32 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4901,7 +4901,7 @@ void intel_guc_submission_print_info(struct intel_guc 
*guc,
 
drm_printf(p, "GuC Number Outstanding Submission G2H: %u\n",
   atomic_read(&guc->outstanding_submission_g2h));
-   drm_printf(p, "GuC tasklet count: %u\n\n",
+   drm_printf(p, "GuC tasklet count: %u\n",
   atomic_read(&sched_engine->tasklet.count));
 
spin_lock_irqsave(&sched_engine->lock, flags);
@@ -4949,7 +4949,7 @@ static inline void guc_log_context(struct drm_printer *p,
   atomic_read(&ce->pin_count));
drm_printf(p, "\t\tGuC ID Ref Count: %u\n",
   atomic_read(&ce->guc_id.ref));
-   drm_printf(p, "\t\tSchedule State: 0x%x\n\n",
+   drm_printf(p, "\t\tSchedule State: 0x%x\n",
   ce->guc_state.sched_state);
 }
 
@@ -4978,7 +4978,7 @@ void intel_guc_submission_print_context_info(struct 
intel_guc *guc,
   
READ_ONCE(*ce->parallel.guc.wq_head));
drm_printf(p, "\t\tWQI Tail: %u\n",
   
READ_ONCE(*ce->parallel.guc.wq_tail));
-   drm_printf(p, "\t\tWQI Status: %u\n\n",
+   drm_printf(p, "\t\tWQI Status: %u\n",
   
READ_ONCE(*ce->parallel.guc.wq_status));
}
 
@@ -4986,7 +4986,7 @@ void intel_guc_submission_print_context_info(struct 
intel_guc *guc,
emit_bb_start_parent_no_preempt_mid_batch) {
u8 i;
 
-   drm_printf(p, "\t\tChildren Go: %u\n\n",
+   drm_printf(p, "\t\tChildren Go: %u\n",
   get_children_go_value(ce));
for (i = 0; i < ce->parallel.number_children; 
++i)
drm_printf(p, "\t\tChildren Join: %u\n",
-- 
2.37.3

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

--- Comment #6 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Martin Šušla from comment #5)
> (In reply to Martin Šušla from comment #3)
> > After the message mention in title appears, not even a single interrupt is
> > registered.
> 
> (Valid for both interrupts of the amdgpu driver.)

There are two GPUs in the system.  You appear to be getting at least some
interrupts.

Are you using the dGPU at all or just the APU?  You might try a newer system
bios if there is one available for your system.

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

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

Re: [PATCH] drm/imx: imx-tve: Fix return type of imx_tve_connector_mode_valid

2022-10-31 Thread Nathan Chancellor

On Tue, Sep 13, 2022 at 01:55:44PM -0700, Nathan Huckleberry wrote:
> The mode_valid field in drm_connector_helper_funcs is expected to be of
> type:
> enum drm_mode_status (* mode_valid) (struct drm_connector *connector,
>  struct drm_display_mode *mode);
> 
> The mismatched return type breaks forward edge kCFI since the underlying
> function definition does not match the function hook definition.
> 
> The return type of imx_tve_connector_mode_valid should be changed from
> int to enum drm_mode_status.
> 
> Reported-by: Dan Carpenter 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> Cc: l...@lists.linux.dev
> Signed-off-by: Nathan Huckleberry 

Reviewed-by: Nathan Chancellor 

Can someone pick this up? It resolves a new work-in-progress clang
warning that we would like to turn on for the kernel:

  drivers/gpu/drm/imx/imx-tve.c:320:16: error: incompatible function pointer 
types initializing 'enum drm_mode_status (*)(struct drm_connector *, struct 
drm_display_mode *)' with an expression of type 'int (struct drm_connector *, 
struct drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]
  .mode_valid = imx_tve_connector_mode_valid,
^~~~
  1 error generated.

> ---
>  drivers/gpu/drm/imx/imx-tve.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
> index 6b34fac3f73a..ab4d1c878fda 100644
> --- a/drivers/gpu/drm/imx/imx-tve.c
> +++ b/drivers/gpu/drm/imx/imx-tve.c
> @@ -218,8 +218,9 @@ static int imx_tve_connector_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int imx_tve_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +static enum drm_mode_status
> +imx_tve_connector_mode_valid(struct drm_connector *connector,
> +  struct drm_display_mode *mode)
>  {
>   struct imx_tve *tve = con_to_tve(connector);
>   unsigned long rate;
> -- 
> 2.37.2.789.g6183377224-goog
> 
>

Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3

2022-10-31 Thread Kuogee Hsieh


Hi Dmitry,


Link rate is advertised by sink, but adjusted (reduced the link rate)  
by host during link training.


Therefore should be fine if host did not support HBR3 rate.

It will reduce to lower link rate during link training procedures.

kuogee

On 10/31/2022 11:46 AM, Dmitry Baryshkov wrote:

On 31/10/2022 20:27, Kuogee Hsieh wrote:

An HBR3-capable device shall also support TPS4. Since TPS4 feature
had been implemented already, it is not necessary to limit link
rate at HBR2 (5.4G). This patch remove this limitation to support
HBR3 (8.1G) link rate.


The DP driver supports several platforms including sdm845 and can 
support, if I'm not mistaken, platforms up to msm8998/sdm630/660. 
Could you please confirm that all these SoCs have support for HBR3?


With that fact being confirmed:

Reviewed-by: Dmitry Baryshkov 




Signed-off-by: Kuogee Hsieh 
---
  drivers/gpu/drm/msm/dp/dp_panel.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
b/drivers/gpu/drm/msm/dp/dp_panel.c

index 5149ceb..3344f5a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel 
*dp_panel)

  if (link_info->num_lanes > dp_panel->max_dp_lanes)
  link_info->num_lanes = dp_panel->max_dp_lanes;
  -    /* Limit support upto HBR2 until HBR3 support is added */
-    if (link_info->rate >= 
(drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))

-    link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
-
  drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
  drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
  drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", 
link_info->num_lanes);

Re: [RFC PATCH 2/3] drm: Adjust atomic checks for solid fill color





On 10/29/2022 4:38 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, Jessica Zhang wrote:

Loosen the requirements for atomic and legacy commit so that, in cases
where solid fill planes is enabled (and FB_ID is NULL), the commit can
still go through.

In addition, add framebuffer NULL checks in other areas to account for
FB being NULL when solid fill is enabled.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic.c    | 68 -
  drivers/gpu/drm/drm_atomic_helper.c | 34 +--
  drivers/gpu/drm/drm_plane.c |  8 ++--
  include/drm/drm_atomic_helper.h |  5 ++-
  4 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f197f59f6d99..b576ed221431 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -601,8 +601,10 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  uint32_t num_clips;
  int ret;
-    /* either *both* CRTC and FB must be set, or neither */
-    if (crtc && !fb) {
+    /* When color_fill is disabled,
+ * either *both* CRTC and FB must be set, or neither
+ */
+    if (crtc && !fb && !new_plane_state->color_fill) {


Using !color_fill sounds bad. It would disallow using black as the plane 
fill color. I think, you need to check color_fill_format instead.


Hey Dmitry,

Good point -- acked.



  drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no 
FB\n",

 plane->base.id, plane->name);
  return -EINVAL;
@@ -626,14 +628,16 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  }



Don't we need to check that the color_fill_format is supported too?


Acked.




  /* Check whether this plane supports the fb pixel format. */
-    ret = drm_plane_check_pixel_format(plane, fb->format->format,
-   fb->modifier);
-    if (ret) {
-    drm_dbg_atomic(plane->dev,
-   "[PLANE:%d:%s] invalid pixel format %p4cc, 
modifier 0x%llx\n",

-   plane->base.id, plane->name,
-   &fb->format->format, fb->modifier);
-    return ret;
+    if (fb) {
+    ret = drm_plane_check_pixel_format(plane, fb->format->format,
+   fb->modifier);
+
+    if (ret)
+    drm_dbg_atomic(plane->dev,
+   "[PLANE:%d:%s] invalid pixel format %p4cc, 
modifier 0x%llx\n",

+   plane->base.id, plane->name,
+   &fb->format->format, fb->modifier);
+    return ret;
  }
  /* Give drivers some help against integer overflows */
@@ -649,28 +653,30 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  return -ERANGE;
  }
-    fb_width = fb->width << 16;
-    fb_height = fb->height << 16;
+    if (fb) {
+    fb_width = fb->width << 16;
+    fb_height = fb->height << 16;
-    /* Make sure source coordinates are inside the fb. */
-    if (new_plane_state->src_w > fb_width ||
-    new_plane_state->src_x > fb_width - new_plane_state->src_w ||
-    new_plane_state->src_h > fb_height ||
-    new_plane_state->src_y > fb_height - new_plane_state->src_h) {
-    drm_dbg_atomic(plane->dev,
-   "[PLANE:%d:%s] invalid source coordinates "
-   "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
-   plane->base.id, plane->name,
-   new_plane_state->src_w >> 16,
-   ((new_plane_state->src_w & 0x) * 15625) >> 10,
-   new_plane_state->src_h >> 16,
-   ((new_plane_state->src_h & 0x) * 15625) >> 10,
-   new_plane_state->src_x >> 16,
-   ((new_plane_state->src_x & 0x) * 15625) >> 10,
-   new_plane_state->src_y >> 16,
-   ((new_plane_state->src_y & 0x) * 15625) >> 10,
-   fb->width, fb->height);
-    return -ENOSPC;
+    /* Make sure source coordinates are inside the fb. */
+    if (new_plane_state->src_w > fb_width ||
+    new_plane_state->src_x > fb_width - 
new_plane_state->src_w ||

+    new_plane_state->src_h > fb_height ||
+    new_plane_state->src_y > fb_height - 
new_plane_state->src_h) {

+    drm_dbg_atomic(plane->dev,
+   "[PLANE:%d:%s] invalid source coordinates "
+   "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
+   plane->base.id, plane->name,
+   new_plane_state->src_w >> 16,
+   ((new_plane_state->src_w & 0x) * 15625) >> 
10,

+   new_plane_state->src_h >> 16,
+   ((new_plane_state->src_h & 0x) * 15625) >> 
10,

+   new_plane_state->src_x >> 16,
+   ((new_plane_state->src_x & 0x

Re: [PATCH 02/11] ARM: sa1100: remove unused board files

2022-10-31 Thread Linus Walleij

On Fri, Oct 21, 2022 at 5:55 PM Arnd Bergmann  wrote:

> From: Arnd Bergmann 
>
> The Cerf, H3100, Badge4, Hackkit, LART, NanoEngine, PLEB, Shannon and
> Simpad machines were all marked as unused as there are no known users
> left. Remove all of these, along with references to them in defconfig
> files and drivers.
>
> Four machines remain now: Assabet, Collie (Zaurus SL5500), iPAQ H3600
> and Jornada 720, each of which had one person still using them, with
> Collie also being supported in Qemu.
>
> Cc: Peter Chubb 
> Cc: Stefan Eletzhofer 
> Signed-off-by: Arnd Bergmann 

Acked-by: Linus Walleij 

Yours,
Linus Walleij

Re: [PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3


On 31/10/2022 20:27, Kuogee Hsieh wrote:

An HBR3-capable device shall also support TPS4. Since TPS4 feature
had been implemented already, it is not necessary to limit link
rate at HBR2 (5.4G). This patch remove this limitation to support
HBR3 (8.1G) link rate.


The DP driver supports several platforms including sdm845 and can 
support, if I'm not mistaken, platforms up to msm8998/sdm630/660. Could 
you please confirm that all these SoCs have support for HBR3?


With that fact being confirmed:

Reviewed-by: Dmitry Baryshkov 




Signed-off-by: Kuogee Hsieh 
---
  drivers/gpu/drm/msm/dp/dp_panel.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149ceb..3344f5a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
if (link_info->num_lanes > dp_panel->max_dp_lanes)
link_info->num_lanes = dp_panel->max_dp_lanes;
  
-	/* Limit support upto HBR2 until HBR3 support is added */

-   if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
-   link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
-
drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", link_info->num_lanes);


--
With best wishes
Dmitry

Re: [PATCH 1/5] drm/i915/mtl: add initial definitions for GSC CS

2022-10-31 Thread Matt Roper

On Mon, Oct 31, 2022 at 09:43:33AM -0700, Ceraolo Spurio, Daniele wrote:
> 
> 
> On 10/31/2022 9:26 AM, Matt Roper wrote:
> > On Thu, Oct 27, 2022 at 03:15:50PM -0700, Daniele Ceraolo Spurio wrote:
> > > Starting on MTL, the GSC is no longer managed with direct MMIO access,
> > > but we instead have a dedicated command streamer for it. As a first step
> > > for adding support for this CS, add the required definitions.
> > > Note that, although it is now a CS, the GSC retains its old
> > > class:instance value (OTHER_CLASS instance 6)
> > > 
> > > Signed-off-by: Daniele Ceraolo Spurio 
> > > Cc: Matt Roper 
> > Now that we have an OTHER_CLASS engine, I think we also need to deal
> > with the class -> reg mapping table in mmio_invalidate_full().  I think
> > the register we want is 0xCF04?
> 
> I missed that. Looks like the the situation is a bit more complex than just
> adding the new register, because on pre-MTL platforms CF04 is the compute
> class invalidation register. On MTL as you said CF04 is marked as the GSC CS
> invalidation register, but I can't find the compute one. Do you know if it
> re-uses the render one or something like that?
> Given that there seem to be non-GSC related changes as well, IMO this should
> probably be a separate patch to specifically handle the TLB inval changes on
> MTL.

Yeah, makes sense; we can follow up with separate patches for this.

+Cc Fei since he's done a lot of work on TLB invalidation and may know
what happens to compute class invalidation on MTL when the GSC takes
over that register.


Matt

> 
> Daniele
> 
> > 
> > Matt
> > 
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_engine_cs.c| 8 
> > >   drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
> > >   drivers/gpu/drm/i915/gt/intel_engine_user.c  | 1 +
> > >   drivers/gpu/drm/i915/i915_reg.h  | 1 +
> > >   4 files changed, 11 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
> > > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > index 3b7d750ad054..e0fbfac03979 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > @@ -244,6 +244,13 @@ static const struct engine_info intel_engines[] = {
> > >   { .graphics_ver = 12, .base = 
> > > GEN12_COMPUTE3_RING_BASE }
> > >   }
> > >   },
> > > + [GSC0] = {
> > > + .class = OTHER_CLASS,
> > > + .instance = OTHER_GSC_INSTANCE,
> > > + .mmio_bases = {
> > > + { .graphics_ver = 12, .base = MTL_GSC_RING_BASE }
> > > + }
> > > + },
> > >   };
> > >   /**
> > > @@ -324,6 +331,7 @@ u32 intel_engine_context_size(struct intel_gt *gt, u8 
> > > class)
> > >   case VIDEO_DECODE_CLASS:
> > >   case VIDEO_ENHANCEMENT_CLASS:
> > >   case COPY_ENGINE_CLASS:
> > > + case OTHER_CLASS:
> > >   if (GRAPHICS_VER(gt->i915) < 8)
> > >   return 0;
> > >   return GEN8_LR_CONTEXT_OTHER_SIZE;
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
> > > b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > index 6b5d4ea22b67..4fd54fb8810f 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > @@ -136,6 +136,7 @@ enum intel_engine_id {
> > >   CCS2,
> > >   CCS3,
> > >   #define _CCS(n) (CCS0 + (n))
> > > + GSC0,
> > >   I915_NUM_ENGINES
> > >   #define INVALID_ENGINE ((enum intel_engine_id)-1)
> > >   };
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c 
> > > b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > index 46a174f8aa00..79312b734690 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> > > @@ -140,6 +140,7 @@ const char *intel_engine_class_repr(u8 class)
> > >   [COPY_ENGINE_CLASS] = "bcs",
> > >   [VIDEO_DECODE_CLASS] = "vcs",
> > >   [VIDEO_ENHANCEMENT_CLASS] = "vecs",
> > > + [OTHER_CLASS] = "other",
> > >   [COMPUTE_CLASS] = "ccs",
> > >   };
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 1c0da50c0dc7..d056c3117ef2 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -970,6 +970,7 @@
> > >   #define GEN11_VEBOX2_RING_BASE  0x1d8000
> > >   #define XEHP_VEBOX3_RING_BASE   0x1e8000
> > >   #define XEHP_VEBOX4_RING_BASE   0x1f8000
> > > +#define MTL_GSC_RING_BASE0x11a000
> > >   #define GEN12_COMPUTE0_RING_BASE0x1a000
> > >   #define GEN12_COMPUTE1_RING_BASE0x1c000
> > >   #define GEN12_COMPUTE2_RING_BASE0x1e000
> > > -- 
> > > 2.37.3
> > > 
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Don't deadlock busyness stats vs reset

2022-10-31 Thread John Harrison


On 10/31/2022 05:51, Tvrtko Ursulin wrote:

On 31/10/2022 10:09, Tvrtko Ursulin wrote:

On 28/10/2022 20:46, john.c.harri...@intel.com wrote:

From: John Harrison 

The engine busyness stats has a worker function to do things like
64bit extend the 32bit hardware counters. The GuC's reset prepare
function flushes out this worker function to ensure no corruption
happens during the reset. Unforunately, the worker function has an
infinite wait for active resets to finish before doing its work. Thus
a deadlock would occur if the worker function had actually started
just as the reset starts.

Update the worker to abort if a reset is in progress rather than
waiting for it to complete. It will still acquire the reset lock in
the case where a reset was not already in progress. So the processing
is still safe from corruption, but the deadlock can no longer occur.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/intel_reset.c | 15 
++-

  drivers/gpu/drm/i915/gt/intel_reset.h |  1 +
  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 --
  3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
b/drivers/gpu/drm/i915/gt/intel_reset.c

index 3159df6cdd492..2f48c6e4420ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1407,7 +1407,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
  intel_runtime_pm_put(gt->uncore->rpm, wakeref);
  }
-int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu)
+static int _intel_gt_reset_trylock(struct intel_gt *gt, int *srcu, 
bool retry)

  {
  might_lock(>->reset.backoff_srcu);
  might_sleep();
@@ -1416,6 +1416,9 @@ int intel_gt_reset_trylock(struct intel_gt 
*gt, int *srcu)

  while (test_bit(I915_RESET_BACKOFF, >->reset.flags)) {
  rcu_read_unlock();
+    if (!retry)
+    return -EBUSY;
+
  if (wait_event_interruptible(gt->reset.queue,
   !test_bit(I915_RESET_BACKOFF,
 >->reset.flags)))


Would it be more obvious to rename the existing semantics to 
intel_gt_reset_interruptible(), while the flavour you add in this 
patch truly is trylock? I am not sure, since it's all a bit special, 
but trylock sure feels confusing if it can sleep forever...
To me, it would seem totally more obvious to have a function called 
'trylock' not wait forever until it can manage to acquire the lock. 
However, according to '2caffbf1176256 drm/i915: Revoke mmaps and prevent 
access to fence registers across reset', the current behaviour is 
exactly how the code was originally written and intended. It hasn't just 
mutated into some confused evolution a thousand patches later. So I 
figure there is some subtle but important reason why it was named how it 
is named and yet does what it does. Therefore it seemed safest to not 
change it unnecessarily.




Oh and might_sleep() shouldn't be there with the trylock version - I 
mean any flavour of the real trylock.
You mean if the code is split into two completely separate functions? Or 
do you just mean to wrap the might_sleep() call with 'if(!retry)'?


And just to be totally clear, the unconditional call to rcu_read_lock() 
is not something that can sleep? One doesn't need a might_sleep() before 
doing that lock?


John.




Regards,

Tvrtko

Re: [PATCH v2] drm/format-helper: Only advertise supported formats for conversion

2022-10-31 Thread Justin Forbes

On Mon, Oct 31, 2022 at 11:52 AM Hector Martin  wrote:
>
> On 01/11/2022 01.15, Justin Forbes wrote:
> > On Thu, Oct 27, 2022 at 8:57 AM Hector Martin  wrote:
> >>
> >> drm_fb_build_fourcc_list() currently returns all emulated formats
> >> unconditionally as long as the native format is among them, even though
> >> not all combinations have conversion helpers. Although the list is
> >> arguably provided to userspace in precedence order, userspace can pick
> >> something out-of-order (and thus break when it shouldn't), or simply
> >> only support a format that is unsupported (and thus think it can work,
> >> which results in the appearance of a hang as FB blits fail later on,
> >> instead of the initialization error you'd expect in this case).
> >>
> >> Add checks to filter the list of emulated formats to only those
> >> supported for conversion to the native format. This presumes that there
> >> is a single native format (only the first is checked, if there are
> >> multiple). Refactoring this API to drop the native list or support it
> >> properly (by returning the appropriate emulated->native mapping table)
> >> is left for a future patch.
> >>
> >> The simpledrm driver is left as-is with a full table of emulated
> >> formats. This keeps all currently working conversions available and
> >> drops all the broken ones (i.e. this a strict bugfix patch, adding no
> >> new supported formats nor removing any actually working ones). In order
> >> to avoid proliferation of emulated formats, future drivers should
> >> advertise only XRGB as the sole emulated format (since some
> >> userspace assumes its presence).
> >>
> >> This fixes a real user regression where the ?RGB2101010 support commit
> >> started advertising it unconditionally where not supported, and KWin
> >> decided to start to use it over the native format and broke, but also
> >> the fixes the spurious RGB565/RGB888 formats which have been wrongly
> >> unconditionally advertised since the dawn of simpledrm.
> >>
> >> Fixes: 6ea966fca084 ("drm/simpledrm: Add [AX]RGB210101
> >
> >
> >> Cc: sta...@vger.kernel.org
> >> Signed-off-by: Hector Martin 
> >
> > There is a CC for stable on here, but this patch does not apply in any
> > way on 6.0 or older kernels as the fourcc bits and considerable churn
> > came in with the 6.1 merge window.  You don't happen to have a
> > backport of this to 6.0 do you?
>
> v1 is probably closer to such a backport, and I offered to figure it out
> on Matrix but I heard you're already working on it ;)

i am, but I didn't want to be, so I thought I would ask.

Justin

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

--- Comment #5 from Martin Šušla (ask4supp...@email.cz) ---
(In reply to Martin Šušla from comment #3)
> After the message mention in title appears, not even a single interrupt is
> registered.

(Valid for both interrupts of the amdgpu driver.)

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

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

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

--- Comment #4 from Martin Šušla (ask4supp...@email.cz) ---
Created attachment 303110
  --> https://bugzilla.kernel.org/attachment.cgi?id=303110&action=edit
Kernel log interlaced with contents of /proc/interrupts polled every second

#! /bin/sh

print() {
printf "$1" ; seq -s" " $(( $(stty size < $t | sed "s/^/(/; s/ / - 1 )
* /") - ${#1} )) | sed s/[0-9]//g
}
t="$(readlink /proc/self/fd/0)"

d="$(env LANG=C udisksctl mount -b /dev/disk/by-uuid/$1 -o sync 2> /dev/null |
sed "s/^Mounted .* at //g; s/\.$//g")"
[ -d "$d" ] && f=oflag=direct || d="${0%/*}" f=oflag=direct

(sudo dmesg -w & while sleep 1 ; do cat /proc/interrupts ; done) | sudo dd
of="$d/${0##*/}.log" $f &
i=0
seq 28 15 | while read N ; do
print $N
timeout 3 env DISPLAY=:0 plasma-systemmonitor > /dev/null 2>&1
n=$N ; while [ 0 -lt $n ] ; do
sleep 1
n=$(( $n - 1 ))
i=$(( $i ^ 1 ))
[ "$i" = 1 ] && printf "\33[30m\33[47m" || printf
"\33[37m\33[40m"
print $n
done
done

echo END!
exit

# This script was used to generate it

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

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

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

--- Comment #3 from Martin Šušla (ask4supp...@email.cz) ---
After the message mention in title appears, not even a single interrupt is
registered.

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

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

Re: [PATCH] drm/i915/mtl: Media GT and Render GT share common GGTT

2022-10-31 Thread Matt Roper

On Mon, Oct 31, 2022 at 06:01:11PM +0530, Aravind Iddamsetty wrote:
> On XE_LPM+ platforms the media engines are carved out into a separate
> GT but have a common GGTMMADR address range which essentially makes
> the GGTT address space to be shared between media and render GT.

While this is all true, I feel like this description is lacking a bit of
explanation for why/how that translates into the code changes below.
For example you should elaborate on the areas this impacts, such as the
need to invalidate both GTs' TLBs, retire requests for both GTs, etc.

Also, the movement of the PAT setup should be noted and explained as
well since it differs from how you approached the other changes here.

> 
> BSPEC: 63834
> 
> Cc: Matt Roper 
> Signed-off-by: Aravind Iddamsetty 
> ---
>  drivers/gpu/drm/i915/gt/intel_ggtt.c  | 49 +++---
>  drivers/gpu/drm/i915/gt/intel_gt.c| 15 +-
>  drivers/gpu/drm/i915/gt/intel_gt_types.h  |  3 ++
>  drivers/gpu/drm/i915/gt/intel_gtt.h   |  3 ++
>  drivers/gpu/drm/i915/i915_driver.c| 19 +--
>  drivers/gpu/drm/i915/i915_gem_evict.c | 63 +--
>  drivers/gpu/drm/i915/i915_vma.c   |  5 +-
>  drivers/gpu/drm/i915/selftests/i915_gem.c |  2 +
>  drivers/gpu/drm/i915/selftests/mock_gtt.c |  1 +
>  9 files changed, 115 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 2518cebbf931..f5c2f3c58627 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -196,10 +196,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
>  
>  void i915_ggtt_suspend(struct i915_ggtt *ggtt)
>  {
> + struct intel_gt *gt;
> +
>   i915_ggtt_suspend_vm(&ggtt->vm);
>   ggtt->invalidate(ggtt);
>  
> - intel_gt_check_and_clear_faults(ggtt->vm.gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_check_and_clear_faults(gt);
>  }
>  
>  void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
> @@ -214,27 +217,36 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
>  
>  static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
>  {
> - struct intel_uncore *uncore = ggtt->vm.gt->uncore;
> + struct intel_uncore *uncore;
> + struct intel_gt *gt;
>  
> - /*
> -  * Note that as an uncached mmio write, this will flush the
> -  * WCB of the writes into the GGTT before it triggers the invalidate.
> -  */
> - intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
> + uncore = gt->uncore;
> + /*
> +  * Note that as an uncached mmio write, this will flush the
> +  * WCB of the writes into the GGTT before it triggers the 
> invalidate.
> +  */
> + intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, 
> GFX_FLSH_CNTL_EN);

This isn't a GT register, so writing it for each GT doesn't do anything
different than just writing it once.  But actually it doesn't look like
this is even a register we should be writing to anymore since Xe_HP.
The GFX_FLSH_CNTL register no longer lives here.

> + }
>  }
>  
>  static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
>  {
> - struct intel_uncore *uncore = ggtt->vm.gt->uncore;
>   struct drm_i915_private *i915 = ggtt->vm.i915;
>  
>   gen8_ggtt_invalidate(ggtt);
>  
> - if (GRAPHICS_VER(i915) >= 12)
> - intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR,
> -   GEN12_GUC_TLB_INV_CR_INVALIDATE);
> - else
> - intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> + if (GRAPHICS_VER(i915) >= 12) {
> + struct intel_gt *gt;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_uncore_write_fw(gt->uncore,
> +   GEN12_GUC_TLB_INV_CR,
> +   GEN12_GUC_TLB_INV_CR_INVALIDATE);
> + } else {
> + intel_uncore_write_fw(ggtt->vm.gt->uncore,
> +   GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> + }
>  }
>  
>  u64 gen8_ggtt_pte_encode(dma_addr_t addr,
> @@ -986,8 +998,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  
>   ggtt->vm.pte_encode = gen8_ggtt_pte_encode;
>  
> - setup_private_pat(ggtt->vm.gt);
> -
>   return ggtt_probe_common(ggtt, size);
>  }
>  
> @@ -1186,7 +1196,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct 
> intel_gt *gt)
>   (u64)ggtt->mappable_end >> 20);
>   drm_dbg(&i915->drm, "DSM size = %lluM\n",
>   (u64)resource_size(&intel_graphics_stolen_res) >> 20);
> -
> + INIT_LIST_HEAD(&ggtt->gt_list);
>   return 0;
>  }
>  
> @@ -1296,9 +1306,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
>  
>  void i915_ggtt_resume(st

Re: 6.1-rc: names of video outputs changed?

2022-10-31 Thread Ville Syrjälä

On Mon, Oct 31, 2022 at 06:19:52PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > I used to be able to do:
> > > 
> > > pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
> > > warning: output HDMI1 not found; ignoring
> > > pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
> > > warning: output VGA1 not found; ignoring
> > > 
> > > ...but now I have to do:
> > > 
> > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > > xrandr: cannot find crtc for output VGA-1
> > > pavel@duo:~$ xrandr --output LVDS-1 --off
> > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > > 
> > > Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
> > > so. Who did it and why? Hardware is thinkpad x220, and this breaks my
> > > scripts :-(.
> > 
> > Are you sure you didn't just switch from intel ddx to modesetting ddx?
> 
> How do I tell?

You can read through your Xorg log file.

Or you can do something like this:
lsof -p $(pidof X) | grep _drv.so

-- 
Ville Syrjälä
Intel

[PATCH] drm/msm/dp: remove limitation of link rate at 5.4G to support HBR3

2022-10-31 Thread Kuogee Hsieh

An HBR3-capable device shall also support TPS4. Since TPS4 feature
had been implemented already, it is not necessary to limit link
rate at HBR2 (5.4G). This patch remove this limitation to support
HBR3 (8.1G) link rate.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149ceb..3344f5a 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -78,10 +78,6 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
if (link_info->num_lanes > dp_panel->max_dp_lanes)
link_info->num_lanes = dp_panel->max_dp_lanes;
 
-   /* Limit support upto HBR2 until HBR3 support is added */
-   if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
-   link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
-
drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", link_info->num_lanes);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: 6.1-rc: names of video outputs changed?

On Mon, Oct 31, 2022 at 1:19 PM Pavel Machek  wrote:
>
> Hi!
>
> > > I used to be able to do:
> > >
> > > pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
> > > warning: output HDMI1 not found; ignoring
> > > pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
> > > warning: output VGA1 not found; ignoring
> > >
> > > ...but now I have to do:
> > >
> > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > > xrandr: cannot find crtc for output VGA-1
> > > pavel@duo:~$ xrandr --output LVDS-1 --off
> > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > >
> > > Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
> > > so. Who did it and why? Hardware is thinkpad x220, and this breaks my
> > > scripts :-(.
> >
> > Are you sure you didn't just switch from intel ddx to modesetting ddx?
>
> How do I tell?

Check your xorg log.

Alex


>
> I don't think I did such changes recently. It is Debian 10.13 system
> (rather old) so I don't think it did update for me.
>
> Thanks and best regards,
> Pavel
> --
> People of Russia, stop Putin before his war on Ukraine escalates.

[PATCH] drm/i915/hwmon: Don't use FIELD_PREP

2022-10-31 Thread Ashutosh Dixit

FIELD_PREP and REG_FIELD_PREP have checks requiring a compile time constant
mask. When the mask comes in as the argument of a function these checks can
can fail depending on the compiler (gcc vs clang), optimization level,
etc. Use a simpler version of FIELD_PREP which skips these checks. The
checks are not needed because the mask is formed using REG_GENMASK (so is
actually a compile time constant).

v2: Split REG_FIELD_PREP into a macro with checks and one without and use
the one without checks in i915_hwmon.c (Gwan-gyeong Mun)

Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7354
Signed-off-by: Ashutosh Dixit 
---
 drivers/gpu/drm/i915/i915_hwmon.c|  2 +-
 drivers/gpu/drm/i915/i915_reg_defs.h | 17 +++--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c 
b/drivers/gpu/drm/i915/i915_hwmon.c
index 9e97814930254..ae435b035229a 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -112,7 +112,7 @@ hwm_field_scale_and_write(struct hwm_drvdata *ddat, 
i915_reg_t rgadr,
nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
 
bits_to_clear = field_msk;
-   bits_to_set = FIELD_PREP(field_msk, nval);
+   bits_to_set = __REG_FIELD_PREP(field_msk, nval);
 
hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
bits_to_clear, bits_to_set);
diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h 
b/drivers/gpu/drm/i915/i915_reg_defs.h
index f1859046a9c48..dddacc8d48928 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -67,12 +67,17 @@
  *
  * @return: @__val masked and shifted into the field defined by @__mask.
  */
-#define REG_FIELD_PREP(__mask, __val)  
\
-   ((u32)typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + 
\
-  BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
-  BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + 
\
-  BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << 
__bf_shf(__mask + \
-  BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), 
(~((__mask) >> __bf_shf(__mask)) & (__val)), 0
+#define __REG_FIELD_PREP_CHK(__mask, __val) \
+   (BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
+BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
+BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << 
__bf_shf(__mask + \
+BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), 
(~((__mask) >> __bf_shf(__mask)) & (__val)), 0)))
+
+#define __REG_FIELD_PREP(__mask, __val) \
+   ((u32)typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask
+
+#define REG_FIELD_PREP(__mask, __val) \
+   (__REG_FIELD_PREP(__mask, __val) + __REG_FIELD_PREP_CHK(__mask, __val))
 
 /**
  * REG_FIELD_GET() - Extract a u32 bitfield value
-- 
2.38.0

Re: 6.1-rc: names of video outputs changed?

2022-10-31 Thread Pavel Machek

Hi!

> > I used to be able to do:
> > 
> > pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
> > warning: output HDMI1 not found; ignoring
> > pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
> > warning: output VGA1 not found; ignoring
> > 
> > ...but now I have to do:
> > 
> > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > xrandr: cannot find crtc for output VGA-1
> > pavel@duo:~$ xrandr --output LVDS-1 --off
> > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> > 
> > Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
> > so. Who did it and why? Hardware is thinkpad x220, and this breaks my
> > scripts :-(.
> 
> Are you sure you didn't just switch from intel ddx to modesetting ddx?

How do I tell?

I don't think I did such changes recently. It is Debian 10.13 system
(rather old) so I don't think it did update for me.

Thanks and best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

[PATCH] drm/rockchip: vop: Quiet always-warning AFBC log

2022-10-31 Thread Brian Norris

The downstream code from which this was derived didn't ever run through
this 'switch' block with non-AFBC formats, but the upstream code does --
we use this function to probe whether a given format is supported.

Demote the warning to eliminate this sort of warning seen on every
boot:

  [drm] unsupported AFBC format[3231564e]

And make it warn more than once, because if we *actually* care to see
what formats we're probing/rejecting and for what reasons, we probably
care about more than just the first message.

Drop the comment, because one of the two *is* commonly reachable.

And lastly, drop the unreachable return; we'd do better to let the
compiler complain if we start hitting this unexpectedly.

Signed-off-by: Brian Norris 
---

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa1f4ee6d195..aab77eb6caa3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -316,13 +316,10 @@ static int vop_convert_afbc_format(uint32_t format)
case DRM_FORMAT_RGB565:
case DRM_FORMAT_BGR565:
return AFBC_FMT_RGB565;
-   /* either of the below should not be reachable */
default:
-   DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
+   DRM_DEBUG_KMS("unsupported AFBC format[%08x]\n", format);
return -EINVAL;
}
-
-   return -EINVAL;
 }
 
 static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
-- 
2.38.1.273.g43a17bfeac-goog

Re: 6.1-rc: names of video outputs changed?

2022-10-31 Thread Ville Syrjälä

On Mon, Oct 31, 2022 at 05:58:34PM +0100, Pavel Machek wrote:
> Hi!
> 
> I used to be able to do:
> 
> pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
> warning: output HDMI1 not found; ignoring
> pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
> warning: output VGA1 not found; ignoring
> 
> ...but now I have to do:
> 
> pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> xrandr: cannot find crtc for output VGA-1
> pavel@duo:~$ xrandr --output LVDS-1 --off
> pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> 
> Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
> so. Who did it and why? Hardware is thinkpad x220, and this breaks my
> scripts :-(.

Are you sure you didn't just switch from intel ddx to modesetting ddx?

-- 
Ville Syrjälä
Intel

Re: 6.1-rc: names of video outputs changed?

Did you change which xorg DDX you are using?  E.g., between
xf86-video-modesetting and a xf86-video-[i915/radeon/amdgpu/etc.]?
They may have different naming conventions.

Alex

On Mon, Oct 31, 2022 at 1:04 PM Pavel Machek  wrote:
>
> Hi!
>
> I used to be able to do:
>
> pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
> warning: output HDMI1 not found; ignoring
> pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
> warning: output VGA1 not found; ignoring
>
> ...but now I have to do:
>
> pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
> xrandr: cannot find crtc for output VGA-1
> pavel@duo:~$ xrandr --output LVDS-1 --off
> pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
>
> Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
> so. Who did it and why? Hardware is thinkpad x220, and this breaks my
> scripts :-(.
>
> Best regards,
> Pavel
> --
> People of Russia, stop Putin before his war on Ukraine escalates.

6.1-rc: names of video outputs changed?

2022-10-31 Thread Pavel Machek

Hi!

I used to be able to do:

pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary
warning: output HDMI1 not found; ignoring
pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1
warning: output VGA1 not found; ignoring

...but now I have to do:

pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1
xrandr: cannot find crtc for output VGA-1
pavel@duo:~$ xrandr --output LVDS-1 --off
pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1

Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or
so. Who did it and why? Hardware is thinkpad x220, and this breaks my
scripts :-(.

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/format-helper: Only advertise supported formats for conversion

2022-10-31 Thread Hector Martin

On 01/11/2022 01.15, Justin Forbes wrote:
> On Thu, Oct 27, 2022 at 8:57 AM Hector Martin  wrote:
>>
>> drm_fb_build_fourcc_list() currently returns all emulated formats
>> unconditionally as long as the native format is among them, even though
>> not all combinations have conversion helpers. Although the list is
>> arguably provided to userspace in precedence order, userspace can pick
>> something out-of-order (and thus break when it shouldn't), or simply
>> only support a format that is unsupported (and thus think it can work,
>> which results in the appearance of a hang as FB blits fail later on,
>> instead of the initialization error you'd expect in this case).
>>
>> Add checks to filter the list of emulated formats to only those
>> supported for conversion to the native format. This presumes that there
>> is a single native format (only the first is checked, if there are
>> multiple). Refactoring this API to drop the native list or support it
>> properly (by returning the appropriate emulated->native mapping table)
>> is left for a future patch.
>>
>> The simpledrm driver is left as-is with a full table of emulated
>> formats. This keeps all currently working conversions available and
>> drops all the broken ones (i.e. this a strict bugfix patch, adding no
>> new supported formats nor removing any actually working ones). In order
>> to avoid proliferation of emulated formats, future drivers should
>> advertise only XRGB as the sole emulated format (since some
>> userspace assumes its presence).
>>
>> This fixes a real user regression where the ?RGB2101010 support commit
>> started advertising it unconditionally where not supported, and KWin
>> decided to start to use it over the native format and broke, but also
>> the fixes the spurious RGB565/RGB888 formats which have been wrongly
>> unconditionally advertised since the dawn of simpledrm.
>>
>> Fixes: 6ea966fca084 ("drm/simpledrm: Add [AX]RGB210101
> 
> 
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Hector Martin 
> 
> There is a CC for stable on here, but this patch does not apply in any
> way on 6.0 or older kernels as the fourcc bits and considerable churn
> came in with the 6.1 merge window.  You don't happen to have a
> backport of this to 6.0 do you?

v1 is probably closer to such a backport, and I offered to figure it out
on Matrix but I heard you're already working on it ;)

- Hector

Re: [PATCH 1/5] drm/i915/mtl: add initial definitions for GSC CS

2022-10-31 Thread Ceraolo Spurio, Daniele





On 10/31/2022 9:26 AM, Matt Roper wrote:

On Thu, Oct 27, 2022 at 03:15:50PM -0700, Daniele Ceraolo Spurio wrote:

Starting on MTL, the GSC is no longer managed with direct MMIO access,
but we instead have a dedicated command streamer for it. As a first step
for adding support for this CS, add the required definitions.
Note that, although it is now a CS, the GSC retains its old
class:instance value (OTHER_CLASS instance 6)

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Matt Roper 

Now that we have an OTHER_CLASS engine, I think we also need to deal
with the class -> reg mapping table in mmio_invalidate_full().  I think
the register we want is 0xCF04?


I missed that. Looks like the the situation is a bit more complex than 
just adding the new register, because on pre-MTL platforms CF04 is the 
compute class invalidation register. On MTL as you said CF04 is marked 
as the GSC CS invalidation register, but I can't find the compute one. 
Do you know if it re-uses the render one or something like that?
Given that there seem to be non-GSC related changes as well, IMO this 
should probably be a separate patch to specifically handle the TLB inval 
changes on MTL.


Daniele



Matt


---
  drivers/gpu/drm/i915/gt/intel_engine_cs.c| 8 
  drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
  drivers/gpu/drm/i915/gt/intel_engine_user.c  | 1 +
  drivers/gpu/drm/i915/i915_reg.h  | 1 +
  4 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 3b7d750ad054..e0fbfac03979 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -244,6 +244,13 @@ static const struct engine_info intel_engines[] = {
{ .graphics_ver = 12, .base = GEN12_COMPUTE3_RING_BASE }
}
},
+   [GSC0] = {
+   .class = OTHER_CLASS,
+   .instance = OTHER_GSC_INSTANCE,
+   .mmio_bases = {
+   { .graphics_ver = 12, .base = MTL_GSC_RING_BASE }
+   }
+   },
  };
  
  /**

@@ -324,6 +331,7 @@ u32 intel_engine_context_size(struct intel_gt *gt, u8 class)
case VIDEO_DECODE_CLASS:
case VIDEO_ENHANCEMENT_CLASS:
case COPY_ENGINE_CLASS:
+   case OTHER_CLASS:
if (GRAPHICS_VER(gt->i915) < 8)
return 0;
return GEN8_LR_CONTEXT_OTHER_SIZE;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 6b5d4ea22b67..4fd54fb8810f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -136,6 +136,7 @@ enum intel_engine_id {
CCS2,
CCS3,
  #define _CCS(n) (CCS0 + (n))
+   GSC0,
I915_NUM_ENGINES
  #define INVALID_ENGINE ((enum intel_engine_id)-1)
  };
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c 
b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 46a174f8aa00..79312b734690 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -140,6 +140,7 @@ const char *intel_engine_class_repr(u8 class)
[COPY_ENGINE_CLASS] = "bcs",
[VIDEO_DECODE_CLASS] = "vcs",
[VIDEO_ENHANCEMENT_CLASS] = "vecs",
+   [OTHER_CLASS] = "other",
[COMPUTE_CLASS] = "ccs",
};
  
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h

index 1c0da50c0dc7..d056c3117ef2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -970,6 +970,7 @@
  #define GEN11_VEBOX2_RING_BASE0x1d8000
  #define XEHP_VEBOX3_RING_BASE 0x1e8000
  #define XEHP_VEBOX4_RING_BASE 0x1f8000
+#define MTL_GSC_RING_BASE  0x11a000
  #define GEN12_COMPUTE0_RING_BASE  0x1a000
  #define GEN12_COMPUTE1_RING_BASE  0x1c000
  #define GEN12_COMPUTE2_RING_BASE  0x1e000
--
2.37.3

[PATCH] drm/nouveau: Add support to control backlight using bl_power for nva3.

2022-10-31 Thread antoniospg

Summary:

* Add support to turn on/off backlight when changing values in bl_power
  file. This is achieved by using function backlight_get_brightness()
  in nva3_set_intensity to get current brightness.

Test plan:

* Turn off:
echo 1 > /sys/class/backlight/nv_backlight/bl_power

* Turn on:
echo 0 > /sys/class/backlight/nv_backlight/bl_power

Signed-off-by: antoniospg 
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index a2141d3d9b1d..5c82f5189b79 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -263,7 +263,11 @@ nva3_set_intensity(struct backlight_device *bd)
u32 div, val;
 
div = nvif_rd32(device, NV50_PDISP_SOR_PWM_DIV(or));
-   val = (bd->props.brightness * div) / 100;
+
+   val = backlight_get_brightness(bd);
+   if (val)
+   val = (val * div) / 100;
+
if (div) {
nvif_wr32(device, NV50_PDISP_SOR_PWM_CTL(or),
  val |
-- 
2.25.1

Re: [PATCH 1/5] drm/i915/mtl: add initial definitions for GSC CS

2022-10-31 Thread Matt Roper

On Thu, Oct 27, 2022 at 03:15:50PM -0700, Daniele Ceraolo Spurio wrote:
> Starting on MTL, the GSC is no longer managed with direct MMIO access,
> but we instead have a dedicated command streamer for it. As a first step
> for adding support for this CS, add the required definitions.
> Note that, although it is now a CS, the GSC retains its old
> class:instance value (OTHER_CLASS instance 6)
> 
> Signed-off-by: Daniele Ceraolo Spurio 
> Cc: Matt Roper 

Now that we have an OTHER_CLASS engine, I think we also need to deal
with the class -> reg mapping table in mmio_invalidate_full().  I think
the register we want is 0xCF04?

Matt

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c| 8 
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
>  drivers/gpu/drm/i915/gt/intel_engine_user.c  | 1 +
>  drivers/gpu/drm/i915/i915_reg.h  | 1 +
>  4 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 3b7d750ad054..e0fbfac03979 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -244,6 +244,13 @@ static const struct engine_info intel_engines[] = {
>   { .graphics_ver = 12, .base = GEN12_COMPUTE3_RING_BASE }
>   }
>   },
> + [GSC0] = {
> + .class = OTHER_CLASS,
> + .instance = OTHER_GSC_INSTANCE,
> + .mmio_bases = {
> + { .graphics_ver = 12, .base = MTL_GSC_RING_BASE }
> + }
> + },
>  };
>  
>  /**
> @@ -324,6 +331,7 @@ u32 intel_engine_context_size(struct intel_gt *gt, u8 
> class)
>   case VIDEO_DECODE_CLASS:
>   case VIDEO_ENHANCEMENT_CLASS:
>   case COPY_ENGINE_CLASS:
> + case OTHER_CLASS:
>   if (GRAPHICS_VER(gt->i915) < 8)
>   return 0;
>   return GEN8_LR_CONTEXT_OTHER_SIZE;
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 6b5d4ea22b67..4fd54fb8810f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -136,6 +136,7 @@ enum intel_engine_id {
>   CCS2,
>   CCS3,
>  #define _CCS(n) (CCS0 + (n))
> + GSC0,
>   I915_NUM_ENGINES
>  #define INVALID_ENGINE ((enum intel_engine_id)-1)
>  };
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c 
> b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> index 46a174f8aa00..79312b734690 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> @@ -140,6 +140,7 @@ const char *intel_engine_class_repr(u8 class)
>   [COPY_ENGINE_CLASS] = "bcs",
>   [VIDEO_DECODE_CLASS] = "vcs",
>   [VIDEO_ENHANCEMENT_CLASS] = "vecs",
> + [OTHER_CLASS] = "other",
>   [COMPUTE_CLASS] = "ccs",
>   };
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1c0da50c0dc7..d056c3117ef2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -970,6 +970,7 @@
>  #define GEN11_VEBOX2_RING_BASE   0x1d8000
>  #define XEHP_VEBOX3_RING_BASE0x1e8000
>  #define XEHP_VEBOX4_RING_BASE0x1f8000
> +#define MTL_GSC_RING_BASE0x11a000
>  #define GEN12_COMPUTE0_RING_BASE 0x1a000
>  #define GEN12_COMPUTE1_RING_BASE 0x1c000
>  #define GEN12_COMPUTE2_RING_BASE 0x1e000
> -- 
> 2.37.3
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation

Re: [PATCH v2] drm/format-helper: Only advertise supported formats for conversion

2022-10-31 Thread Justin Forbes

On Thu, Oct 27, 2022 at 8:57 AM Hector Martin  wrote:
>
> drm_fb_build_fourcc_list() currently returns all emulated formats
> unconditionally as long as the native format is among them, even though
> not all combinations have conversion helpers. Although the list is
> arguably provided to userspace in precedence order, userspace can pick
> something out-of-order (and thus break when it shouldn't), or simply
> only support a format that is unsupported (and thus think it can work,
> which results in the appearance of a hang as FB blits fail later on,
> instead of the initialization error you'd expect in this case).
>
> Add checks to filter the list of emulated formats to only those
> supported for conversion to the native format. This presumes that there
> is a single native format (only the first is checked, if there are
> multiple). Refactoring this API to drop the native list or support it
> properly (by returning the appropriate emulated->native mapping table)
> is left for a future patch.
>
> The simpledrm driver is left as-is with a full table of emulated
> formats. This keeps all currently working conversions available and
> drops all the broken ones (i.e. this a strict bugfix patch, adding no
> new supported formats nor removing any actually working ones). In order
> to avoid proliferation of emulated formats, future drivers should
> advertise only XRGB as the sole emulated format (since some
> userspace assumes its presence).
>
> This fixes a real user regression where the ?RGB2101010 support commit
> started advertising it unconditionally where not supported, and KWin
> decided to start to use it over the native format and broke, but also
> the fixes the spurious RGB565/RGB888 formats which have been wrongly
> unconditionally advertised since the dawn of simpledrm.
>
> Fixes: 6ea966fca084 ("drm/simpledrm: Add [AX]RGB210101


> Cc: sta...@vger.kernel.org
> Signed-off-by: Hector Martin 

There is a CC for stable on here, but this patch does not apply in any
way on 6.0 or older kernels as the fourcc bits and considerable churn
came in with the 6.1 merge window.  You don't happen to have a
backport of this to 6.0 do you?

Thanks,
Justin

> ---
> I'm proposing this alternative approach after a heated discussion on
> IRC. I'm out of ideas, if y'all don't like this one you can figure it
> out for yourseves :-)
>
> Changes since v1:
> This v2 moves all the changes to the helper (so they will apply to
> the upcoming ofdrm, though ofdrm also needs to be fixed to trim its
> format table to only formats that should be emulated, probably only
> XRGB, to avoid further proliferating the use of conversions),
> and avoids touching more than one file. The API still needs cleanup
> as mentioned (supporting more than one native format is fundamentally
> broken, since the helper would need to tell the driver *what* native
> format to use for *each* emulated format somehow), but all current and
> planned users only pass in one native format, so this can (and should)
> be fixed later.
>
> Aside: After other IRC discussion, I'm testing nuking the
> XRGB2101010 <-> ARGB2101010 advertisement (which does not involve
> conversion) by removing those entries from simpledrm in the Asahi Linux
> downstream tree. As far as I'm concerned, it can be removed if nobody
> complains (by removing those entries from the simpledrm array), if
> maintainers are generally okay with removing advertised formats at all.
> If so, there might be other opportunities for further trimming the list
> non-native formats advertised to userspace.
>
> Tested with KWin-X11, KWin-Wayland, GNOME-X11, GNOME-Wayland, and Weston
> on both XRGB2101010 and RGB simpledrm framebuffers.
>
>  drivers/gpu/drm/drm_format_helper.c | 66 -
>  1 file changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c 
> b/drivers/gpu/drm/drm_format_helper.c
> index e2f76621453c..3ee59bae9d2f 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -807,6 +807,38 @@ static bool is_listed_fourcc(const uint32_t *fourccs, 
> size_t nfourccs, uint32_t
> return false;
>  }
>
> +static const uint32_t conv_from_xrgb[] = {
> +   DRM_FORMAT_XRGB,
> +   DRM_FORMAT_ARGB,
> +   DRM_FORMAT_XRGB2101010,
> +   DRM_FORMAT_ARGB2101010,
> +   DRM_FORMAT_RGB565,
> +   DRM_FORMAT_RGB888,
> +};
> +
> +static const uint32_t conv_from_rgb565_888[] = {
> +   DRM_FORMAT_XRGB,
> +   DRM_FORMAT_ARGB,
> +};
> +
> +static bool is_conversion_supported(uint32_t from, uint32_t to)
> +{
> +   switch (from) {
> +   case DRM_FORMAT_XRGB:
> +   case DRM_FORMAT_ARGB:
> +   return is_listed_fourcc(conv_from_xrgb, 
> ARRAY_SIZE(conv_from_xrgb), to);
> +   case DRM_FORMAT_RGB565:
> +   case DRM_FORMAT_RGB888:
> +   return is_listed_fourcc(conv_from_rgb565_888, 
> ARRAY_SIZE(conv_from_rg

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

--- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) ---
Are you getting interrupts on the GPU?  Check /proc/interrupts to see if you
are getting interrupts for the GPU.

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

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

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Martin Šušla from comment #0)
> 
> Would someone also tell us which workaround should be used under which
> performace/latency requirements? ("Maybe wrong but still an" EXAMPLE: Users
> who need the best performace or lowest latency should use pcie_port_pm=off,
> users who need the best battery life should use amdgpu.msi=0.)
> 

You should not need to override any of the defaults other than for debugging.

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

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

Re: [PATCH] drm/amd/display: add parameter backlight_min

On Sun, Oct 30, 2022 at 5:26 AM Filip Moc  wrote:
>
> There are some devices on which amdgpu won't allow user to set brightness
> to sufficiently low values even though the hardware would support it just
> fine.
>
> This usually happens in two cases when either configuration of brightness
> levels via ACPI/ATIF is not available and amdgpu falls back to defaults
> (currently 12 for minimum level) which may be too high for some devices or
> even the configuration via ATIF is available but the minimum brightness
> level provided by the manufacturer is set to unreasonably high value.
>
> In either case user can use this new module parameter to adjust the
> minimum allowed backlight brightness level.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=203439
> Signed-off-by: Filip Moc 

Does your system require an override and if so, what is it?  It would
be good to add a quirk for your system as well so that other users of
the same system wouldn't need to manually figure out an apply the
settings.

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 15 +++
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 0e6ddf05c23c..c5445402c49d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -200,6 +200,9 @@ extern uint amdgpu_dc_debug_mask;
>  extern uint amdgpu_dc_visual_confirm;
>  extern uint amdgpu_dm_abm_level;
>  extern int amdgpu_backlight;
> +#ifdef CONFIG_DRM_AMD_DC
> +extern int amdgpu_backlight_override_min[];
> +#endif
>  extern struct amdgpu_mgpu_info mgpu_info;
>  extern int amdgpu_ras_enable;
>  extern uint amdgpu_ras_mask;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 16f6a313335e..f2fb549ac52f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -43,6 +43,7 @@
>  #include "amdgpu_irq.h"
>  #include "amdgpu_dma_buf.h"
>  #include "amdgpu_sched.h"
> +#include "amdgpu_dm.h"
>  #include "amdgpu_fdinfo.h"
>  #include "amdgpu_amdkfd.h"
>
> @@ -853,6 +854,20 @@ int amdgpu_backlight = -1;
>  MODULE_PARM_DESC(backlight, "Backlight control (0 = pwm, 1 = aux, -1 auto 
> (default))");
>  module_param_named(backlight, amdgpu_backlight, bint, 0444);
>
> +/**
> + * DOC: backlight_min (array of int)
> + * Override minimum allowed backlight brightness signal (per display).
> + * Must be less than the maximum brightness signal.
> + * Negative value means no override.
> + *
> + * Defaults to all -1 (no override on any display).
> + */
> +#ifdef CONFIG_DRM_AMD_DC
> +int amdgpu_backlight_override_min[AMDGPU_DM_MAX_NUM_EDP] = {[0 ... 
> (AMDGPU_DM_MAX_NUM_EDP-1)] = -1};
> +MODULE_PARM_DESC(backlight_min, "Override minimum backlight brightness 
> signal (0..max-1, -1 = no override (default))");
> +module_param_array_named(backlight_min, amdgpu_backlight_override_min, int, 
> NULL, 0444);
> +#endif
> +
>  /**
>   * DOC: tmz (int)
>   * Trusted Memory Zone (TMZ) is a method to protect data being written
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index eb4ce7216104..e2c36ba93d05 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3911,6 +3911,21 @@ static void amdgpu_dm_update_backlight_caps(struct 
> amdgpu_display_manager *dm,
> dm->backlight_caps[bl_idx].min_input_signal = 
> AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> dm->backlight_caps[bl_idx].max_input_signal = 
> AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
>  #endif
> +
> +   if (amdgpu_backlight_override_min[bl_idx] >= 0) {
> +   if (amdgpu_backlight_override_min[bl_idx] < 
> dm->backlight_caps[bl_idx].max_input_signal) {
> +   DRM_INFO("amdgpu: backlight[%i]: overriding minimum 
> brightness from %i to %i\n",
> + bl_idx,
> + dm->backlight_caps[bl_idx].min_input_signal,
> + amdgpu_backlight_override_min[bl_idx]);
> +   dm->backlight_caps[bl_idx].min_input_signal = 
> amdgpu_backlight_override_min[bl_idx];
> +   } else {
> +   DRM_ERROR("amdgpu: backlight[%i]: minimum brightness 
> override (%i) is not below maximum (%i)\n",
> + bl_idx,
> + amdgpu_backlight_override_min[bl_idx],
> + 
> dm->backlight_caps[bl_idx].max_input_signal);
> +   }
> +   }
>  }
>
>  static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
>
> base-commit: d8c03bfe146fd5e081a252cd34f3f12ca0255357
> --
> 2.30.2
>

Re: [PATCH 02/11] ARM: sa1100: remove unused board files

2022-10-31 Thread Lee Jones

On Fri, 21 Oct 2022, Arnd Bergmann wrote:

> From: Arnd Bergmann 
> 
> The Cerf, H3100, Badge4, Hackkit, LART, NanoEngine, PLEB, Shannon and
> Simpad machines were all marked as unused as there are no known users
> left. Remove all of these, along with references to them in defconfig
> files and drivers.
> 
> Four machines remain now: Assabet, Collie (Zaurus SL5500), iPAQ H3600
> and Jornada 720, each of which had one person still using them, with
> Collie also being supported in Qemu.
> 
> Cc: Peter Chubb 
> Cc: Stefan Eletzhofer 
> Signed-off-by: Arnd Bergmann 
> ---
>  MAINTAINERS   |  11 -
>  arch/arm/Kconfig  |   6 -
>  arch/arm/boot/compressed/head-sa1100.S|   4 -
>  arch/arm/configs/badge4_defconfig | 105 -
>  arch/arm/configs/cerfcube_defconfig   |  73 ---
>  arch/arm/configs/hackkit_defconfig|  48 --
>  arch/arm/configs/lart_defconfig   |  64 ---
>  arch/arm/configs/pleb_defconfig   |  53 ---
>  arch/arm/configs/shannon_defconfig|  45 --
>  arch/arm/configs/simpad_defconfig | 100 -
>  arch/arm/mach-sa1100/Kconfig  | 111 -
>  arch/arm/mach-sa1100/Makefile |  21 -
>  arch/arm/mach-sa1100/badge4.c | 338 --
>  arch/arm/mach-sa1100/cerf.c   | 181 
>  arch/arm/mach-sa1100/h3100.c  | 140 --
>  arch/arm/mach-sa1100/hackkit.c| 184 
>  arch/arm/mach-sa1100/include/mach/badge4.h|  71 ---
>  arch/arm/mach-sa1100/include/mach/cerf.h  |  20 -
>  .../arm/mach-sa1100/include/mach/nanoengine.h |  48 --
>  arch/arm/mach-sa1100/include/mach/shannon.h   |  40 --
>  arch/arm/mach-sa1100/include/mach/simpad.h| 159 ---
>  arch/arm/mach-sa1100/lart.c   | 177 
>  arch/arm/mach-sa1100/nanoengine.c | 136 --
>  arch/arm/mach-sa1100/pci-nanoengine.c | 191 
>  arch/arm/mach-sa1100/pleb.c   | 148 --
>  arch/arm/mach-sa1100/shannon.c| 157 ---
>  arch/arm/mach-sa1100/simpad.c | 423 --
>  drivers/cpufreq/sa1110-cpufreq.c  |   6 -
>  drivers/mfd/Kconfig   |   2 +-

Acked-by: Lee Jones 

>  drivers/pcmcia/sa1100_generic.c   |   5 +-
>  drivers/pcmcia/sa1100_h3600.c |   2 +-
>  drivers/pcmcia/sa_generic.c   |   4 -
>  drivers/usb/host/ohci-sa.c|   5 +-
>  drivers/video/fbdev/sa1100fb.c|   1 -
>  34 files changed, 4 insertions(+), 3075 deletions(-)
>  delete mode 100644 arch/arm/configs/badge4_defconfig
>  delete mode 100644 arch/arm/configs/cerfcube_defconfig
>  delete mode 100644 arch/arm/configs/hackkit_defconfig
>  delete mode 100644 arch/arm/configs/lart_defconfig
>  delete mode 100644 arch/arm/configs/pleb_defconfig
>  delete mode 100644 arch/arm/configs/shannon_defconfig
>  delete mode 100644 arch/arm/configs/simpad_defconfig
>  delete mode 100644 arch/arm/mach-sa1100/badge4.c
>  delete mode 100644 arch/arm/mach-sa1100/cerf.c
>  delete mode 100644 arch/arm/mach-sa1100/h3100.c
>  delete mode 100644 arch/arm/mach-sa1100/hackkit.c
>  delete mode 100644 arch/arm/mach-sa1100/include/mach/badge4.h
>  delete mode 100644 arch/arm/mach-sa1100/include/mach/cerf.h
>  delete mode 100644 arch/arm/mach-sa1100/include/mach/nanoengine.h
>  delete mode 100644 arch/arm/mach-sa1100/include/mach/shannon.h
>  delete mode 100644 arch/arm/mach-sa1100/include/mach/simpad.h
>  delete mode 100644 arch/arm/mach-sa1100/lart.c
>  delete mode 100644 arch/arm/mach-sa1100/nanoengine.c
>  delete mode 100644 arch/arm/mach-sa1100/pci-nanoengine.c
>  delete mode 100644 arch/arm/mach-sa1100/pleb.c
>  delete mode 100644 arch/arm/mach-sa1100/shannon.c
>  delete mode 100644 arch/arm/mach-sa1100/simpad.c

-- 
Lee Jones [李琼斯]

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-31 Thread Ceraolo Spurio, Daniele





On 10/31/2022 5:55 AM, Tvrtko Ursulin wrote:


On 28/10/2022 18:00, Ceraolo Spurio, Daniele wrote:

On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote:


On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 


Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 
++

  1 file changed, 43 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c 
b/drivers/gpu/drm/i915/gt/intel_gt_irq.c

index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, 
const u8 instance,

    instance, iir);
  }
  -static void
-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
- const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 
instance)

  {
-    struct intel_engine_cs *engine;
-
-    /*
- * Platforms with standalone media have their media engines in 
another

- * GT.
- */
-    if (MEDIA_VER(gt->i915) >= 13 &&
-    (class == VIDEO_DECODE_CLASS || class == 
VIDEO_ENHANCEMENT_CLASS)) {

-    if (!gt->i915->media_gt)
-    goto err;
+    struct intel_gt *media_gt = gt->i915->media_gt;
  -    gt = gt->i915->media_gt;
+    /* we expect the non-media gt to be passed in */
+    GEM_BUG_ON(gt == media_gt);
+
+    if (!media_gt)
+    return gt;
+
+    switch (class) {
+    case VIDEO_DECODE_CLASS:
+    case VIDEO_ENHANCEMENT_CLASS:
+    return media_gt;
+    case OTHER_CLASS:
+    if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))

+    return media_gt;
+    fallthrough;
+    default:
+    return gt;
  }
-
-    if (instance <= MAX_ENGINE_INSTANCE)
-    engine = gt->engine_class[class][instance];
-    else
-    engine = NULL;
-
-    if (likely(engine))
-    return intel_engine_cs_irq(engine, iir);
-
-err:
-    WARN_ONCE(1, "unhandled engine interrupt class=0x%x, 
instance=0x%x\n",

-  class, instance);
  }
    static void
@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt 
*gt, const u32 identity)

  const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
  const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
  const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+    struct intel_engine_cs *engine;
    if (unlikely(!intr))
  return;
  -    if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)
-    return gen11_engine_irq_handler(gt, class, instance, intr);
+    /*
+ * Platforms with standalone media have the media and GSC 
engines in

+ * another GT.
+ */
+    gt = pick_gt(gt, class, instance);
+
+    if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+    engine = gt->engine_class[class][instance];
+    else
+    engine = NULL;
+
+    if (engine)
+    return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one 
since engine appears unused afterwards.


engine can be NULL in both branches of the if statement, so to get a 
unified if we'd have to do something like:


if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) {
     struct intel_engine_cs *engine = 
gt->engine_class[class][instance];

     if (engine)
             return intel_engine_cs_irq(engine, intr);
}

Is this what you are suggesting?


Right, two ifs are needed after all. Well at least it would avoid the 
pointless engine = NULL assignment. Up to you.


Absence of any out-of-range class/instance logging is intentional?


There is already a log further down in this function.

Daniele



Regards,

Tvrtko

[PATCH v2] dma-buf: fix racing conflict of dma_heap_add()

2022-10-31 Thread Dawei Li

Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
   list_for_each_entry
   strcmp(h->name)
kzallockzalloc
.. .
device_create  device_create
list_add
   list_add

The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.

v1: 
https://lore.kernel.org/all/tycp286mb2323950197f60fc3473123b7ca...@tycp286mb2323.jpnp286.prod.outlook.com/

v1->v2: Narrow down locking scope, check the existence of heap before
insertion, as suggested by Andrew Davis.

Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")

base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7

Signed-off-by: Dawei Li 
---
 drivers/dma-buf/dma-heap.c | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..1c787a147e3a 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -216,9 +216,21 @@ const char *dma_heap_get_name(struct dma_heap *heap)
return heap->name;
 }
 
+static inline bool dma_heap_exist(const char *name)
+{
+   struct dma_heap *h;
+
+   list_for_each_entry(h, &heap_list, list) {
+   if (!strcmp(h->name, name))
+   return true;
+   }
+
+   return false;
+}
+
 struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 {
-   struct dma_heap *heap, *h, *err_ret;
+   struct dma_heap *heap, *err_ret;
struct device *dev_ret;
unsigned int minor;
int ret;
@@ -235,13 +247,11 @@ struct dma_heap *dma_heap_add(const struct 
dma_heap_export_info *exp_info)
 
/* check the name is unique */
mutex_lock(&heap_list_lock);
-   list_for_each_entry(h, &heap_list, list) {
-   if (!strcmp(h->name, exp_info->name)) {
-   mutex_unlock(&heap_list_lock);
-   pr_err("dma_heap: Already registered heap named %s\n",
-  exp_info->name);
-   return ERR_PTR(-EINVAL);
-   }
+   if (dma_heap_exist(exp_info->name)) {
+   mutex_unlock(&heap_list_lock);
+   pr_err("dma_heap: Already registered heap named %s\n",
+  exp_info->name);
+   return ERR_PTR(-EINVAL);
}
mutex_unlock(&heap_list_lock);
 
@@ -283,13 +293,22 @@ struct dma_heap *dma_heap_add(const struct 
dma_heap_export_info *exp_info)
err_ret = ERR_CAST(dev_ret);
goto err2;
}
+
/* Add heap to the list */
mutex_lock(&heap_list_lock);
+   if (unlikely(dma_heap_exist(exp_info->name))) {
+   mutex_unlock(&heap_list_lock);
+   pr_err("dma_heap: Already registered heap named %s\n",
+  exp_info->name);
+   err_ret = ERR_PTR(-EINVAL);
+   goto err3;
+   }
list_add(&heap->list, &heap_list);
mutex_unlock(&heap_list_lock);
 
return heap;
-
+err3:
+   device_destroy(dma_heap_class, heap->heap_devt);
 err2:
cdev_del(&heap->heap_cdev);
 err1:
-- 
2.25.1

Re: [PATCH] drm/amd/display: drop vblank_lock from struct amdgpu_display_manager

2022-10-31 Thread Harry Wentland

On 2022-10-28 16:44, Hamza Mahfooz wrote:
> As of commit 09a5df6c444c ("drm/amd/display: Fix multi-display support
> for idle opt workqueue"), vblank_lock is no longer being used. So, don't
> init it in amdgpu_dm_init() and remove it from struct
> amdgpu_display_manager.
> 
> Signed-off-by: Hamza Mahfooz 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 ---
>  2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index eb4ce7216104..11afb4b24fd9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1394,7 +1394,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>  
>   mutex_init(&adev->dm.dc_lock);
>   mutex_init(&adev->dm.audio_lock);
> - spin_lock_init(&adev->dm.vblank_lock);
>  
>   if(amdgpu_dm_irq_init(adev)) {
>   DRM_ERROR("amdgpu: failed to initialize DM IRQ support.\n");
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index b5ce15c43bcc..b618b2586e7b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -365,13 +365,6 @@ struct amdgpu_display_manager {
>*/
>   struct mutex audio_lock;
>  
> - /**
> -  * @vblank_lock:
> -  *
> -  * Guards access to deferred vblank work state.
> -  */
> - spinlock_t vblank_lock;
> -
>   /**
>* @audio_component:
>*

Re: [PATCH] drm/amd/display: add parameter backlight_min

2022-10-31 Thread Harry Wentland

On 2022-10-29 15:13, Filip Moc wrote:
> There are some devices on which amdgpu won't allow user to set brightness
> to sufficiently low values even though the hardware would support it just
> fine.
> 
> This usually happens in two cases when either configuration of brightness
> levels via ACPI/ATIF is not available and amdgpu falls back to defaults
> (currently 12 for minimum level) which may be too high for some devices or
> even the configuration via ATIF is available but the minimum brightness
> level provided by the manufacturer is set to unreasonably high value.
> 
> In either case user can use this new module parameter to adjust the
> minimum allowed backlight brightness level.
> 

Thanks for this patch and covering all the bases.

It might be useful to have an example in the commit description on
how to set the array property. I assume it looks like this if I
wanted to set the first device to a minimum of 2 and leave the default
for the 2nd one:

amdgpu.backlight_min=2:-1

Either way, this patch is
Reviewed-by: Harry Wentland 

Harry

> Link: https://bugzilla.kernel.org/show_bug.cgi?id=203439>> 
> Signed-off-by: Filip Moc 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 15 +++
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 0e6ddf05c23c..c5445402c49d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -200,6 +200,9 @@ extern uint amdgpu_dc_debug_mask;
>  extern uint amdgpu_dc_visual_confirm;
>  extern uint amdgpu_dm_abm_level;
>  extern int amdgpu_backlight;
> +#ifdef CONFIG_DRM_AMD_DC
> +extern int amdgpu_backlight_override_min[];
> +#endif
>  extern struct amdgpu_mgpu_info mgpu_info;
>  extern int amdgpu_ras_enable;
>  extern uint amdgpu_ras_mask;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 16f6a313335e..f2fb549ac52f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -43,6 +43,7 @@
>  #include "amdgpu_irq.h"
>  #include "amdgpu_dma_buf.h"
>  #include "amdgpu_sched.h"
> +#include "amdgpu_dm.h"
>  #include "amdgpu_fdinfo.h"
>  #include "amdgpu_amdkfd.h"
>  
> @@ -853,6 +854,20 @@ int amdgpu_backlight = -1;
>  MODULE_PARM_DESC(backlight, "Backlight control (0 = pwm, 1 = aux, -1 auto 
> (default))");
>  module_param_named(backlight, amdgpu_backlight, bint, 0444);
>  
> +/**
> + * DOC: backlight_min (array of int)
> + * Override minimum allowed backlight brightness signal (per display).
> + * Must be less than the maximum brightness signal.
> + * Negative value means no override.
> + *
> + * Defaults to all -1 (no override on any display).
> + */
> +#ifdef CONFIG_DRM_AMD_DC
> +int amdgpu_backlight_override_min[AMDGPU_DM_MAX_NUM_EDP] = {[0 ... 
> (AMDGPU_DM_MAX_NUM_EDP-1)] = -1};
> +MODULE_PARM_DESC(backlight_min, "Override minimum backlight brightness 
> signal (0..max-1, -1 = no override (default))");
> +module_param_array_named(backlight_min, amdgpu_backlight_override_min, int, 
> NULL, 0444);
> +#endif
> +
>  /**
>   * DOC: tmz (int)
>   * Trusted Memory Zone (TMZ) is a method to protect data being written
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index eb4ce7216104..e2c36ba93d05 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3911,6 +3911,21 @@ static void amdgpu_dm_update_backlight_caps(struct 
> amdgpu_display_manager *dm,
>   dm->backlight_caps[bl_idx].min_input_signal = 
> AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
>   dm->backlight_caps[bl_idx].max_input_signal = 
> AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
>  #endif
> +
> + if (amdgpu_backlight_override_min[bl_idx] >= 0) {
> + if (amdgpu_backlight_override_min[bl_idx] < 
> dm->backlight_caps[bl_idx].max_input_signal) {
> + DRM_INFO("amdgpu: backlight[%i]: overriding minimum 
> brightness from %i to %i\n",
> +   bl_idx,
> +   dm->backlight_caps[bl_idx].min_input_signal,
> +   amdgpu_backlight_override_min[bl_idx]);
> + dm->backlight_caps[bl_idx].min_input_signal = 
> amdgpu_backlight_override_min[bl_idx];
> + } else {
> + DRM_ERROR("amdgpu: backlight[%i]: minimum brightness 
> override (%i) is not below maximum (%i)\n",
> +   bl_idx,
> +   amdgpu_backlight_override_min[bl_idx],
> +   dm->backlight_caps[bl_idx].max_input_signal);
> + }
> + }
>  }
>  
>  static int get_brightness_range(const struct amdgpu_dm_backlight

Re: [PATCH] drm/amd/display: fix an incorrect comment

On Fri, Oct 28, 2022 at 11:22 PM Alex Hung  wrote:
>
> This is a copy-and-paste error. Fix the comment to match the macro
> definition.
>
> Signed-off-by: Alex Hung 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h 
> b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
> index 63219ecd8478..1bf6b12f5663 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
> @@ -29,4 +29,4 @@
>
>  void dcn10_resource_construct_fp(struct dc *dc);
>
> -#endif /* __DCN20_FPU_H__ */
> +#endif /* __DCN10_FPU_H__ */
> --
> 2.38.1
>

Re: [PATCH] drm/amd/display (gcc13): fix enum mismatch

2022-10-31 Thread Harry Wentland

On 2022-10-31 07:42, Jiri Slaby (SUSE) wrote:
> rn_vbios_smu_set_dcn_low_power_state() produces a valid warning with
> gcc-13:
>   drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c:237:6: 
> error: conflicting types for 'rn_vbios_smu_set_dcn_low_power_state' due to 
> enum/integer mismatch; have 'void(struct clk_mgr_internal *, enum 
> dcn_pwr_state)'
>   drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h:36:6: 
> note: previous declaration of 'rn_vbios_smu_set_dcn_low_power_state' with 
> type 'void(struct clk_mgr_internal *, int)'
> 
> I.e. the type of the 2nd parameter of
> rn_vbios_smu_set_dcn_low_power_state() in the declaration is int, while
> the definition spells enum dcn_pwr_state. Synchronize them to the
> latter (and add a forward enum declaration).
> 
> Cc: Martin Liska 
> Cc: Harry Wentland 
> Cc: Leo Li 
> Cc: Rodrigo Siqueira 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: "Pan, Xinhui" 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jiri Slaby (SUSE) 

Reviewed-by: Harry Wentland 

Harry

> ---
>  .../drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h   | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h 
> b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h
> index 3e5df27aa96f..1ce19d875358 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.h
> @@ -26,6 +26,8 @@
>  #ifndef DAL_DC_RN_CLK_MGR_VBIOS_SMU_H_
>  #define DAL_DC_RN_CLK_MGR_VBIOS_SMU_H_
>  
> +enum dcn_pwr_state;
> +
>  int rn_vbios_smu_get_smu_version(struct clk_mgr_internal *clk_mgr);
>  int rn_vbios_smu_set_dispclk(struct clk_mgr_internal *clk_mgr, int 
> requested_dispclk_khz);
>  int rn_vbios_smu_set_dprefclk(struct clk_mgr_internal *clk_mgr);
> @@ -33,7 +35,7 @@ int rn_vbios_smu_set_hard_min_dcfclk(struct 
> clk_mgr_internal *clk_mgr, int reque
>  int rn_vbios_smu_set_min_deep_sleep_dcfclk(struct clk_mgr_internal *clk_mgr, 
> int requested_min_ds_dcfclk_khz);
>  void rn_vbios_smu_set_phyclk(struct clk_mgr_internal *clk_mgr, int 
> requested_phyclk_khz);
>  int rn_vbios_smu_set_dppclk(struct clk_mgr_internal *clk_mgr, int 
> requested_dpp_khz);
> -void rn_vbios_smu_set_dcn_low_power_state(struct clk_mgr_internal *clk_mgr, 
> int display_count);
> +void rn_vbios_smu_set_dcn_low_power_state(struct clk_mgr_internal *clk_mgr, 
> enum dcn_pwr_state);
>  void rn_vbios_smu_enable_48mhz_tmdp_refclk_pwrdwn(struct clk_mgr_internal 
> *clk_mgr, bool enable);
>  void rn_vbios_smu_enable_pme_wa(struct clk_mgr_internal *clk_mgr);
>  int rn_vbios_smu_is_periodic_retraining_disabled(struct clk_mgr_internal 
> *clk_mgr);

Re: [PATCH] drm/i915/mtl: Add MC6 Wa_14017210380 for SAMedia

2022-10-31 Thread Nilawar, Badal


Hi Rodrigo,

On 31-10-2022 15:19, Rodrigo Vivi wrote:

On Sat, Oct 29, 2022 at 12:59:35AM +0530, Badal Nilawar wrote:

This workaround is added for Media Tile of MTL A step. It is to help
pcode workaround which handles the hardware bug seen on CXL splitter
during package C2/C3 transitins due to MC6 entry/exit. As a part of
workaround pcode expect kmd to send mailbox message "media busy" when
components of Media tile is in use and "media not busy" when not in use.
As per workaround description gucrc need to be disabled so enabled
host based RC for Media tile.

HSD: 14017210380

Cc: Rodrigo Vivi 
Cc: Radhakrishna Sripada 
Cc: Vinay Belgaumkar 
Cc: Chris Wilson 
Signed-off-by: Badal Nilawar 
---
  drivers/gpu/drm/i915/gt/intel_gt_pm.c | 33 +++
  drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 13 -
  drivers/gpu/drm/i915/i915_drv.h   |  4 +++
  drivers/gpu/drm/i915/i915_reg.h   |  9 +++
  4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c 
b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index f553e2173bda..398dbeb298ca 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -19,10 +19,37 @@
  #include "intel_rc6.h"
  #include "intel_rps.h"
  #include "intel_wakeref.h"
+#include "intel_pcode.h"
  #include "pxp/intel_pxp_pm.h"
  
  #define I915_GT_SUSPEND_IDLE_TIMEOUT (HZ / 2)
  
+/*

+ * Wa_14017210380: mtl
+ */
+
+static bool mtl_needs_media_mc6_wa(struct intel_gt *gt)


+Ashutosh since we recently discussed about the term "MC6".

in that discussion we have concluded to not introduce a new term
since it is not used in any kind of spec and might only bring
more doubts then answers, although "Render" in the media gt
makes no sense as well.

In the end, most of the code is common and is called as rc6.
so we should maybe s/mc6/media_rc6 here?

Sure I will make above change and resend the patch.


The rest of the patch looks good to me. But we need to check the IGT
failure on a pm related test that failed... just to be sure.

Failures reported in IGT so far are not related to this change.

Regards,
Badal



+{
+   return (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
+   gt->type == GT_MEDIA);
+}
+
+static void mtl_mc6_wa_media_busy(struct intel_gt *gt)
+{
+   if (mtl_needs_media_mc6_wa(gt))
+   snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
+ PCODE_MBOX_GT_STATE_MEDIA_BUSY,
+ PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
+}
+
+static void mtl_mc6_wa_media_not_busy(struct intel_gt *gt)
+{
+   if (mtl_needs_media_mc6_wa(gt))
+   snb_pcode_write_p(gt->uncore, PCODE_MBOX_GT_STATE,
+ PCODE_MBOX_GT_STATE_MEDIA_NOT_BUSY,
+ PCODE_MBOX_GT_STATE_DOMAIN_MEDIA, 0);
+}
+
  static void user_forcewake(struct intel_gt *gt, bool suspend)
  {
int count = atomic_read(>->user_wakeref);
@@ -70,6 +97,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
  
  	GT_TRACE(gt, "\n");
  
+	/* Wa_14017210380: mtl */

+   mtl_mc6_wa_media_busy(gt);
+
/*
 * It seems that the DMC likes to transition between the DC states a lot
 * when there are no connected displays (no active power domains) during
@@ -119,6 +149,9 @@ static int __gt_park(struct intel_wakeref *wf)
GEM_BUG_ON(!wakeref);
intel_display_power_put_async(i915, POWER_DOMAIN_GT_IRQ, wakeref);
  
+	/* Wa_14017210380: mtl */

+   mtl_mc6_wa_media_not_busy(gt);
+
return 0;
  }
  
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c

index 8f8dd05835c5..cc6356ff84a5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
@@ -11,9 +11,20 @@
  
  static bool __guc_rc_supported(struct intel_guc *guc)

  {
+   struct intel_gt *gt = guc_to_gt(guc);
+
+   /*
+* Wa_14017210380: mtl
+* Do not enable gucrc to avoid additional interrupts which
+* may disrupt pcode wa.
+*/
+   if (IS_MTL_GRAPHICS_STEP(gt->i915, P, STEP_A0, STEP_B0) &&
+   gt->type == GT_MEDIA)
+   return false;
+
/* GuC RC is unavailable for pre-Gen12 */
return guc->submission_supported &&
-   GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
+   GRAPHICS_VER(gt->i915) >= 12;
  }
  
  static bool __guc_rc_selected(struct intel_guc *guc)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05b3300cc4ed..659b92382ff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -740,6 +740,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
  #define IS_XEHPSDV_GRAPHICS_STEP(__i915, since, until) \
(IS_XEHPSDV(__i915) && IS_GRAPHICS_STEP(__i915, since, until))
  
+#define IS_MTL_GRAPHICS_STEP(__i915, variant, s

Re: [PATCH v3] dma-buf: cma_heap: Remove duplicated 'by' in comment

2022-10-31 Thread AngeloGioacchino Del Regno


Il 28/10/22 08:55, Mark-PK Tsai ha scritto:

Remove duplicated 'by' from comment in cma_heap_allocate().

Signed-off-by: Mark-PK Tsai 
Reviewed-By: Mukesh Ojha 
Acked-by: John Stultz 


Reviewed-by: AngeloGioacchino Del Regno

Re: [PATCH v2 14/21] drm/fb-helper: Rename drm_fb_helper_unregister_fbi() to use _info postfix

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Rename drm_fb_helper_unregister_fbi() to drm_fb_helper_unregister_info()
> as part of unifying the naming within fbdev helpers. Adapt drivers. No
> functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

[Bug 216645] Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

Martin Šušla (ask4supp...@email.cz) changed:

   What|Removed |Added

 Attachment #303109|Kernel log created by the   |Kernel log created by the
description|script in the menuetry  |script in the menuentry

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

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

[Bug 216645] New: Fence fallback timer expired on ring gfx

https://bugzilla.kernel.org/show_bug.cgi?id=216645

Bug ID: 216645
   Summary: Fence fallback timer expired on ring gfx
   Product: Drivers
   Version: 2.5
Kernel Version: 5.15.0-43-generic
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: ask4supp...@email.cz
Regression: No

Created attachment 303109
  --> https://bugzilla.kernel.org/attachment.cgi?id=303109&action=edit
Kernel log created by the script in the menuetry

Sometimes when I run a KDE system monitor, or Chrome, my laptop freezes and
won't unfreeze until reboot (well, after a while I can move the mouse cursor,
but that's all I can do). 
I'm using Dell G5 SE 5505 with AMD Ryzen 7 4800H as a CPU, Radeon RX Vega 7 as
iGPU and AMD Radeon RX 5600M as dGPU. 

I've searched through existing bugs and found that it might be related to
interrupts. With that in mind, I've compiled a list of kernel parameters which
might be related and, as well as that, I've tested all of them: 

PW = Probably Working, NW = Not Working, NB = Not Booting
PW  pcie_port_pm=off
PW  amdgpu.msi=0
NW  amd_iommu=fullflush
NW  amd_iommu=force_isolation
NW  amd_iommu=off
NW  amd_iommu_intr=legacy
NW  amd_iommu_intr=vapic kvm-amd.avic=1
NW  iommu=off
NW  iommu=force
NW  iommu=noforce
NW  iommu=biomerge
NW  iommu=merge
NW  iommu=nomerge
NW  iommu=forcesac
NW  iommu=soft
NW  iommu=pt
NW  irqfixup
NW  irqpoll
NW  nointremap
NW  pcie_port_pm=force
NW  amdgpu.pcie_gen2=1
NW  amdgpu.pcie_gen2=0
NW  amdgpu.msi=1
NW  amdgpu.lockup_timeout=1000
NW  amdgpu.lockup_timeout=100
NW  amdgpu.aspm=1
NW  amdgpu.aspm=0
NW  amdgpu.bapm=1
NW  amdgpu.bapm=0
NW  amdgpu.ppfeaturemask=0xfff7bff7
NW  amdgpu.ppfeaturemask=0xfff7bdff
NW  amdgpu.ppfeaturemask=0xfff7bbff
NW  amdgpu.ppfeaturemask=0xfff73fff
NW  amdgpu.ppfeaturemask=0xfff3bfff
NW  amdgpu.exp_hw_support=1
NW  amdgpu.exp_hw_support=0
NW  amdgpu.forcelongtraining=0
NW  amdgpu.forcelongtraining=1
NW  amdgpu.cg_mask=0x
NW  amdgpu.cg_mask=0x
NW  amdgpu.pg_mask=0x
NW  amdgpu.ngg=1
NW  amdgpu.ngg=0
NW  amdgpu.job_hang_limit=1000
NW  amdgpu.job_hang_limit=100
NW  amdgpu.lbpw=1
NW  amdgpu.lbpw=0
NW  amdgpu.gpu_recovery=1
NW  amdgpu.gpu_recovery=0
NW  amdgpu.sched_policy=2
NW  amdgpu.sched_policy=1
NW  amdgpu.sched_policy=0
NW  amdgpu.ignore_crat=0
NW  amdgpu.ignore_crat=1
NW  amdgpu.ras_enable=0
NW  amdgpu.ras_enable=1
NW  amdgpu.async_gfx_ring=0
NW  amdgpu.async_gfx_ring=1
NW  amdgpu.mcbp=1
NW  amdgpu.mcbp=0
NW  amdgpu.mes=0
NW  amdgpu.mes_kiq=1
NW  amdgpu.mes_kiq=0
NW  amdgpu.reset_method=0
NW  amdgpu.reset_method=1
NW  amdgpu.reset_method=2
NW  amdgpu.reset_method=3
NW  amdgpu.reset_method=4
NW  amdgpu.reset_method=-1
NW  idle=nomwait
NB  amdgpu.pg_mask=0x
NB  amdgpu.mes=1



I've developed a script and a GRUB2 menu entry for live Kubuntu that triggers
the freeze and saves the dmesg into a file called Freeze_Dell_G5_SE_5505.sh.log
at the root of the drive it's being booted from.
Replace the ISO variable value with the path to your iso file if it's not at
root directory of the drive and/or if it's of a different version: 

menuentry "Start Kubuntu 22.04.1 (64 bit) without Ubiquity and with a freezing
script" {
ISO=/kubuntu-22.04.1-desktop-amd64.iso
set gfxpayload=keep
loopback loop "$ISO"
probe -u $root --set=rootid
linux   (loop)/casper/vmlinuz   iso-scan/filename="$ISO"
file=/cdrom/preseed/kubuntu.seed maybe-ubiquity quiet splash init=/bin/sh -- -c
'for script in /home/kubuntu/Desktop/Freeze_Dell_G5_SE_5505.sh ; do for autorun
in /home/kubuntu/.config/autostart/${script##*/} ; do ln -fs /dev/null
/etc/systemd/system/graphical.target.wants/ubiquity.service ; mkdir -p
${script%/*} ${autorun%/*} ; printf
\043!_/bin/sh++print\050\051_{+\tprintf_"@1"_,_seq_-s"_"_@\050\050_@\050stty_size_\074_@t_?_sed_"s/^/\050/,_s/_/_-_1_\051_*_/"\051_-_@{\0431}_\051\051_?_sed_s/[0-9]//g+}+t\075"@\050readlink_/proc/self/fd/0\051"++d\075"@\050env_LANG\075C_udisksctl_mount_-b_/dev/disk/by-uuid/$0_-o_sync_2\076_/dev/null_?_sed_"s/^Mounted_.*_at_//g,_s/\\.@//g"\051"+[_-d_"@d"_]_\046\046_f\075oflag\075direct_??_d\075"@{0%%/*}"+sudo_dmesg_-w_?_sudo_dd_of\075"@d/@{0\043\043*/}.log"_@f_\046+i\0750+seq_28_15_?_while_read_N_,_do+\tprint_@N+\ttimeout_3_env_DISPLAY\075:0_plasma-systemmonitor_\076_/dev/null_2\076\0461+\tn\075@N_,_while_[_0_-lt_@n_]_,_do+\t\tsleep_1+\t\tn\075@\050\050_@n_-_1_\051\051+\t\ti\075@\050\050_@i_^_1_\051\051+\t\t[_"@i"_\075_1_]_\046\046_pr

[PATCH] drm/i915/dg2: Introduce Wa_18017747507

2022-10-31 Thread Wayne Boyer

WA 18017747507 applies to all DG2 skus.

BSpec: 56035, 46121, 68173

Signed-off-by: Wayne Boyer 
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 +++
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index f4624262dc81..27b2641e1a53 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -501,6 +501,9 @@
 #define VF_PREEMPTION  _MMIO(0x83a4)
 #define   PREEMPTION_VERTEX_COUNT  REG_GENMASK(15, 0)
 
+#define VFG_PREEMPTION_CHICKEN _MMIO(0x83b4)
+#define  POLYGON_TRIFAN_LINELOOP_DISABLE   REG_BIT(4)
+
 #define GEN8_RC6_CTX_INFO  _MMIO(0x8504)
 
 #define XEHP_SQCM  MCR_REG(0x8724)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 2a35e7e66625..3cdf5c24dbc5 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2975,6 +2975,9 @@ general_render_compute_wa_init(struct intel_engine_cs 
*engine, struct i915_wa_li
 * Wa_22015475538:dg2
 */
wa_mcr_write_or(wal, LSC_CHICKEN_BIT_0_UDW, DIS_CHAIN_2XSIMD8);
+
+   /* Wa_18017747507:dg2 */
+   wa_masked_en(wal, VFG_PREEMPTION_CHICKEN, 
POLYGON_TRIFAN_LINELOOP_DISABLE);
}
 }
 
-- 
2.37.3

Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation

2022-10-31 Thread Ville Syrjälä

On Sun, Oct 30, 2022 at 08:42:52AM -0600, jim.cro...@gmail.com wrote:
> On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
>  wrote:
> >
> > On Thu, Oct 27, 2022 at 01:55:39PM -0600, jim.cro...@gmail.com wrote:
> > > On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
> > >  wrote:
> > > >
> > > > On Thu, Oct 27, 2022 at 09:37:52AM -0600, jim.cro...@gmail.com wrote:
> > > > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron  wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > > > On Thu, 20 Oct 2022, Ville Syrjälä 
> > > > > > >  wrote:
> > > > > > >> On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:
> > > > > > >>> On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:
> > > > > >  hi Greg, Dan, Jason, DRM-folk,
> > > > > > 
> > > > > >  heres follow-up to V6:
> > > > > >    rebased on driver-core/driver-core-next for -v6 applied bits 
> > > > > >  (thanks)
> > > > > >    rework drm_debug_enabled{_raw,_instrumented,} per Dan.
> > > > > > 
> > > > > >  It excludes:
> > > > > >    nouveau parts (immature)
> > > > > >    tracefs parts (I missed --to=Steve on v6)
> > > > > >    split _ddebug_site and de-duplicate experiment (way unready)
> > > > > > 
> > > > > >  IOW, its the remaining commits of V6 on which Dan gave his 
> > > > > >  Reviewed-by.
> > > > > > 
> > > > > >  If these are good to apply, I'll rebase and repost the rest 
> > > > > >  separately.
> > > > > > >>>
> > > > > > >>> All now queued up, thanks.
> > > > > > >>
> > > > > > >> This stuff broke i915 debugs. When I first load i915 no debug 
> > > > > > >> prints are
> > > > > > >> produced. If I then go fiddle around in 
> > > > > > >> /sys/module/drm/parameters/debug
> > > > > > >> the debug prints start to suddenly work.
> > > > > > >
> > > > > > > Wait what? I always assumed the default behaviour would stay the 
> > > > > > > same,
> > > > > > > which is usually how we roll. It's a regression in my books. 
> > > > > > > We've got a
> > > > > > > CI farm that's not very helpful in terms of dmesg logging right 
> > > > > > > now
> > > > > > > because of this.
> > > > > > >
> > > > > > > BR,
> > > > > > > Jani.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > That doesn't sound good - so you are saying that prior to this 
> > > > > > change some
> > > > > > of the drm debugs were default enabled. But now you have to 
> > > > > > manually enable
> > > > > > them?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > -Jason
> > > > >
> > > > >
> > > > > Im just seeing this now.
> > > > > Any new details ?
> > > >
> > > > No. We just disabled it as BROKEN for now. I was just today thinking
> > > > about sending that patch out if no solutin is forthcoming soon since
> > > > we need this working before 6.1 is released.
> > > >
> > > > Pretty sure you should see the problem immediately with any driver
> > > > (at least if it's built as a module, didn't try builtin). Or at least
> > > > can't think what would make i915 any more special.
> > > >
> > >
> > > So, I should note -
> > > 99% of my time & energy on this dyndbg + drm patchset
> > > has been done using virtme,
> > > so my world-view (and dev-hack-test env) has been smaller, simpler
> > > maybe its been fatally simplistic.
> > >
> > > ive just rebuilt v6.0  (before the trouble)
> > > and run it thru my virtual home box,
> > > I didnt see any unfamiliar drm-debug output
> > > that I might have inadvertently altered somehow
> > >
> > > I have some real HW I can put a reference kernel on,0
> > > to look for the missing output, but its all gonna take some time,
> > > esp to tighten up my dev-test-env
> > >
> > > in the meantime, there is:
> > >
> > > config DRM_USE_DYNAMIC_DEBUG
> > > bool "use dynamic debug to implement drm.debug"
> > > default y
> > > depends on DRM
> > > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > > depends on JUMP_LABEL
> > > help
> > >   Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > >   Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > >   bytes per callsite, the .data costs can be substantial, and
> > >   are therefore configurable.
> > >
> > > Does changing the default fix things for i915 dmesg ?
> >
> > I think we want to mark it BROKEN in addition to make sure no one
> 
> Ok, I get the distinction now.
> youre spelling that
>   depends on BROKEN
> 
> I have a notional explanation, and a conflating commit:
> 
> can you eliminate
> git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
> 
> as the cause ?

Reverting that doesn't help.

> 
> 
> 
> commit ccc2b496324c13e917ef05f563626f4e7826bef1
> Author: Jim Cromie 
> Date:   Sun Sep 11 23:28:51 2022 -0600
> 
> drm_print: prefer bare printk KERN_DEBUG on generic fn
> 
> drm_print.c calls pr_debug() just once, from __drm_printfn_debug(),
> which is a generic/service fn.  The callsite is compile-time enabled
> by DEBUG in both DYNAMIC_DEBUG=y/n builds

Re: [Intel-gfx] [PATCH 3/5] drm/i915/mtl: add GSC CS interrupt support

2022-10-31 Thread Tvrtko Ursulin




On 28/10/2022 18:00, Ceraolo Spurio, Daniele wrote:

On 10/28/2022 1:38 AM, Tvrtko Ursulin wrote:


On 27/10/2022 23:15, Daniele Ceraolo Spurio wrote:

The GSC CS re-uses the same interrupt bits that the GSC used in older
platforms. This means that we can now have an engine interrupt coming
out of OTHER_CLASS, so we need to handle that appropriately.

Signed-off-by: Daniele Ceraolo Spurio 
Cc: Matt Roper 
---
  drivers/gpu/drm/i915/gt/intel_gt_irq.c | 78 ++
  1 file changed, 43 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c 
b/drivers/gpu/drm/i915/gt/intel_gt_irq.c

index f26882fdc24c..34ff1ee7e931 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -81,35 +81,27 @@ gen11_other_irq_handler(struct intel_gt *gt, 
const u8 instance,

    instance, iir);
  }
  -static void
-gen11_engine_irq_handler(struct intel_gt *gt, const u8 class,
- const u8 instance, const u16 iir)
+static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 
instance)

  {
-    struct intel_engine_cs *engine;
-
-    /*
- * Platforms with standalone media have their media engines in 
another

- * GT.
- */
-    if (MEDIA_VER(gt->i915) >= 13 &&
-    (class == VIDEO_DECODE_CLASS || class == 
VIDEO_ENHANCEMENT_CLASS)) {

-    if (!gt->i915->media_gt)
-    goto err;
+    struct intel_gt *media_gt = gt->i915->media_gt;
  -    gt = gt->i915->media_gt;
+    /* we expect the non-media gt to be passed in */
+    GEM_BUG_ON(gt == media_gt);
+
+    if (!media_gt)
+    return gt;
+
+    switch (class) {
+    case VIDEO_DECODE_CLASS:
+    case VIDEO_ENHANCEMENT_CLASS:
+    return media_gt;
+    case OTHER_CLASS:
+    if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, 
GSC0))

+    return media_gt;
+    fallthrough;
+    default:
+    return gt;
  }
-
-    if (instance <= MAX_ENGINE_INSTANCE)
-    engine = gt->engine_class[class][instance];
-    else
-    engine = NULL;
-
-    if (likely(engine))
-    return intel_engine_cs_irq(engine, iir);
-
-err:
-    WARN_ONCE(1, "unhandled engine interrupt class=0x%x, 
instance=0x%x\n",

-  class, instance);
  }
    static void
@@ -118,12 +110,24 @@ gen11_gt_identity_handler(struct intel_gt *gt, 
const u32 identity)

  const u8 class = GEN11_INTR_ENGINE_CLASS(identity);
  const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
  const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
+    struct intel_engine_cs *engine;
    if (unlikely(!intr))
  return;
  -    if (class <= COPY_ENGINE_CLASS || class == COMPUTE_CLASS)
-    return gen11_engine_irq_handler(gt, class, instance, intr);
+    /*
+ * Platforms with standalone media have the media and GSC 
engines in

+ * another GT.
+ */
+    gt = pick_gt(gt, class, instance);
+
+    if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE)
+    engine = gt->engine_class[class][instance];
+    else
+    engine = NULL;
+
+    if (engine)
+    return intel_engine_cs_irq(engine, intr);


Drive by observation - you could fold the above two ifs into one since 
engine appears unused afterwards.


engine can be NULL in both branches of the if statement, so to get a 
unified if we'd have to do something like:


if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) {
         struct intel_engine_cs *engine = 
gt->engine_class[class][instance];

         if (engine)
                 return intel_engine_cs_irq(engine, intr);
}

Is this what you are suggesting?


Right, two ifs are needed after all. Well at least it would avoid the 
pointless engine = NULL assignment. Up to you.


Absence of any out-of-range class/instance logging is intentional?

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Don't deadlock busyness stats vs reset

2022-10-31 Thread Tvrtko Ursulin




On 31/10/2022 10:09, Tvrtko Ursulin wrote:


On 28/10/2022 20:46, john.c.harri...@intel.com wrote:

From: John Harrison 

The engine busyness stats has a worker function to do things like
64bit extend the 32bit hardware counters. The GuC's reset prepare
function flushes out this worker function to ensure no corruption
happens during the reset. Unforunately, the worker function has an
infinite wait for active resets to finish before doing its work. Thus
a deadlock would occur if the worker function had actually started
just as the reset starts.

Update the worker to abort if a reset is in progress rather than
waiting for it to complete. It will still acquire the reset lock in
the case where a reset was not already in progress. So the processing
is still safe from corruption, but the deadlock can no longer occur.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/gt/intel_reset.c | 15 ++-
  drivers/gpu/drm/i915/gt/intel_reset.h |  1 +
  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  6 --
  3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
b/drivers/gpu/drm/i915/gt/intel_reset.c

index 3159df6cdd492..2f48c6e4420ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1407,7 +1407,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
  intel_runtime_pm_put(gt->uncore->rpm, wakeref);
  }
-int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu)
+static int _intel_gt_reset_trylock(struct intel_gt *gt, int *srcu, 
bool retry)

  {
  might_lock(>->reset.backoff_srcu);
  might_sleep();
@@ -1416,6 +1416,9 @@ int intel_gt_reset_trylock(struct intel_gt *gt, 
int *srcu)

  while (test_bit(I915_RESET_BACKOFF, >->reset.flags)) {
  rcu_read_unlock();
+    if (!retry)
+    return -EBUSY;
+
  if (wait_event_interruptible(gt->reset.queue,
   !test_bit(I915_RESET_BACKOFF,
 >->reset.flags)))


Would it be more obvious to rename the existing semantics to 
intel_gt_reset_interruptible(), while the flavour you add in this patch 
truly is trylock? I am not sure, since it's all a bit special, but 
trylock sure feels confusing if it can sleep forever...


Oh and might_sleep() shouldn't be there with the trylock version - I 
mean any flavour of the real trylock.


Regards,

Tvrtko

Re: [PATCH v2 13/21] drm/fb-helper: Rename drm_fb_helper_alloc_fbi() to use _info postfix

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Rename drm_fb_helper_alloc_fbi() to drm_fb_helper_alloc_info() as
> part of unifying the naming within fbdev helpers. Adapt drivers. No
> functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 12/21] drm/fb_helper: Rename field fbdev to info in struct drm_fb_helper

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Rename struct drm_fb_helper.fbdev to info. The current name is
> misleading as it overlaps with generic fbdev naming conventions.
> Adapt to the usual naming in fbdev drivers by calling the field
> 'info'. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Agreed. I got confused by this naming in the past.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 00/10] Connect VFIO to IOMMUFD

2022-10-31 Thread Yi Liu


On 2022/10/31 20:18, Jason Gunthorpe wrote:

On Mon, Oct 31, 2022 at 06:38:45PM +0800, Yi Liu wrote:

Hi Jason,

On 2022/10/26 02:17, Jason Gunthorpe wrote:

This series provides an alternative container layer for VFIO implemented
using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will
not be compiled in.

At this point iommufd can be injected by passing in a iommfd FD to
VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd
to obtain the compat IOAS and then connect up all the VFIO drivers as
appropriate.

This is temporary stopping point, a following series will provide a way to
directly open a VFIO device FD and directly connect it to IOMMUFD using
native ioctls that can expose the IOMMUFD features like hwpt, future
vPASID and dynamic attachment.

This series, in compat mode, has passed all the qemu tests we have
available, including the test suites for the Intel GVT mdev. Aside from
the temporary limitation with P2P memory this is belived to be fully
compatible with VFIO.

This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd


In our side, we found the gvt-g test failed. Guest i915 driver stuck at
init phase. While with your former version  (commit ID
a249441ba6fd9d658f4a1b568453e3a742d12686), gvt-g test is passing.


Oh, I didn't realize you grabbed such an older version for this testing..


yeah, this was grabbed before your posting. :-)


noticed there a quite a few change in iommufd/pages.c from last version.
We are internally tracing in the gvt-g side, may also good to have your
attention.


syzkaller just ran into this that I was starting to investigate:

@@ -1505,7 +1505,7 @@ int iopt_pages_fill_xarray(struct iopt_pages *pages, 
unsigned long start_index,
 int rc;
  
 pfn_reader_user_init(&user, pages);

-   user.upages_len = last_index - start_index + 1;
+   user.upages_len = (last_index - start_index + 1) * sizeof(*out_pages);
 interval_tree_for_each_double_span(&span, &pages->access_itree,

It would certainly hit gvt - but you should get WARN_ON's not hangs

There is something wrong with the test suite that it isn't covering
the above, I'm going to look into that today.


sounds to be the cause. I didn't see any significant change in vfio_main.c
that may fail gvt. So should the iommufd changes. Then we will re-run the
test after your update.:-)

--
Regards,
Yi Liu

[PATCH] drm/i915/mtl: Media GT and Render GT share common GGTT

2022-10-31 Thread Aravind Iddamsetty

On XE_LPM+ platforms the media engines are carved out into a separate
GT but have a common GGTMMADR address range which essentially makes
the GGTT address space to be shared between media and render GT.

BSPEC: 63834

Cc: Matt Roper 
Signed-off-by: Aravind Iddamsetty 
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c  | 49 +++---
 drivers/gpu/drm/i915/gt/intel_gt.c| 15 +-
 drivers/gpu/drm/i915/gt/intel_gt_types.h  |  3 ++
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  3 ++
 drivers/gpu/drm/i915/i915_driver.c| 19 +--
 drivers/gpu/drm/i915/i915_gem_evict.c | 63 +--
 drivers/gpu/drm/i915/i915_vma.c   |  5 +-
 drivers/gpu/drm/i915/selftests/i915_gem.c |  2 +
 drivers/gpu/drm/i915/selftests/mock_gtt.c |  1 +
 9 files changed, 115 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 2518cebbf931..f5c2f3c58627 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -196,10 +196,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
 
 void i915_ggtt_suspend(struct i915_ggtt *ggtt)
 {
+   struct intel_gt *gt;
+
i915_ggtt_suspend_vm(&ggtt->vm);
ggtt->invalidate(ggtt);
 
-   intel_gt_check_and_clear_faults(ggtt->vm.gt);
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_gt_check_and_clear_faults(gt);
 }
 
 void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
@@ -214,27 +217,36 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
 
 static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
 {
-   struct intel_uncore *uncore = ggtt->vm.gt->uncore;
+   struct intel_uncore *uncore;
+   struct intel_gt *gt;
 
-   /*
-* Note that as an uncached mmio write, this will flush the
-* WCB of the writes into the GGTT before it triggers the invalidate.
-*/
-   intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
+   uncore = gt->uncore;
+   /*
+* Note that as an uncached mmio write, this will flush the
+* WCB of the writes into the GGTT before it triggers the 
invalidate.
+*/
+   intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, 
GFX_FLSH_CNTL_EN);
+   }
 }
 
 static void guc_ggtt_invalidate(struct i915_ggtt *ggtt)
 {
-   struct intel_uncore *uncore = ggtt->vm.gt->uncore;
struct drm_i915_private *i915 = ggtt->vm.i915;
 
gen8_ggtt_invalidate(ggtt);
 
-   if (GRAPHICS_VER(i915) >= 12)
-   intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR,
- GEN12_GUC_TLB_INV_CR_INVALIDATE);
-   else
-   intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+   if (GRAPHICS_VER(i915) >= 12) {
+   struct intel_gt *gt;
+
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_uncore_write_fw(gt->uncore,
+ GEN12_GUC_TLB_INV_CR,
+ GEN12_GUC_TLB_INV_CR_INVALIDATE);
+   } else {
+   intel_uncore_write_fw(ggtt->vm.gt->uncore,
+ GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+   }
 }
 
 u64 gen8_ggtt_pte_encode(dma_addr_t addr,
@@ -986,8 +998,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
ggtt->vm.pte_encode = gen8_ggtt_pte_encode;
 
-   setup_private_pat(ggtt->vm.gt);
-
return ggtt_probe_common(ggtt, size);
 }
 
@@ -1186,7 +1196,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct 
intel_gt *gt)
(u64)ggtt->mappable_end >> 20);
drm_dbg(&i915->drm, "DSM size = %lluM\n",
(u64)resource_size(&intel_graphics_stolen_res) >> 20);
-
+   INIT_LIST_HEAD(&ggtt->gt_list);
return 0;
 }
 
@@ -1296,9 +1306,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
 
 void i915_ggtt_resume(struct i915_ggtt *ggtt)
 {
+   struct intel_gt *gt;
bool flush;
 
-   intel_gt_check_and_clear_faults(ggtt->vm.gt);
+   list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+   intel_gt_check_and_clear_faults(gt);
 
flush = i915_ggtt_resume_vm(&ggtt->vm);
 
@@ -1307,9 +1319,6 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt)
if (flush)
wbinvd_on_all_cpus();
 
-   if (GRAPHICS_VER(ggtt->vm.i915) >= 8)
-   setup_private_pat(ggtt->vm.gt);
-
intel_ggtt_restore_fences(ggtt);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 2e796ffad911..d72efb74563a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -110,9 +110,17 @@ static int intel_gt_probe_lmem(struct intel_gt *gt)
 
 int intel_gt_assign_ggtt(struct intel_gt

Re: [PATCH v8 5/5] drm/bridge: cdns-dsi: Add support for J721E wrapper

Hi Tomi,

On 09:12-20221026, Tomi Valkeinen wrote:
> Hi,
> 
> On 21/10/2022 20:18, Rahul T R wrote:
> > Add support for wrapper settings for DSI bridge on
> > j721e. Also set the DPI input to DPI0
> 
> I think a few more words on the HW layout would be nice. What does the
> wrapper do and how is it connected to the DSS.
> 
> > Signed-off-by: Rahul T R 
> > ---
> >   drivers/gpu/drm/bridge/cadence/Kconfig| 10 
> >   drivers/gpu/drm/bridge/cadence/Makefile   |  1 +
> >   .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 37 +-
> >   .../gpu/drm/bridge/cadence/cdns-dsi-core.h| 13 +
> >   .../gpu/drm/bridge/cadence/cdns-dsi-j721e.c   | 51 +++
> >   .../gpu/drm/bridge/cadence/cdns-dsi-j721e.h   | 18 +++
> >   6 files changed, 129 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.c
> >   create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.h
> > 
> > diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig 
> > b/drivers/gpu/drm/bridge/cadence/Kconfig
> > index 8fbb46c66094..663a02d96420 100644
> > --- a/drivers/gpu/drm/bridge/cadence/Kconfig
> > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig
> > @@ -36,3 +36,13 @@ config DRM_CDNS_DSI
> > help
> >   Support Cadence DPI to DSI bridge. This is an internal
> >   bridge and is meant to be directly embedded in a SoC.
> > +
> > +if DRM_CDNS_DSI
> > +
> > +config DRM_CDNS_DSI_J721E
> > +   bool "J721E Cadence DPI/DSI wrapper support"
> > +   default y
> > +   help
> > + Support J721E Cadence DPI/DSI wrapper. This wrapper adds
> > + support to select which DPI input to use for the bridge.
> 
> I'm not sure if the above is quite necessary here. If I understand right,
> there's only one way on J721E to mux the DPI signal going to the DSI. If you
> write "adds support to select DPI input" it sounds like there's something to
> select, and this config somehow enables that selection for the user.
> 
> Perhaps instead just say something like "Support J721E Cadence DPI/DSI
> wrapper. The wrapper manages the routing of the DSS DPI signal to the
> Cadence DSI.", or something along those lines.
> 
> Also, you say "DPI/DSI wrapper". How does this wrap DPI? Isn't this just a
> DSI wrapper?
> 
> > +endif
> > diff --git a/drivers/gpu/drm/bridge/cadence/Makefile 
> > b/drivers/gpu/drm/bridge/cadence/Makefile
> > index e3d8e9a40784..4cffc8ff71c4 100644
> > --- a/drivers/gpu/drm/bridge/cadence/Makefile
> > +++ b/drivers/gpu/drm/bridge/cadence/Makefile
> > @@ -4,3 +4,4 @@ cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
> >   cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
> >   obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> >   cdns-dsi-y := cdns-dsi-core.o
> > +cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o
> > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c 
> > b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> > index cba91247ab26..4b7de38ef1b0 100644
> > --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> > +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> > @@ -15,12 +15,16 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> >   #include 
> >   #include "cdns-dsi-core.h"
> > +#ifdef CONFIG_DRM_CDNS_DSI_J721E
> > +#include "cdns-dsi-j721e.h"
> > +#endif
> >   static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input)
> >   {
> > @@ -265,6 +269,10 @@ static void cdns_dsi_bridge_disable(struct drm_bridge 
> > *bridge)
> > val = readl(dsi->regs + MCTL_MAIN_EN) & ~IF_EN(input->id);
> > writel(val, dsi->regs + MCTL_MAIN_EN);
> > +
> > +   if (dsi->platform_ops && dsi->platform_ops->disable)
> > +   dsi->platform_ops->disable(dsi);
> > +
> > pm_runtime_put(dsi->base.dev);
> >   }
> > @@ -360,6 +368,9 @@ static void cdns_dsi_bridge_enable(struct drm_bridge 
> > *bridge)
> > if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
> > return;
> > +   if (dsi->platform_ops && dsi->platform_ops->enable)
> > +   dsi->platform_ops->enable(dsi);
> > +
> > mode = &bridge->encoder->crtc->state->adjusted_mode;
> > nlanes = output->dev->lanes;
> > @@ -800,6 +811,8 @@ static int cdns_dsi_drm_probe(struct platform_device 
> > *pdev)
> > goto err_disable_pclk;
> > }
> > +   dsi->platform_ops = of_device_get_match_data(&pdev->dev);
> > +
> > val = readl(dsi->regs + IP_CONF);
> > dsi->direct_cmd_fifo_depth = 1 << (DIRCMD_FIFO_DEPTH(val) + 2);
> > dsi->rx_fifo_depth = RX_FIFO_DEPTH(val);
> > @@ -835,14 +848,27 @@ static int cdns_dsi_drm_probe(struct platform_device 
> > *pdev)
> > dsi->base.dev = &pdev->dev;
> > dsi->base.ops = &cdns_dsi_ops;
> > +   if (dsi->platform_ops && dsi->platform_ops->init) {
> > +   ret = dsi->platform_ops->init(dsi);
> > +   if (ret != 0) {
> > +   dev_err(&pdev->dev, "platform initialization faile

Re: [PATCH v2 11/21] drm/fb-helper: Cleanup include statements in header file

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Only include what we have to.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
Nice cleanup.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 10/21] drm/tve200: Include

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Include  for of_match_ptr().
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 09/21] drm/panel-ili9341: Include

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Include  for devm_of_find_backlight().
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

[PATCH v9 3/5] drm/bridge: cdns-dsi: Move to drm/bridge/cadence

Move the cadence dsi bridge under drm/bridge/cadence
directory, to prepare for adding j721e wrapper
support

Signed-off-by: Rahul T R 
Reviewed-by: Tomi Valkeinen 
---
 drivers/gpu/drm/bridge/Kconfig| 11 ---
 drivers/gpu/drm/bridge/Makefile   |  1 -
 drivers/gpu/drm/bridge/cadence/Kconfig| 11 +++
 drivers/gpu/drm/bridge/cadence/Makefile   |  2 ++
 .../bridge/{cdns-dsi.c => cadence/cdns-dsi-core.c}|  0
 5 files changed, 13 insertions(+), 12 deletions(-)
 rename drivers/gpu/drm/bridge/{cdns-dsi.c => cadence/cdns-dsi-core.c} (100%)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 57946d80b02d..8b2226f72b24 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -15,17 +15,6 @@ config DRM_PANEL_BRIDGE
 menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
 
-config DRM_CDNS_DSI
-   tristate "Cadence DPI/DSI bridge"
-   select DRM_KMS_HELPER
-   select DRM_MIPI_DSI
-   select DRM_PANEL_BRIDGE
-   select GENERIC_PHY_MIPI_DPHY
-   depends on OF
-   help
- Support Cadence DPI to DSI bridge. This is an internal
- bridge and is meant to be directly embedded in a SoC.
-
 config DRM_CHIPONE_ICN6211
tristate "Chipone ICN6211 MIPI-DSI/RGB Converter bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1884803c6860..52f6e8b4a821 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,5 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
 obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
 obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
 obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig 
b/drivers/gpu/drm/bridge/cadence/Kconfig
index 1d06182bea71..8fbb46c66094 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -25,3 +25,14 @@ config DRM_CDNS_MHDP8546_J721E
  initializes the J721E Display Port and sets up the
  clock and data muxes.
 endif
+
+config DRM_CDNS_DSI
+   tristate "Cadence DPI/DSI bridge"
+   select DRM_KMS_HELPER
+   select DRM_MIPI_DSI
+   select DRM_PANEL_BRIDGE
+   select GENERIC_PHY_MIPI_DPHY
+   depends on OF
+   help
+ Support Cadence DPI to DSI bridge. This is an internal
+ bridge and is meant to be directly embedded in a SoC.
diff --git a/drivers/gpu/drm/bridge/cadence/Makefile 
b/drivers/gpu/drm/bridge/cadence/Makefile
index 4d2db8df1bc6..e3d8e9a40784 100644
--- a/drivers/gpu/drm/bridge/cadence/Makefile
+++ b/drivers/gpu/drm/bridge/cadence/Makefile
@@ -2,3 +2,5 @@
 obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o
 cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
 cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
+obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
+cdns-dsi-y := cdns-dsi-core.o
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c 
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
similarity index 100%
rename from drivers/gpu/drm/bridge/cdns-dsi.c
rename to drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
-- 
2.38.0

[PATCH v9 5/5] drm/bridge: cdns-dsi: Add support for J721E wrapper

Add support for wrapper settings for DSI bridge on
j721e. Also enable DPI0

---  ---
|  ---|  |---  |
| DSS  | DPI2 |->| DPI0 |  DSI Wrapper |
|  ---|  |---  |
---  ---

As shown above DPI2 output of DSS is connected
to DPI0 input of DSI Wrapper, DSI wrapper
gives control wheather to enable/disable DPI0
input. In j721e above is the only configuration
supported

Signed-off-by: Rahul T R 
---
 drivers/gpu/drm/bridge/cadence/Kconfig| 10 
 drivers/gpu/drm/bridge/cadence/Makefile   |  1 +
 .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 35 -
 .../gpu/drm/bridge/cadence/cdns-dsi-core.h| 13 +
 .../gpu/drm/bridge/cadence/cdns-dsi-j721e.c   | 51 +++
 .../gpu/drm/bridge/cadence/cdns-dsi-j721e.h   | 16 ++
 6 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.h

diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig 
b/drivers/gpu/drm/bridge/cadence/Kconfig
index 8fbb46c66094..f8ea0393fe8a 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -36,3 +36,13 @@ config DRM_CDNS_DSI
help
  Support Cadence DPI to DSI bridge. This is an internal
  bridge and is meant to be directly embedded in a SoC.
+
+if DRM_CDNS_DSI
+
+config DRM_CDNS_DSI_J721E
+   bool "J721E Cadence DSI wrapper support"
+   default y
+   help
+ Support J721E Cadence DSI wrapper. The wrapper manages
+ the routing of the DSS DPI signal to the Cadence DSI.
+endif
diff --git a/drivers/gpu/drm/bridge/cadence/Makefile 
b/drivers/gpu/drm/bridge/cadence/Makefile
index e3d8e9a40784..4cffc8ff71c4 100644
--- a/drivers/gpu/drm/bridge/cadence/Makefile
+++ b/drivers/gpu/drm/bridge/cadence/Makefile
@@ -4,3 +4,4 @@ cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
 cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
 cdns-dsi-y := cdns-dsi-core.o
+cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index cba91247ab26..a5b5dfbf09a0 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -15,12 +15,16 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 
 #include "cdns-dsi-core.h"
+#ifdef CONFIG_DRM_CDNS_DSI_J721E
+#include "cdns-dsi-j721e.h"
+#endif
 
 static inline struct cdns_dsi *input_to_dsi(struct cdns_dsi_input *input)
 {
@@ -265,6 +269,10 @@ static void cdns_dsi_bridge_disable(struct drm_bridge 
*bridge)
 
val = readl(dsi->regs + MCTL_MAIN_EN) & ~IF_EN(input->id);
writel(val, dsi->regs + MCTL_MAIN_EN);
+
+   if (dsi->platform_ops && dsi->platform_ops->disable)
+   dsi->platform_ops->disable(dsi);
+
pm_runtime_put(dsi->base.dev);
 }
 
@@ -360,6 +368,9 @@ static void cdns_dsi_bridge_enable(struct drm_bridge 
*bridge)
if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
return;
 
+   if (dsi->platform_ops && dsi->platform_ops->enable)
+   dsi->platform_ops->enable(dsi);
+
mode = &bridge->encoder->crtc->state->adjusted_mode;
nlanes = output->dev->lanes;
 
@@ -800,6 +811,8 @@ static int cdns_dsi_drm_probe(struct platform_device *pdev)
goto err_disable_pclk;
}
 
+   dsi->platform_ops = of_device_get_match_data(&pdev->dev);
+
val = readl(dsi->regs + IP_CONF);
dsi->direct_cmd_fifo_depth = 1 << (DIRCMD_FIFO_DEPTH(val) + 2);
dsi->rx_fifo_depth = RX_FIFO_DEPTH(val);
@@ -835,14 +848,27 @@ static int cdns_dsi_drm_probe(struct platform_device 
*pdev)
dsi->base.dev = &pdev->dev;
dsi->base.ops = &cdns_dsi_ops;
 
+   if (dsi->platform_ops && dsi->platform_ops->init) {
+   ret = dsi->platform_ops->init(dsi);
+   if (ret != 0) {
+   dev_err(&pdev->dev, "platform initialization failed: 
%d\n",
+   ret);
+   goto err_disable_runtime_pm;
+   }
+   }
+
ret = mipi_dsi_host_register(&dsi->base);
if (ret)
-   goto err_disable_runtime_pm;
+   goto err_deinit_platform;
 
clk_disable_unprepare(dsi->dsi_p_clk);
 
return 0;
 
+err_deinit_platform:
+   if (dsi->platform_ops && dsi->platform_ops->deinit)
+   dsi->platform_ops->deinit(dsi);
+
 err_disable_runtime_pm:
pm_runtime_disable(&pdev->dev);
 
@@ -857,6 +883,10 @@ static int cdns_dsi_drm_remove(struct platform_device 
*pdev)
struct cdns_dsi *dsi = platform_get_drvdata(pdev);
 
mipi_ds

[PATCH v9 2/5] dt-bindings: display: bridge: cdns, dsi: Add compatible for dsi on j721e

Add compatible to support dsi bridge on j721e

Signed-off-by: Rahul T R 
Reviewed-by: Rob Herring 
---
 .../bindings/display/bridge/cdns,dsi.yaml | 25 ++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml 
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
index 3161c33093c1..23060324d16e 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
@@ -16,9 +16,15 @@ properties:
   compatible:
 enum:
   - cdns,dsi
+  - ti,j721e-dsi
 
   reg:
-maxItems: 1
+minItems: 1
+items:
+  - description:
+  Register block for controller's registers.
+  - description:
+  Register block for wrapper settings registers in case of TI J7 SoCs.
 
   clocks:
 items:
@@ -67,6 +73,23 @@ properties:
 allOf:
   - $ref: ../dsi-controller.yaml#
 
+  - if:
+  properties:
+compatible:
+  contains:
+const: ti,j721e-dsi
+then:
+  properties:
+reg:
+  minItems: 2
+  maxItems: 2
+power-domains:
+  maxItems: 1
+else:
+  properties:
+reg:
+  maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.38.0

[PATCH v9 4/5] drm/bridge: cdns-dsi: Create a header file

Create a header file for cdns dsi and move
register offsets and structure to header,
to prepare for adding j721e wrapper support

Signed-off-by: Rahul T R 
Reviewed-by: Tomi Valkeinen 
---
 .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 446 +
 .../gpu/drm/bridge/cadence/cdns-dsi-core.h| 458 ++
 2 files changed, 459 insertions(+), 445 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 20bece84ff8c..cba91247ab26 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -6,10 +6,7 @@
  */
 
 #include 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include 
 
@@ -23,448 +20,7 @@
 #include 
 #include 
 
-#include 
-#include 
-
-#define IP_CONF0x0
-#define SP_HS_FIFO_DEPTH(x)(((x) & GENMASK(30, 26)) >> 26)
-#define SP_LP_FIFO_DEPTH(x)(((x) & GENMASK(25, 21)) >> 21)
-#define VRS_FIFO_DEPTH(x)  (((x) & GENMASK(20, 16)) >> 16)
-#define DIRCMD_FIFO_DEPTH(x)   (((x) & GENMASK(15, 13)) >> 13)
-#define SDI_IFACE_32   BIT(12)
-#define INTERNAL_DATAPATH_32   (0 << 10)
-#define INTERNAL_DATAPATH_16   (1 << 10)
-#define INTERNAL_DATAPATH_8(3 << 10)
-#define INTERNAL_DATAPATH_SIZE ((x) & GENMASK(11, 10))
-#define NUM_IFACE(x)   x) & GENMASK(9, 8)) >> 8) + 1)
-#define MAX_LANE_NB(x) (((x) & GENMASK(7, 6)) >> 6)
-#define RX_FIFO_DEPTH(x)   ((x) & GENMASK(5, 0))
-
-#define MCTL_MAIN_DATA_CTL 0x4
-#define TE_MIPI_POLLING_EN BIT(25)
-#define TE_HW_POLLING_EN   BIT(24)
-#define DISP_EOT_GEN   BIT(18)
-#define HOST_EOT_GEN   BIT(17)
-#define DISP_GEN_CHECKSUM  BIT(16)
-#define DISP_GEN_ECC   BIT(15)
-#define BTA_EN BIT(14)
-#define READ_ENBIT(13)
-#define REG_TE_EN  BIT(12)
-#define IF_TE_EN(x)BIT(8 + (x))
-#define TVG_SELBIT(6)
-#define VID_EN BIT(5)
-#define IF_VID_SELECT(x)   ((x) << 2)
-#define IF_VID_SELECT_MASK GENMASK(3, 2)
-#define IF_VID_MODEBIT(1)
-#define LINK_ENBIT(0)
-
-#define MCTL_MAIN_PHY_CTL  0x8
-#define HS_INVERT_DAT(x)   BIT(19 + ((x) * 2))
-#define SWAP_PINS_DAT(x)   BIT(18 + ((x) * 2))
-#define HS_INVERT_CLK  BIT(17)
-#define SWAP_PINS_CLK  BIT(16)
-#define HS_SKEWCAL_EN  BIT(15)
-#define WAIT_BURST_TIME(x) ((x) << 10)
-#define DATA_ULPM_EN(x)BIT(6 + (x))
-#define CLK_ULPM_ENBIT(5)
-#define CLK_CONTINUOUS BIT(4)
-#define DATA_LANE_EN(x)BIT((x) - 1)
-
-#define MCTL_MAIN_EN   0xc
-#define DATA_FORCE_STOPBIT(17)
-#define CLK_FORCE_STOP BIT(16)
-#define IF_EN(x)   BIT(13 + (x))
-#define DATA_LANE_ULPM_REQ(l)  BIT(9 + (l))
-#define CLK_LANE_ULPM_REQ  BIT(8)
-#define DATA_LANE_START(x) BIT(4 + (x))
-#define CLK_LANE_ENBIT(3)
-#define PLL_START  BIT(0)
-
-#define MCTL_DPHY_CFG0 0x10
-#define DPHY_C_RSTBBIT(20)
-#define DPHY_D_RSTB(x) GENMASK(15 + (x), 16)
-#define DPHY_PLL_PDN   BIT(10)
-#define DPHY_CMN_PDN   BIT(9)
-#define DPHY_C_PDN BIT(8)
-#define DPHY_D_PDN(x)  GENMASK(3 + (x), 4)
-#define DPHY_ALL_D_PDN GENMASK(7, 4)
-#define DPHY_PLL_PSO   BIT(1)
-#define DPHY_CMN_PSO   BIT(0)
-
-#define MCTL_DPHY_TIMEOUT1 0x14
-#define HSTX_TIMEOUT(x)((x) << 4)
-#define HSTX_TIMEOUT_MAX   GENMASK(17, 0)
-#define CLK_DIV(x) (x)
-#define CLK_DIV_MAXGENMASK(3, 0)
-
-#define MCTL_DPHY_TIMEOUT2 0x18
-#define LPRX_TIMEOUT(x)(x)
-
-#define MCTL_ULPOUT_TIME   0x1c
-#define DATA_LANE_ULPOUT_TIME(x)   ((x) << 9)
-#define CLK_LANE_ULPOUT_TIME(x)(x)
-
-#define MCTL_3DVIDEO_CTL   0x20
-#define VID_VSYNC_3D_ENBIT(7)
-#define VID_VSYNC_3D_LRBIT(5)
-#define VID_VSYNC_3D_SECOND_EN BIT(4)
-#define VID_VSYNC_3DFORMAT_LINE(0 << 2)
-#define VID_VSYNC_3DFORMAT_FRAME   (1 << 2)
-#define VID_VSYNC_3DFORMAT_PIXEL   (2 << 2)
-#define VID_VSYNC_3DMODE_OFF   0
-#define VID_VSYN

[PATCH v9 0/5] Add support for CDNS DSI J721E wrapper

Following series of patches adds supports for CDNS DSI
bridge on j721e.

v9:
 - Fixed below based on review comments in v8
 - Added more info on wrapper in the commit message
 - Fixed the description in Kconfig
 - Fixed the formatting of of_match table
 - exit -> deinit in platform ops
 - Remove duplicate of struct declaration in cdns-dsi-j721e.h

v8:
 - Rebased to 6.1-rc1

v7:
 - Rebased to next-20220920
 - Accumulated the Reviewed-by acks

v6:
 - Dropped generic definations for properties like reg, resets etc..
 - Fixed the defination for port@0 and port@1
 - removed the ti,sn65dsi86 node from the example, which is not related

v5:
 - Remove power-domain property in the conversion commit
 - Add power-domain only for j721e compatible
 - Fix white space error in one of the commit

v4:
 - split conversion txt to yaml
 - seperate commit for addinig new compatible
 - conditionally limit the items for reg property, based on the compatible

v3:
 - Convert cdns-dsi.txt binding to yaml
 - Move the bridge under display/bridge/cadence
 - Add new compatible to enable the wrapper module

v2:
 - Moved setting DPI0 to bridge_enable, since it
   should be done after pm_runtime_get

Rahul T R (5):
  dt-bindings: display: bridge: Convert cdns,dsi.txt to yaml
  dt-bindings: display: bridge: cdns,dsi: Add compatible for dsi on
j721e
  drm/bridge: cdns-dsi: Move to drm/bridge/cadence
  drm/bridge: cdns-dsi: Create a header file
  drm/bridge: cdns-dsi: Add support for J721E wrapper

 .../bindings/display/bridge/cdns,dsi.txt  | 112 
 .../bindings/display/bridge/cdns,dsi.yaml | 180 +++
 drivers/gpu/drm/bridge/Kconfig|  11 -
 drivers/gpu/drm/bridge/Makefile   |   1 -
 drivers/gpu/drm/bridge/cadence/Kconfig|  21 +
 drivers/gpu/drm/bridge/cadence/Makefile   |   3 +
 .../{cdns-dsi.c => cadence/cdns-dsi-core.c}   | 481 ++
 .../gpu/drm/bridge/cadence/cdns-dsi-core.h| 471 +
 .../gpu/drm/bridge/cadence/cdns-dsi-j721e.c   |  51 ++
 .../gpu/drm/bridge/cadence/cdns-dsi-j721e.h   |  16 +
 10 files changed, 777 insertions(+), 570 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
 rename drivers/gpu/drm/bridge/{cdns-dsi.c => cadence/cdns-dsi-core.c} (65%)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-j721e.h

-- 
2.38.0

[PATCH v9 1/5] dt-bindings: display: bridge: Convert cdns, dsi.txt to yaml

Convert cdns,dsi.txt binding to yaml format

Signed-off-by: Rahul T R 
Reviewed-by: Rob Herring 
---
 .../bindings/display/bridge/cdns,dsi.txt  | 112 -
 .../bindings/display/bridge/cdns,dsi.yaml | 157 ++
 2 files changed, 157 insertions(+), 112 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt 
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
deleted file mode 100644
index 525a4bfd8634..
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
+++ /dev/null
@@ -1,112 +0,0 @@
-Cadence DSI bridge
-==
-
-The Cadence DSI bridge is a DPI to DSI bridge supporting up to 4 DSI lanes.
-
-Required properties:
-- compatible: should be set to "cdns,dsi".
-- reg: physical base address and length of the controller's registers.
-- interrupts: interrupt line connected to the DSI bridge.
-- clocks: DSI bridge clocks.
-- clock-names: must contain "dsi_p_clk" and "dsi_sys_clk".
-- phys: phandle link to the MIPI D-PHY controller.
-- phy-names: must contain "dphy".
-- #address-cells: must be set to 1.
-- #size-cells: must be set to 0.
-
-Optional properties:
-- resets: DSI reset lines.
-- reset-names: can contain "dsi_p_rst".
-
-Required subnodes:
-- ports: Ports as described in Documentation/devicetree/bindings/graph.txt.
-  2 ports are available:
-  * port 0: this port is only needed if some of your DSI devices are
-   controlled through  an external bus like I2C or SPI. Can have at
-   most 4 endpoints. The endpoint number is directly encoding the
-   DSI virtual channel used by this device.
-  * port 1: represents the DPI input.
-  Other ports will be added later to support the new kind of inputs.
-
-- one subnode per DSI device connected on the DSI bus. Each DSI device should
-  contain a reg property encoding its virtual channel.
-
-Example:
-   dsi0: dsi@fd0c {
-   compatible = "cdns,dsi";
-   reg = <0x0 0xfd0c 0x0 0x1000>;
-   clocks = <&pclk>, <&sysclk>;
-   clock-names = "dsi_p_clk", "dsi_sys_clk";
-   interrupts = <1>;
-   phys = <&dphy0>;
-   phy-names = "dphy";
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   ports {
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   port@1 {
-   reg = <1>;
-   dsi0_dpi_input: endpoint {
-   remote-endpoint = <&xxx_dpi_output>;
-   };
-   };
-   };
-
-   panel: dsi-dev@0 {
-   compatible = "";
-   reg = <0>;
-   };
-   };
-
-or
-
-   dsi0: dsi@fd0c {
-   compatible = "cdns,dsi";
-   reg = <0x0 0xfd0c 0x0 0x1000>;
-   clocks = <&pclk>, <&sysclk>;
-   clock-names = "dsi_p_clk", "dsi_sys_clk";
-   interrupts = <1>;
-   phys = <&dphy1>;
-   phy-names = "dphy";
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   ports {
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   port@0 {
-   reg = <0>;
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   dsi0_output: endpoint@0 {
-   reg = <0>;
-   remote-endpoint = <&dsi_panel_input>;
-   };
-   };
-
-   port@1 {
-   reg = <1>;
-   dsi0_dpi_input: endpoint {
-   remote-endpoint = <&xxx_dpi_output>;
-   };
-   };
-   };
-   };
-
-   i2c@xxx {
-   panel: panel@59 {
-   compatible = "";
-   reg = <0x59>;
-
-   port {
-   dsi_panel_input: endpoint {
-   remote-endpoint = <&dsi0_output>;
-   };
-   };
-   };
-   };
diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml 
b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
new file mode 100644
index ..3161c33093c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
@@ -0,0 +1,157 @@
+# SPDX-License-I

Re: [PATCH v2 08/21] drm/rockchip: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as rockchip uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 07/21] drm/logicvc: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as logicvc uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 00/10] Connect VFIO to IOMMUFD

2022-10-31 Thread Jason Gunthorpe

On Mon, Oct 31, 2022 at 06:38:45PM +0800, Yi Liu wrote:
> Hi Jason,
> 
> On 2022/10/26 02:17, Jason Gunthorpe wrote:
> > This series provides an alternative container layer for VFIO implemented
> > using iommufd. This is optional, if CONFIG_IOMMUFD is not set then it will
> > not be compiled in.
> > 
> > At this point iommufd can be injected by passing in a iommfd FD to
> > VFIO_GROUP_SET_CONTAINER which will use the VFIO compat layer in iommufd
> > to obtain the compat IOAS and then connect up all the VFIO drivers as
> > appropriate.
> > 
> > This is temporary stopping point, a following series will provide a way to
> > directly open a VFIO device FD and directly connect it to IOMMUFD using
> > native ioctls that can expose the IOMMUFD features like hwpt, future
> > vPASID and dynamic attachment.
> > 
> > This series, in compat mode, has passed all the qemu tests we have
> > available, including the test suites for the Intel GVT mdev. Aside from
> > the temporary limitation with P2P memory this is belived to be fully
> > compatible with VFIO.
> > 
> > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd
> 
> In our side, we found the gvt-g test failed. Guest i915 driver stuck at
> init phase. While with your former version  (commit ID
> a249441ba6fd9d658f4a1b568453e3a742d12686), gvt-g test is passing. 

Oh, I didn't realize you grabbed such an older version for this testing..

> noticed there a quite a few change in iommufd/pages.c from last version.
> We are internally tracing in the gvt-g side, may also good to have your
> attention.

syzkaller just ran into this that I was starting to investigate:

@@ -1505,7 +1505,7 @@ int iopt_pages_fill_xarray(struct iopt_pages *pages, 
unsigned long start_index,
int rc;
 
pfn_reader_user_init(&user, pages);
-   user.upages_len = last_index - start_index + 1;
+   user.upages_len = (last_index - start_index + 1) * sizeof(*out_pages);
interval_tree_for_each_double_span(&span, &pages->access_itree,

It would certainly hit gvt - but you should get WARN_ON's not hangs

There is something wrong with the test suite that it isn't covering
the above, I'm going to look into that today.

Jason

Re: [PATCH v2 06/21] drm/ingenic: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as ingenic uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian, Sergey)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 05/21] drm/imx/dcss: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as DCSS uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 04/21] drm/amdgpu: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as amdgpu uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 04/21] drm/amdgpu: Don't set struct drm_driver.output_poll_changed

On 10/24/22 13:19, Thomas Zimmermann wrote:
> Don't set struct drm_driver.output_poll_changed. It's used to restore
> the fbdev console. But as amdgpu uses generic fbdev emulation, the
> console is being restored by the DRM client helpers already. See the
> functions drm_kms_helper_hotplug_event() and
> drm_kms_helper_connector_hotplug_event() in drm_probe_helper.c.
> 
> v2:
>   * fix commit description (Christian)
> 
> Signed-off-by: Thomas Zimmermann 
> ---

Reviewed-by: Javier Martinez Canillas 

Do you think that the fbdev helpers kernel doc has to be updated to mention
that drm_fb_helper_lastclose() and drm_fb_helper_output_poll_changed() are
not needed when generic fbdev emulation is used? Because by reading that is
not clear that's the case:

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L86

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] staging: fbtft: Use ARRAY_SIZE() to get argument count

2022-10-31 Thread Julia Lawall




On Mon, 31 Oct 2022, Deepak R Varma wrote:

> On Mon, Oct 31, 2022 at 12:41:40PM +0530, Deepak Varma wrote:
> > On Sat, Oct 29, 2022 at 07:34:26PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Sat, 29 Oct 2022, Deepak R Varma wrote:
> > >
> > > > On Sat, Oct 29, 2022 at 09:32:50AM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Oct 28, 2022 at 07:00:05PM +0530, Deepak R Varma wrote:
> > > > > > The ARRAY_SIZE(foo) macro should be preferred over sizeof operator
> > > > > > based computation such as sizeof(foo)/sizeof(foo[0]) for finding
> > > > > > number of elements in an array. Issue identified using coccicheck.
> > > > > >
> > > > > > Signed-off-by: Deepak R Varma 
> > > > > > ---
> > > > > >  drivers/staging/fbtft/fbtft.h | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/fbtft/fbtft.h 
> > > > > > b/drivers/staging/fbtft/fbtft.h
> > > > > > index 2c2b5f1c1df3..5506a473be91 100644
> > > > > > --- a/drivers/staging/fbtft/fbtft.h
> > > > > > +++ b/drivers/staging/fbtft/fbtft.h
> > > > > > @@ -231,7 +231,7 @@ struct fbtft_par {
> > > > > > bool polarity;
> > > > > >  };
> > > > > >
> > > > > > -#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
> > > > > > +#define NUMARGS(...)  ARRAY_SIZE(((int[]){ __VA_ARGS__ }))
> > > > >
> > > > > Please please please test-build your patches before sending them out.
> > > > > To not do so just wastes reviewer resources :(
> > > >
> > > > Hello Greg,
> > > > I did build the .ko files by making the driver/staging/fbtft/ path. I 
> > > > verified
> > > > .o and .ko files were built.
> > > >
> > > > I did a make clean just now and was again able to rebuild without any 
> > > > errors.
> > > > Please see the attached log file.
> > > >
> > > > Is there something wrong with the way I am firing the build?
> > >
> > > The change is in the definition of a macro.  The compiler won't help you
> > > in this case unless the macro is actually used in code that is compiled.
> > > Find the uses and check for any nearby ifdefs.  For file foo.c you can
> > > also do make foo.i to see the result of reducing ifdef and expanding
> > > macros.  Then you can see if the code you changed is actually included in
> > > the build.
> >
> > Okay. This is helpful. I understand. Looking into the file where the macro
> > expansion is reported to be failed.
>
> Hi Julia,
> I could see the macro expansions in the .i files for the fbtft-core.c and
> fb_hx8353d.c file. I am not sure why it built successfully on my x86 though. 
> The
> error in Kerbel bot seems to be specific to ARM arch. I will try that later
> today. I am on the right track to the build error triage?
>
> Also, while reviewing the macro expansion, I saw change in the computation 
> that
> seems odd to me. In the denominator of the expanded macro, there is a "+
> ((int)" computation that I am not sure if is result of ARRAY_SIZE. I have
> attached the old anf the new .i file diff for your review. If you get a change
> could you help me understand why this additional computation is added to the
> denominator?

I took a look, but it's pretty complex.  You could take the code and
reorganize it so that it is more readable, and then take the definition of
the ARRAY_SIZE macro, to better see what is going on.

julia

>
> Thank you,
> ./drv
> >
> > Thank you,
> > ./drv
> >
> > >
> > > julia
> > >
> >
> >
> >
>
>
>

Re: [PATCH v2 03/21] drm/vboxvideo: Don't set struct drm_driver.lastclose