Re: [PATCH v2 16/17] drm: Nuke mode->private_flags

2020-04-03 Thread abhinavk

Hi Ville

Thanks for the patch.

Our understanding of private_flags was that we can use it within our 
drivers to handle vendor specific features.
Hence we do have several features in our downstream drivers as well as 
some planned work based on this.


This was the only method to pass around and consume the driver only 
information which we have been using.


In the current qualcomm upstream display drivers, the only usage of the 
mode->private_flags is what you have removed in 
https://patchwork.kernel.org/patch/11473497/.


However, for other projects which do not use upstream drivers yet, we 
have several features already which are using the mode->private_flags.


We do have a plan to remove the usage of mode->private_flags for those 
drivers as well but its not ready yet.


These downstream drivers still use the upstream drm files for 
compilation.


So how it works is we use all the headers under include/drm and also the 
files under drivers/gpu/drm as-it-is from upstream but maintain our 
drivers on top of this.


Removing this will result in compilation failures for us in the near 
term.


Can we keep this one as-it-is and when our changes are ready to post it 
upstream we shall remove private_flags from the drm_modes.h


Thanks

Abhinav

On 2020-04-03 13:40, Ville Syrjala wrote:

From: Ville Syrjälä 

The last two uses of mode->private_flags (in i915 and gma500)
are now gone. So let's remove mode->private_flags entirely.

CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 47d62b9d8d20..1e97138a9b8c 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -348,16 +348,6 @@ struct drm_display_mode {
 */
u8 type;

-   /**
-* @private_flags:
-*
-* Driver private flags. private_flags can only be used for mode
-	 * objects passed to drivers in modeset operations. It shouldn't be 
used

-* by atomic drivers since they can store any additional data by
-* subclassing state structures.
-*/
-   u8 private_flags;
-
/**
 * @head:
 *

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Laurent Pinchart
Hi Sakari,

On Sat, Apr 04, 2020 at 03:14:25AM +0300, Sakari Ailus wrote:
> On Fri, Apr 03, 2020 at 09:32:42PM +0300, Andy Shevchenko wrote:
> > On Fri, Apr 3, 2020 at 8:54 PM Joe Perches  wrote:
> > > On Fri, 2020-04-03 at 19:32 +0200, Mauro Carvalho Chehab wrote:
> > > > Em Fri, 03 Apr 2020 09:56:42 -0700
> > > > Joe Perches  escreveu:
> > 
> > > It _might_ be useful to use a CONFIG_MEDIA_SUPPORT guard
> > > in lib/vsprintf for this.
> > 
> > No need. FourCC, if Sakari makes it more generic, can be used for
> > other purposes, e.g. printing component names from the chips (not
> > related to media at all).
> 
> Could you elaborate?
> 
> This could be already used on DRM, presumably, and that does not depend on
> CONFIG_MEDIA_SUPPORT. I don't know how much there would be a need for that,
> though, but this remains a possibility.

/**
 * drm_get_format_name - fill a string with a drm fourcc format's name
 * @format: format to compute name of
 * @buf: caller-supplied buffer
 */
const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf 
*buf)
{
snprintf(buf->str, sizeof(buf->str),
 "%c%c%c%c %s-endian (0x%08x)",
 printable_char(format & 0xff),
 printable_char((format >> 8) & 0xff),
 printable_char((format >> 16) & 0xff),
 printable_char((format >> 24) & 0x7f),
 format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
 format);

return buf->str;
}
EXPORT_SYMBOL(drm_get_format_name);

I'm not advocating for one approach or the other in this case, but we
should standardize 4CC printing between the two subsystems.

-- 
Regards,

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Sakari Ailus
Hi Andy,

On Fri, Apr 03, 2020 at 09:32:42PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 3, 2020 at 8:54 PM Joe Perches  wrote:
> > On Fri, 2020-04-03 at 19:32 +0200, Mauro Carvalho Chehab wrote:
> > > Em Fri, 03 Apr 2020 09:56:42 -0700
> > > Joe Perches  escreveu:
> 
> > It _might_ be useful to use a CONFIG_MEDIA_SUPPORT guard
> > in lib/vsprintf for this.
> 
> No need. FourCC, if Sakari makes it more generic, can be used for
> other purposes, e.g. printing component names from the chips (not
> related to media at all).

Could you elaborate?

This could be already used on DRM, presumably, and that does not depend on
CONFIG_MEDIA_SUPPORT. I don't know how much there would be a need for that,
though, but this remains a possibility.

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Joe Perches
On Sat, 2020-04-04 at 02:36 +0300, Sakari Ailus wrote:
> Hi Joe,

Hi Sakari.

> How would these be different in functionality? fmt[2] won't be accessed if
> fmt[1] is not 'c' (including '\0'), just like on the line above. I find the
> original easier to read.

Oops. You are right of course.

cheers, Joe

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Sakari Ailus
Hi Joe,

On Fri, Apr 03, 2020 at 11:38:29AM -0700, Joe Perches wrote:
> On Fri, 2020-04-03 at 13:47 +0300, Sakari Ailus wrote:
> > On Fri, Apr 03, 2020 at 01:24:49PM +0300, Laurent Pinchart wrote:
> > > On Fri, Apr 03, 2020 at 12:11:56PM +0300, Sakari Ailus wrote:
> > > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > > the same implementation can be used.
> []
> > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> []
> > > > +static noinline_for_stack
> > > > +char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> > > > +   struct printf_spec spec, const char *fmt)
> []
> > > > +   if (fmt[1] != 'c' || fmt[2] != 'c')
> 
> This could check outside a format string if
> the %p4 is at the end of the format string.
> 
>   printk("%p4", fourcc)
> 
> So this should verify
> 
>   if (!(fmt[1] == 'c' && fmt[2] == 'c'))

How would these be different in functionality? fmt[2] won't be accessed if
fmt[1] is not 'c' (including '\0'), just like on the line above. I find the
original easier to read.

-- 
Regards,

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


[PATCH] drm/i915/selftest: Create mock_destroy_device

2020-04-03 Thread Daniel Vetter
Just some prep work before we rework the lifetime handling, which
requires replacing all the drm_dev_put in selftests by something else.

v2: Don't go with a static inline, upsets the header tests and
separation.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/gem/selftests/huge_pages.c   | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c| 2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_evict.c   | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c | 2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
 drivers/gpu/drm/i915/selftests/intel_memory_region.c  | 2 +-
 drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 7 ++-
 drivers/gpu/drm/i915/selftests/mock_gem_device.h  | 2 ++
 13 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 2d0fd50c5312..d19bb011fc6b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1789,7 +1789,7 @@ int i915_gem_huge_page_mock_selftests(void)
i915_vm_put(>vm);
 
 out_unlock:
-   drm_dev_put(_priv->drm);
+   mock_destroy_device(dev_priv);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index f4f933240b39..d9d96d23e37e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1986,7 +1986,7 @@ int i915_gem_context_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 2a52b92586b9..0845ce1ae37c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -272,7 +272,7 @@ int i915_gem_dmabuf_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
index 2b6db6f799de..085644edebfc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
@@ -85,7 +85,7 @@ int i915_gem_object_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
index 34932871b3a5..2a9709eb5a42 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
@@ -73,6 +73,6 @@ int i915_gem_phys_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c 
b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index c2578a0f2f14..1c0865227714 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -157,7 +157,7 @@ static int mock_hwsp_freelist(void *arg)
__mock_hwsp_record(, na, NULL);
kfree(state.history);
 err_put:
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 028baae9631f..f88473d396f4 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -536,7 +536,7 @@ int i915_gem_evict_mock_selftests(void)
with_intel_runtime_pm(>runtime_pm, wakeref)
err = i915_subtests(tests, >gt);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 5d2a02fcf595..035e4f77f06f 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1714,7 +1714,7 @@ int i915_gem_gtt_mock_selftests(void)
mock_fini_ggtt(ggtt);
kfree(ggtt);
 out_put:
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c 

[PATCH] drm/i915/selftests: align more to real device lifetimes

2020-04-03 Thread Daniel Vetter
The big change is device_add so that device_del can auto-cleanup
devres resources. This allows us to use devm_drm_dev_alloc, which
removes the last user of drm_dev_init.

v2: Rebased

Signed-off-by: Daniel Vetter 
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 33 +--
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 41afc357f4d0..1ab97fa55929 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -123,12 +123,6 @@ struct drm_i915_private *mock_gem_device(void)
pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
if (!pdev)
return NULL;
-   i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-   if (!i915) {
-   kfree(pdev);
-   return NULL;
-   }
-
device_initialize(>dev);
pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
pdev->dev.release = release_dev;
@@ -139,8 +133,23 @@ struct drm_i915_private *mock_gem_device(void)
/* hack to disable iommu for the fake device; force identity mapping */
pdev->dev.archdata.iommu = (void *)-1;
 #endif
+   err = device_add(>dev);
+   if (err) {
+   kfree(pdev);
+   return NULL;
+   }
+
+   i915 = devm_drm_dev_alloc(>dev, _driver,
+ struct drm_i915_private, drm);
+   if (err) {
+   pr_err("Failed to allocate mock GEM device: err=%d\n", err);
+   put_device(>dev);
+
+   return NULL;
+   }
 
pci_set_drvdata(pdev, i915);
+   i915->drm.pdev = pdev;
 
dev_pm_domain_set(>dev, _domain);
pm_runtime_enable(>dev);
@@ -148,16 +157,6 @@ struct drm_i915_private *mock_gem_device(void)
if (pm_runtime_enabled(>dev))
WARN_ON(pm_runtime_get_sync(>dev));
 
-   err = drm_dev_init(>drm, _driver, >dev);
-   if (err) {
-   pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-   put_device(>dev);
-   kfree(i915);
-
-   return NULL;
-   }
-   i915->drm.pdev = pdev;
-   drmm_add_final_kfree(>drm, i915);
 
intel_runtime_pm_init_early(>runtime_pm);
 
@@ -221,5 +220,5 @@ struct drm_i915_private *mock_gem_device(void)
 
 void mock_destroy_device(struct drm_i915_private *i915)
 {
-   drm_dev_put(>drm);
+   device_del(i915->drm.dev);
 }
-- 
2.25.1

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


[PATCH] drm/dp_mst: Remove drm_dp_mst_has_audio()

2020-04-03 Thread Lyude Paul
Drive-by fix I noticed the other day - drm_dp_mst_has_audio() only ever
made sense back when we still had to validate ports before accessing
them in order to (attempt to) avoid NULL dereferences. Since we have
proper reference counting that guarantees we always can safely access
the MST port, there's no use in keeping this function around as all it
does is validate the port pointer before checking the audio status.

Note - drm_dp_mst_port->has_audio is technically protected by
drm_device->mode_config.connection_mutex, since it's only ever updated
from drm_dp_mst_get_edid(). Additionally, we change the declaration for
port in struct intel_connector to be properly typed, so we can directly
access it.

Cc: "Lee, Shawn C" 
Cc: Sean Paul 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 21 ---
 .../drm/i915/display/intel_display_debugfs.c  | 10 ++---
 .../drm/i915/display/intel_display_types.h|  2 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  3 +--
 include/drm/drm_dp_mst_helper.h   |  2 --
 5 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1ff49547b2e8..129126091e90 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4063,27 +4063,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_dp_mst_detect_port);
 
-/**
- * drm_dp_mst_port_has_audio() - Check whether port has audio capability or not
- * @mgr: manager for this port
- * @port: unverified pointer to a port.
- *
- * This returns whether the port supports audio or not.
- */
-bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
-   struct drm_dp_mst_port *port)
-{
-   bool ret = false;
-
-   port = drm_dp_mst_topology_get_port_validated(mgr, port);
-   if (!port)
-   return ret;
-   ret = port->has_audio;
-   drm_dp_mst_topology_put_port(port);
-   return ret;
-}
-EXPORT_SYMBOL(drm_dp_mst_port_has_audio);
-
 /**
  * drm_dp_mst_get_edid() - get EDID for an MST port
  * @connector: toplevel connector to get EDID for
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 424f4e52f783..9f736420d83f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -631,15 +631,9 @@ static void intel_dp_info(struct seq_file *m,
 }
 
 static void intel_dp_mst_info(struct seq_file *m,
- struct intel_connector *intel_connector)
+ struct intel_connector *intel_connector)
 {
-   struct intel_encoder *intel_encoder = 
intel_attached_encoder(intel_connector);
-   struct intel_dp_mst_encoder *intel_mst =
-   enc_to_mst(intel_encoder);
-   struct intel_digital_port *intel_dig_port = intel_mst->primary;
-   struct intel_dp *intel_dp = _dig_port->dp;
-   bool has_audio = drm_dp_mst_port_has_audio(_dp->mst_mgr,
-   intel_connector->port);
+   bool has_audio = intel_connector->port->has_audio;
 
seq_printf(m, "\taudio support: %s\n", yesno(has_audio));
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 2bedd626c686..1de7bef0a49b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -436,7 +436,7 @@ struct intel_connector {
   state of connector->polled in case hotplug storm detection changes 
it */
u8 polled;
 
-   void *port; /* store this opaque as its illegal to dereference it */
+   struct drm_dp_mst_port *port;
 
struct intel_dp *mst_port;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 61605eb8c2af..c35efc9e628d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -114,8 +114,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder 
*encoder,
 
if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
pipe_config->has_audio =
-   drm_dp_mst_port_has_audio(_dp->mst_mgr,
- connector->port);
+   connector->port->has_audio;
else
pipe_config->has_audio =
intel_conn_state->force_audio == HDMI_AUDIO_ON;
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7af51c947b81..2d7c26592c05 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -732,8 +732,6 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
   struct drm_dp_mst_topology_mgr *mgr,

Re: [PATCH v2 03/17] drm: Nuke mode->vrefresh

2020-04-03 Thread Laurent Pinchart
Hi Ville,

Thank you for the patch.

On Fri, Apr 03, 2020 at 11:39:54PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Get rid of mode->vrefresh and just calculate it on demand. Saves
> a bit of space and avoids the cached value getting out of sync
> with reality.
> 
> Mostly done with cocci, with the following manual fixups:
> - Remove the now empty loop in drm_helper_probe_single_connector_modes()
> - Fix __MODE() macro in ch7006_mode.c
> - Fix DRM_MODE_ARG() macro in drm_modes.h
> - Remove leftover comment from samsung_s6d16d0_mode
> - Drop the TODO
> 
> @@
> @@
> struct drm_display_mode {
>   ...
> - int vrefresh;
>   ...
> };
> 
> @@
> identifier N;
> expression E;
> @@
> struct drm_display_mode N = {
> - .vrefresh = E
> };
> 
> @@
> identifier N;
> expression E;
> @@
> struct drm_display_mode N[...] = {
> ...,
> {
> - .vrefresh = E
> }
> ,...
> };
> 
> @@
> expression E;
> @@
> {
>   DRM_MODE(...),
> - .vrefresh = E,
> }
> 
> @@
> identifier M, R;
> @@
> int drm_mode_vrefresh(const struct drm_display_mode *M)
> {
>   ...
> - if (M->vrefresh > 0)
> - R = M->vrefresh;
> - else
>   if (...) {
>   ...
>   }
>   ...
> }
> 
> @@
> struct drm_display_mode *p;
> expression E;
> @@
> (
> - p->vrefresh = E;
> |
> - p->vrefresh
> + drm_mode_vrefresh(p)
> )
> 
> @@
> struct drm_display_mode s;
> expression E;
> @@
> (
> - s.vrefresh = E;
> |
> - s.vrefresh
> + drm_mode_vrefresh()
> )
> 
> @@
> expression E;
> @@
> - drm_mode_vrefresh(E) ? drm_mode_vrefresh(E) : drm_mode_vrefresh(E)
> + drm_mode_vrefresh(E)
> 
> @find_substruct@
> identifier X;
> identifier S;
> @@
> struct X {
> ...
>   struct drm_display_mode S;
> ...
> };
> 
> @@
> identifier find_substruct.S;
> expression E;
> identifier I;
> @@
> {
> .S = {
> - .vrefresh = E
> }
> }
> 
> @@
> identifier find_substruct.S;
> identifier find_substruct.X;
> expression E;
> identifier I;
> @@
> struct X I[...] = {
> ...,
> .S = {
> - .vrefresh = E
> }
> ,...
> };
> 
> v2: Drop TODO
> 
> Cc: Andrzej Hajda 
> Cc: Neil Armstrong 
> Cc: Laurent Pinchart 
> Cc: Jonas Karlman 
> Cc: Jernej Skrabec 
> Cc: Inki Dae 
> Cc: Joonyoung Shim 
> Cc: Seung-Woo Kim 
> Cc: Kyungmin Park 
> Cc: Linus Walleij 
> Cc: CK Hu 
> Cc: Philipp Zabel 
> Cc: Ben Skeggs 
> Cc: Thierry Reding 
> Cc: Sam Ravnborg 
> Cc: Jerry Han 
> Cc: Icenowy Zheng 
> Cc: Jagan Teki 
> Cc: Stefan Mavrodiev 
> Cc: Robert Chiras 
> Cc: "Guido Günther" 
> Cc: Purism Kernel Team 
> Cc: Benjamin Gaignard 
> Cc: Vincent Abriou 
> Cc: VMware Graphics 
> Cc: Thomas Hellstrom 
> Cc: linux-amlo...@lists.infradead.org
> Cc: nouv...@lists.freedesktop.org
> Reviewed-by: Emil Velikov 
> Reviewed-by: Sam Ravnborg 
> Acked-by: Linus Walleij 
> Signed-off-by: Ville Syrjälä 
> ---
>  Documentation/gpu/todo.rst|  20 --
>  drivers/gpu/drm/bridge/sii902x.c  |   2 +-
>  drivers/gpu/drm/drm_client_modeset.c  |   2 +-
>  drivers/gpu/drm/drm_edid.c| 328 +-
>  drivers/gpu/drm/drm_modes.c   |   9 +-
>  drivers/gpu/drm/drm_probe_helper.c|   3 -
>  drivers/gpu/drm/exynos/exynos_hdmi.c  |   5 +-
>  drivers/gpu/drm/exynos/exynos_mixer.c |   2 +-
>  drivers/gpu/drm/i2c/ch7006_mode.c |   1 -
>  drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>  .../drm/i915/display/intel_display_debugfs.c  |   4 +-
>  drivers/gpu/drm/i915/display/intel_dp.c   |  10 +-
>  drivers/gpu/drm/i915/display/intel_tv.c   |   3 -
>  drivers/gpu/drm/mcde/mcde_dsi.c   |   6 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c   |   4 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c   |   2 +-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c   |   2 -
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |   5 +-
>  drivers/gpu/drm/panel/panel-arm-versatile.c   |   4 -
>  drivers/gpu/drm/panel/panel-boe-himax8279d.c  |   3 +-
>  .../gpu/drm/panel/panel-boe-tv101wum-nl6.c|   6 +-
>  drivers/gpu/drm/panel/panel-elida-kd35t133.c  |   3 +-
>  .../gpu/drm/panel/panel-feixin-k101-im2ba02.c |   3 +-
>  .../drm/panel/panel-feiyang-fy07024di26a30d.c |   3 +-
>  drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |   7 -
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c |   3 +-
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c |   4 +-
>  .../gpu/drm/panel/panel-jdi-lt070me05000.c|   3 +-
>  .../drm/panel/panel-kingdisplay-kd097d04.c|   3 +-
>  .../drm/panel/panel-leadtek-ltk500hd1829.c|   3 +-
>  drivers/gpu/drm/panel/panel-lg-lb035q02.c |   1 -
>  drivers/gpu/drm/panel/panel-lg-lg4573.c   |   3 +-
>  drivers/gpu/drm/panel/panel-nec-nl8048hl11.c  |   1 -
>  drivers/gpu/drm/panel/panel-novatek-nt35510.c |   1 -
>  drivers/gpu/drm/panel/panel-novatek-nt39016.c |   1 -
>  .../drm/panel/panel-olimex-lcd-olinuxino.c|   1 -
>  .../gpu/drm/panel/panel-orisetech-otm8009a.c  |   3 +-
>  .../drm/panel/panel-osd-osd101t2587-53ts.c|   3 +-
>  

[PATCH v2 14/17] drm/i915: Replace I915_MODE_FLAG_INHERITED with a boolean

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

There's no reason for I915_MODE_FLAG_INHERITED to exist as a flag
anymore. Just make it a boolean.

CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 15 ++-
 .../gpu/drm/i915/display/intel_display_types.h|  2 +-
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index 5863e339a426..2deafaa9ec74 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -249,10 +249,10 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->update_wm_post = false;
crtc_state->fifo_changed = false;
crtc_state->preload_luts = false;
+   crtc_state->inherited = false;
crtc_state->wm.need_postvbl_update = false;
crtc_state->fb_bits = 0;
crtc_state->update_planes = 0;
-   crtc_state->mode_flags &= ~I915_MODE_FLAG_INHERITED;
 
return _state->uapi;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index d88cade45c35..550369444811 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6413,8 +6413,7 @@ static bool hsw_post_update_enable_ips(const struct 
intel_crtc_state *old_crtc_s
 * We can't read out IPS on broadwell, assume the worst and
 * forcibly enable IPS on the first fastset.
 */
-   if (new_crtc_state->update_pipe &&
-   old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
+   if (new_crtc_state->update_pipe && old_crtc_state->inherited)
return true;
 
return !old_crtc_state->ips_enabled;
@@ -13516,8 +13515,7 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
bool ret = true;
u32 bp_gamma = 0;
bool fixup_inherited = fastset &&
-   (current_config->mode_flags & I915_MODE_FLAG_INHERITED) &&
-   !(pipe_config->mode_flags & I915_MODE_FLAG_INHERITED);
+   current_config->inherited && !pipe_config->inherited;
 
if (fixup_inherited && !fastboot_enabled(dev_priv)) {
drm_dbg_kms(_priv->drm,
@@ -14667,10 +14665,9 @@ static int intel_atomic_check(struct drm_device *dev,
int ret, i;
bool any_ms = false;
 
-   /* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
-   if (new_crtc_state->mode_flags != old_crtc_state->mode_flags)
+   if (new_crtc_state->inherited != old_crtc_state->inherited)
new_crtc_state->uapi.mode_changed = true;
}
 
@@ -15016,7 +15013,7 @@ static void intel_update_crtc(struct intel_atomic_state 
*state,
 * of enabling them on the CRTC's first fastset.
 */
if (new_crtc_state->update_pipe && !modeset &&
-   old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
+   old_crtc_state->inherited)
intel_crtc_arm_fifo_underrun(crtc, new_crtc_state);
 }
 
@@ -17494,7 +17491,7 @@ static int intel_initial_commit(struct drm_device *dev)
 * happen only for the first real commit from userspace.
 * So preserve the inherited flag for the time being.
 */
-   crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
+   crtc_state->inherited = true;
 
ret = drm_atomic_add_affected_planes(state, 
>base);
if (ret)
@@ -18266,7 +18263,7 @@ static void intel_modeset_readout_hw_state(struct 
drm_device *dev)
 * set a flag to indicate that a full recalculation is
 * needed on the next commit.
 */
-   crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
+   crtc_state->inherited = true;
 
intel_crtc_compute_pixel_rate(crtc_state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 26df856f8b72..f529b14fbb2a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -642,7 +642,6 @@ struct intel_crtc_scaler_state {
 };
 
 /* {crtc,crtc_state}->mode_flags */
-#define I915_MODE_FLAG_INHERITED (1<<0)
 /* Flag to get scanline using frame time stamps */
 #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
 /* Flag to use the scanline counter instead of the pixel counter */
@@ -837,6 +836,7 @@ struct intel_crtc_state {
bool update_wm_pre, update_wm_post; /* 

[PATCH v2 16/17] drm: Nuke mode->private_flags

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

The last two uses of mode->private_flags (in i915 and gma500)
are now gone. So let's remove mode->private_flags entirely.

CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 47d62b9d8d20..1e97138a9b8c 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -348,16 +348,6 @@ struct drm_display_mode {
 */
u8 type;
 
-   /**
-* @private_flags:
-*
-* Driver private flags. private_flags can only be used for mode
-* objects passed to drivers in modeset operations. It shouldn't be used
-* by atomic drivers since they can store any additional data by
-* subclassing state structures.
-*/
-   u8 private_flags;
-
/**
 * @head:
 *
-- 
2.24.1

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


[PATCH v2 15/17] drm/gma500: Stop using mode->private_flags

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

gma500 only uses mode->private_flags to convey the sdvo pixel
multiplier from the encoder .mode_fixup() hook to the encoder
.mode_set() hook. Those always seems get called as a pair so
let's just stuff the pixel multiplier into the encoder itself
as there are no state objects we could use in this non-atomic
driver.

Paves the way for nuking mode->private_flag.

Cc: Patrik Jakobsson 
CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/gma500/psb_intel_drv.h  | 19 ---
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 11 ++-
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h 
b/drivers/gpu/drm/gma500/psb_intel_drv.h
index fb601983cef0..3dd5718c3e31 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -56,25 +56,6 @@
 #define INTEL_OUTPUT_DISPLAYPORT 9
 #define INTEL_OUTPUT_EDP 10
 
-#define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0)
-#define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf << 
INTEL_MODE_PIXEL_MULTIPLIER_SHIFT)
-
-static inline void
-psb_intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
-   int multiplier)
-{
-   mode->clock *= multiplier;
-   mode->private_flags |= multiplier;
-}
-
-static inline int
-psb_intel_mode_get_pixel_multiplier(const struct drm_display_mode *mode)
-{
-   return (mode->private_flags & INTEL_MODE_PIXEL_MULTIPLIER_MASK)
-  >> INTEL_MODE_PIXEL_MULTIPLIER_SHIFT;
-}
-
-
 /*
  * Hold information useally put on the device driver privates here,
  * since it needs to be shared across multiple of devices drivers privates.
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c 
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 264d7ad004b4..9e88a37f55e9 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -132,6 +132,8 @@ struct psb_intel_sdvo {
/* DDC bus used by this SDVO encoder */
uint8_t ddc_bus;
 
+   u8 pixel_multiplier;
+
/* Input timings for adjusted_mode */
struct psb_intel_sdvo_dtd input_dtd;
 
@@ -958,7 +960,6 @@ static bool psb_intel_sdvo_mode_fixup(struct drm_encoder 
*encoder,
  struct drm_display_mode *adjusted_mode)
 {
struct psb_intel_sdvo *psb_intel_sdvo = to_psb_intel_sdvo(encoder);
-   int multiplier;
 
/* We need to construct preferred input timings based on our
 * output timings.  To do that, we have to set the output
@@ -985,8 +986,9 @@ static bool psb_intel_sdvo_mode_fixup(struct drm_encoder 
*encoder,
/* Make the CRTC code factor in the SDVO pixel multiplier.  The
 * SDVO device will factor out the multiplier during mode_set.
 */
-   multiplier = psb_intel_sdvo_get_pixel_multiplier(adjusted_mode);
-   psb_intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
+   psb_intel_sdvo->pixel_multiplier =
+   psb_intel_sdvo_get_pixel_multiplier(adjusted_mode);
+   adjusted_mode->clock *= psb_intel_sdvo->pixel_multiplier;
 
return true;
 }
@@ -1002,7 +1004,6 @@ static void psb_intel_sdvo_mode_set(struct drm_encoder 
*encoder,
u32 sdvox;
struct psb_intel_sdvo_in_out_map in_out;
struct psb_intel_sdvo_dtd input_dtd;
-   int pixel_multiplier = 
psb_intel_mode_get_pixel_multiplier(adjusted_mode);
int rate;
int need_aux = IS_MRST(dev) ? 1 : 0;
 
@@ -1060,7 +1061,7 @@ static void psb_intel_sdvo_mode_set(struct drm_encoder 
*encoder,
 
(void) psb_intel_sdvo_set_input_timing(psb_intel_sdvo, _dtd);
 
-   switch (pixel_multiplier) {
+   switch (psb_intel_sdvo->pixel_multiplier) {
default:
case 1: rate = SDVO_CLOCK_RATE_MULT_1X; break;
case 2: rate = SDVO_CLOCK_RATE_MULT_2X; break;
-- 
2.24.1

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


[PATCH v2 17/17] drm: Replace mode->export_head with a boolean

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

In order to shrink drm_display_mode below the magic two cacheline
mark in 64bit we need to shrink it by another 8 bytes. The easiest
thing to eliminate is the 'export_head' list head which is only
used during the getconnector ioctl to temporarly track which modes
on the connector's mode list are to be exposed and which are to
remain hidden.

We can simply replace the list head with a boolean which we use
to tag the modes that are to be exposed. If we make sure to clear
the tags after we're done with them we don't even need an extra
loop over the modes to reset the tags at the start of the
getconnector ioctl.

Conveniently we already have a hole for the boolean left
behind by the removal of mode->private_flags. The final size
of the struct is now 112 bytes on 32bit and 120 bytes on 64bit.

The downside is that drm_mode_expose_to_userspace() gets to
iterate a few more modes. It already was O(n^2), now it's
a slightly worse O(n^2).

Another alternative would be a temp bitmask so we wouldn't have
to have anything in the mode struct itself. The mail issues is
how large of a bitmask do we need? I guess we could allocate
it dynamically but that means an extra kcalloc() and an extra
loop through the modes to count them first (or grow the bitmask
with krealloc() as needed).

CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_connector.c | 45 +++--
 include/drm/drm_modes.h | 24 --
 2 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b1099e1251a2..7e719b08564d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2198,7 +2198,7 @@ static struct drm_encoder 
*drm_connector_get_encoder(struct drm_connector *conne
 
 static bool
 drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
-const struct list_head *export_list,
+const struct list_head *modes,
 const struct drm_file *file_priv)
 {
/*
@@ -2214,15 +2214,17 @@ drm_mode_expose_to_userspace(const struct 
drm_display_mode *mode,
 * while preparing the list of user-modes.
 */
if (!file_priv->aspect_ratio_allowed) {
-   struct drm_display_mode *mode_itr;
+   const struct drm_display_mode *mode_itr;
 
-   list_for_each_entry(mode_itr, export_list, export_head)
-   if (drm_mode_match(mode_itr, mode,
+   list_for_each_entry(mode_itr, modes, head) {
+   if (mode_itr->expose_to_userspace &&
+   drm_mode_match(mode_itr, mode,
   DRM_MODE_MATCH_TIMINGS |
   DRM_MODE_MATCH_CLOCK |
   DRM_MODE_MATCH_FLAGS |
   DRM_MODE_MATCH_3D_FLAGS))
return false;
+   }
}
 
return true;
@@ -2242,7 +2244,6 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
struct drm_mode_modeinfo u_mode;
struct drm_mode_modeinfo __user *mode_ptr;
uint32_t __user *encoder_ptr;
-   LIST_HEAD(export_list);
 
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
@@ -2286,25 +2287,30 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
out_resp->connection = connector->status;
 
/* delayed so we get modes regardless of pre-fill_modes state */
-   list_for_each_entry(mode, >modes, head)
-   if (drm_mode_expose_to_userspace(mode, _list,
+   list_for_each_entry(mode, >modes, head) {
+   WARN_ON(mode->expose_to_userspace);
+
+   if (drm_mode_expose_to_userspace(mode, >modes,
 file_priv)) {
-   list_add_tail(>export_head, _list);
+   mode->expose_to_userspace = true;
mode_count++;
}
+   }
 
/*
 * This ioctl is called twice, once to determine how much space is
 * needed, and the 2nd time to fill it.
-* The modes that need to be exposed to the user are maintained in the
-* 'export_list'. When the ioctl is called first time to determine the,
-* space, the export_list gets filled, to find the no.of modes. In the
-* 2nd time, the user modes are filled, one by one from the export_list.
 */
if ((out_resp->count_modes >= mode_count) && mode_count) {
copied = 0;
mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned 
long)out_resp->modes_ptr;
-   list_for_each_entry(mode, _list, export_head) {
+   

[PATCH v2 04/17] drm/msm/dpu: Stop copying around mode->private_flags

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

The driver never sets mode->private_flags so copying
it back and forth is entirely pointless. Stop doing it.

Also drop private_flags from the tracepoint.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h   | 10 +++
 2 files changed, 5 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index a1b79ee2bd9d..d22ecabebb08 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -498,23 +498,6 @@ void dpu_encoder_helper_split_config(
}
 }
 
-static void _dpu_encoder_adjust_mode(struct drm_connector *connector,
-   struct drm_display_mode *adj_mode)
-{
-   struct drm_display_mode *cur_mode;
-
-   if (!connector || !adj_mode)
-   return;
-
-   list_for_each_entry(cur_mode, >modes, head) {
-   if (cur_mode->vdisplay == adj_mode->vdisplay &&
-   cur_mode->hdisplay == adj_mode->hdisplay &&
-   drm_mode_vrefresh(cur_mode) == drm_mode_vrefresh(adj_mode)) 
{
-   adj_mode->private_flags |= cur_mode->private_flags;
-   }
-   }
-}
-
 static struct msm_display_topology dpu_encoder_get_topology(
struct dpu_encoder_virt *dpu_enc,
struct dpu_kms *dpu_kms,
@@ -580,15 +563,6 @@ static int dpu_encoder_virt_atomic_check(
global_state = dpu_kms_get_existing_global_state(dpu_kms);
trace_dpu_enc_atomic_check(DRMID(drm_enc));
 
-   /*
-* display drivers may populate private fields of the drm display mode
-* structure while registering possible modes of a connector with DRM.
-* These private fields are not populated back while DRM invokes
-* the mode_set callbacks. This module retrieves and populates the
-* private fields of the given mode.
-*/
-   _dpu_encoder_adjust_mode(conn_state->connector, adj_mode);
-
/* perform atomic check on the first physical encoder (master) */
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
@@ -621,8 +595,7 @@ static int dpu_encoder_virt_atomic_check(
}
}
 
-   trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags,
-   adj_mode->private_flags);
+   trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
 
return ret;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index eecfe9b3199e..6714b088970f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -327,20 +327,18 @@ DEFINE_EVENT(dpu_enc_keyval_template, 
dpu_enc_trigger_start,
 );
 
 TRACE_EVENT(dpu_enc_atomic_check_flags,
-   TP_PROTO(uint32_t drm_id, unsigned int flags, int private_flags),
-   TP_ARGS(drm_id, flags, private_flags),
+   TP_PROTO(uint32_t drm_id, unsigned int flags),
+   TP_ARGS(drm_id, flags),
TP_STRUCT__entry(
__field(uint32_t,   drm_id  )
__field(unsigned int,   flags   )
-   __field(int,private_flags   )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->flags = flags;
-   __entry->private_flags = private_flags;
),
-   TP_printk("id=%u, flags=%u, private_flags=%d",
- __entry->drm_id, __entry->flags, __entry->private_flags)
+   TP_printk("id=%u, flags=%u",
+ __entry->drm_id, __entry->flags)
 );
 
 DECLARE_EVENT_CLASS(dpu_enc_id_enable_template,
-- 
2.24.1

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


[PATCH v2 11/17] drm: pahole struct drm_display_mode

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Reorganize drm_display_mode to eliminate all the holes.
We'll put all the actual timings to the start of the
struct and all the extra junk to the end.

Gets the size down to 136 bytes on 64bit and 120 bytes on
32bit. With a bit more work we should be able to get this
below the two cacheline mark even on 64bit.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 139 
 1 file changed, 70 insertions(+), 69 deletions(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 95c23f86ae0c..47d62b9d8d20 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -222,56 +222,6 @@ enum drm_mode_status {
  * For printing you can use %DRM_MODE_FMT and DRM_MODE_ARG().
  */
 struct drm_display_mode {
-   /**
-* @head:
-*
-* struct list_head for mode lists.
-*/
-   struct list_head head;
-
-   /**
-* @name:
-*
-* Human-readable name of the mode, filled out with drm_mode_set_name().
-*/
-   char name[DRM_DISPLAY_MODE_LEN];
-
-   /**
-* @status:
-*
-* Status of the mode, used to filter out modes not supported by the
-* hardware. See enum _mode_status.
-*/
-   enum drm_mode_status status;
-
-   /**
-* @type:
-*
-* A bitmask of flags, mostly about the source of a mode. Possible flags
-* are:
-*
-*  - DRM_MODE_TYPE_PREFERRED: Preferred mode, usually the native
-*resolution of an LCD panel. There should only be one preferred
-*mode per connector at any given time.
-*  - DRM_MODE_TYPE_DRIVER: Mode created by the driver, which is all of
-*them really. Drivers must set this bit for all modes they create
-*and expose to userspace.
-*  - DRM_MODE_TYPE_USERDEF: Mode defined via kernel command line
-*
-* Plus a big list of flags which shouldn't be used at all, but are
-* still around since these flags are also used in the userspace ABI.
-* We no longer accept modes with these types though:
-*
-*  - DRM_MODE_TYPE_BUILTIN: Meant for hard-coded modes, unused.
-*Use DRM_MODE_TYPE_DRIVER instead.
-*  - DRM_MODE_TYPE_DEFAULT: Again a leftover, use
-*DRM_MODE_TYPE_PREFERRED instead.
-*  - DRM_MODE_TYPE_CLOCK_C and DRM_MODE_TYPE_CRTC_C: Define leftovers
-*which are stuck around for hysterical raisins only. No one has an
-*idea what they were meant for. Don't use.
-*/
-   u8 type;
-
/**
 * @clock:
 *
@@ -324,22 +274,6 @@ struct drm_display_mode {
 */
u32 flags;
 
-   /**
-* @width_mm:
-*
-* Addressable size of the output in mm, projectors should set this to
-* 0.
-*/
-   u16 width_mm;
-
-   /**
-* @height_mm:
-*
-* Addressable size of the output in mm, projectors should set this to
-* 0.
-*/
-   u16 height_mm;
-
/**
 * @crtc_clock:
 *
@@ -370,6 +304,50 @@ struct drm_display_mode {
u16 crtc_vsync_end;
u16 crtc_vtotal;
 
+   /**
+* @width_mm:
+*
+* Addressable size of the output in mm, projectors should set this to
+* 0.
+*/
+   u16 width_mm;
+
+   /**
+* @height_mm:
+*
+* Addressable size of the output in mm, projectors should set this to
+* 0.
+*/
+   u16 height_mm;
+
+   /**
+* @type:
+*
+* A bitmask of flags, mostly about the source of a mode. Possible flags
+* are:
+*
+*  - DRM_MODE_TYPE_PREFERRED: Preferred mode, usually the native
+*resolution of an LCD panel. There should only be one preferred
+*mode per connector at any given time.
+*  - DRM_MODE_TYPE_DRIVER: Mode created by the driver, which is all of
+*them really. Drivers must set this bit for all modes they create
+*and expose to userspace.
+*  - DRM_MODE_TYPE_USERDEF: Mode defined via kernel command line
+*
+* Plus a big list of flags which shouldn't be used at all, but are
+* still around since these flags are also used in the userspace ABI.
+* We no longer accept modes with these types though:
+*
+*  - DRM_MODE_TYPE_BUILTIN: Meant for hard-coded modes, unused.
+*Use DRM_MODE_TYPE_DRIVER instead.
+*  - DRM_MODE_TYPE_DEFAULT: Again a leftover, use
+*DRM_MODE_TYPE_PREFERRED instead.
+*  - DRM_MODE_TYPE_CLOCK_C and DRM_MODE_TYPE_CRTC_C: Define leftovers
+*which are stuck around for hysterical raisins only. No one has an
+*idea what they were meant for. Don't use.
+*/
+   u8 type;
+
/**
  

[PATCH v2 13/17] drm/i915: Stop using mode->private_flags

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Replace the use of mode->private_flags with a truly private bitmaks
in our own crtc state. We also need a copy in the crtc itself so the
vblank code can get at it. We already have scanline_offset in there
for a similar reason, as well as the vblank->hwmode which is assigned
via drm_calc_timestamping_constants(). Fortunately we now have a
nice place for doing the crtc_state->crtc copy in
intel_crtc_update_active_timings() which gets called both for
modesets and init/resume readout.

The one slightly iffy spot is the INHERITED flag which we want to
preserve until userspace/fb_helper does the first proper commit after
actually calling .detecti() on the connectors. Otherwise we don't have
the full sink capabilities (audio,infoframes,etc.) when .compute_config()
gets called and thus we will fail to enable those features when the
first userspace commit happens. The only internal commit we do prior to
that should be from intel_initial_commit() and there we can simply
preserve the INHERITED flag from the readout.

CC: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/icl_dsi.c| 13 --
 drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
 drivers/gpu/drm/i915/display/intel_display.c  | 24 +--
 .../drm/i915/display/intel_display_types.h|  9 ++-
 drivers/gpu/drm/i915/display/intel_tv.c   |  4 ++--
 drivers/gpu/drm/i915/display/vlv_dsi.c|  6 ++---
 drivers/gpu/drm/i915/i915_irq.c   |  4 ++--
 7 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c 
b/drivers/gpu/drm/i915/display/icl_dsi.c
index 99a25c0bb08f..4d6788ef2e5e 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1469,8 +1469,7 @@ static void gen11_dsi_get_config(struct intel_encoder 
*encoder,
pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
 
if (gen11_dsi_is_periodic_cmd_mode(intel_dsi))
-   pipe_config->hw.adjusted_mode.private_flags |=
-   I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
+   pipe_config->mode_flags |= I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
 }
 
 static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
@@ -1555,10 +1554,6 @@ static int gen11_dsi_compute_config(struct intel_encoder 
*encoder,
 
pipe_config->port_clock = afe_clk(encoder, pipe_config) / 5;
 
-   /* We would not operate in periodic command mode */
-   pipe_config->hw.adjusted_mode.private_flags &=
-   ~I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
-
/*
 * In case of TE GATE cmd mode, we
 * receive TE from the slave if
@@ -1566,14 +1561,14 @@ static int gen11_dsi_compute_config(struct 
intel_encoder *encoder,
 */
if (is_cmd_mode(intel_dsi)) {
if (intel_dsi->ports == (BIT(PORT_B) | BIT(PORT_A)))
-   pipe_config->hw.adjusted_mode.private_flags |=
+   pipe_config->mode_flags |=
I915_MODE_FLAG_DSI_USE_TE1 |
I915_MODE_FLAG_DSI_USE_TE0;
else if (intel_dsi->ports == BIT(PORT_B))
-   pipe_config->hw.adjusted_mode.private_flags |=
+   pipe_config->mode_flags |=
I915_MODE_FLAG_DSI_USE_TE1;
else
-   pipe_config->hw.adjusted_mode.private_flags |=
+   pipe_config->mode_flags |=
I915_MODE_FLAG_DSI_USE_TE0;
}
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c 
b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa0..5863e339a426 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -252,6 +252,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->wm.need_postvbl_update = false;
crtc_state->fb_bits = 0;
crtc_state->update_planes = 0;
+   crtc_state->mode_flags &= ~I915_MODE_FLAG_INHERITED;
 
return _state->uapi;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index bcb5d754f20d..d88cade45c35 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6414,7 +6414,7 @@ static bool hsw_post_update_enable_ips(const struct 
intel_crtc_state *old_crtc_s
 * forcibly enable IPS on the first fastset.
 */
if (new_crtc_state->update_pipe &&
-   old_crtc_state->hw.adjusted_mode.private_flags & 
I915_MODE_FLAG_INHERITED)
+   old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
return true;
 
return !old_crtc_state->ips_enabled;
@@ -13516,8 

[PATCH v2 06/17] drm: Shrink mode->type to u8

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

We only have 7 bits defined for mode->type. Shrink the storage to u8.

Reviewed-by: Emil Velikov 
Reviewed-by: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 3625e3681488..f4507b987038 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -270,7 +270,7 @@ struct drm_display_mode {
 *which are stuck around for hysterical raisins only. No one has an
 *idea what they were meant for. Don't use.
 */
-   unsigned int type;
+   u8 type;
 
/**
 * @clock:
-- 
2.24.1

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


[PATCH v2 08/17] drm: Shrink drm_display_mode timings

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Store the timings (apart from the clock) as u16. The uapi mode
struct already uses u16 for everything so using something bigger
internally doesn't really help us.

Reviewed-by: Emil Velikov 
Reviewed-by: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_modes.c |  7 --
 include/drm/drm_modes.h | 46 ++---
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index e3d5f011f7bd..77d68120931a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1901,13 +1901,6 @@ EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode);
 void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
   const struct drm_display_mode *in)
 {
-   WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX ||
-in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX ||
-in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX ||
-in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX ||
-in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX,
-"timing values too large for mode info\n");
-
out->clock = in->clock;
out->hdisplay = in->hdisplay;
out->hsync_start = in->hsync_start;
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index da7db74a215e..917527eb8067 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -278,16 +278,16 @@ struct drm_display_mode {
 * Pixel clock in kHz.
 */
int clock;  /* in kHz */
-   int hdisplay;
-   int hsync_start;
-   int hsync_end;
-   int htotal;
-   int hskew;
-   int vdisplay;
-   int vsync_start;
-   int vsync_end;
-   int vtotal;
-   int vscan;
+   u16 hdisplay;
+   u16 hsync_start;
+   u16 hsync_end;
+   u16 htotal;
+   u16 hskew;
+   u16 vdisplay;
+   u16 vsync_start;
+   u16 vsync_end;
+   u16 vtotal;
+   u16 vscan;
/**
 * @flags:
 *
@@ -356,19 +356,19 @@ struct drm_display_mode {
 * difference is exactly a factor of 10.
 */
int crtc_clock;
-   int crtc_hdisplay;
-   int crtc_hblank_start;
-   int crtc_hblank_end;
-   int crtc_hsync_start;
-   int crtc_hsync_end;
-   int crtc_htotal;
-   int crtc_hskew;
-   int crtc_vdisplay;
-   int crtc_vblank_start;
-   int crtc_vblank_end;
-   int crtc_vsync_start;
-   int crtc_vsync_end;
-   int crtc_vtotal;
+   u16 crtc_hdisplay;
+   u16 crtc_hblank_start;
+   u16 crtc_hblank_end;
+   u16 crtc_hsync_start;
+   u16 crtc_hsync_end;
+   u16 crtc_htotal;
+   u16 crtc_hskew;
+   u16 crtc_vdisplay;
+   u16 crtc_vblank_start;
+   u16 crtc_vblank_end;
+   u16 crtc_vsync_start;
+   u16 crtc_vsync_end;
+   u16 crtc_vtotal;
 
/**
 * @private_flags:
-- 
2.24.1

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


[PATCH v2 09/17] drm: Flatten drm_mode_vrefresh()

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Remove the pointless whole-function indentation. Also don't
need to worry about negative values anymore since we switched
everything to u16.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_modes.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 77d68120931a..f2865f88bd54 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -757,24 +757,22 @@ EXPORT_SYMBOL(drm_mode_set_name);
  */
 int drm_mode_vrefresh(const struct drm_display_mode *mode)
 {
-   int refresh = 0;
+   unsigned int num, den;
 
-   if (mode->htotal > 0 && mode->vtotal > 0) {
-   unsigned int num, den;
+   if (mode->htotal == 0 || mode->vtotal == 0)
+   return 0;
 
-   num = mode->clock * 1000;
-   den = mode->htotal * mode->vtotal;
+   num = mode->clock * 1000;
+   den = mode->htotal * mode->vtotal;
 
-   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-   num *= 2;
-   if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
-   den *= 2;
-   if (mode->vscan > 1)
-   den *= mode->vscan;
+   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+   num *= 2;
+   if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
+   den *= 2;
+   if (mode->vscan > 1)
+   den *= mode->vscan;
 
-   refresh = DIV_ROUND_CLOSEST(num, den);
-   }
-   return refresh;
+   return DIV_ROUND_CLOSEST(num, den);
 }
 EXPORT_SYMBOL(drm_mode_vrefresh);
 
-- 
2.24.1

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


[PATCH v2 07/17] drm: Make mode->flags u32

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

The mode flags are direclty exposed in the uapi as u32. Use the
same size type to store them internally.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index f4507b987038..da7db74a215e 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -322,7 +322,7 @@ struct drm_display_mode {
 *  - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF: frame split into left and
 *right parts.
 */
-   unsigned int flags;
+   u32 flags;
 
/**
 * @width_mm:
-- 
2.24.1

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


[PATCH v2 12/17] drm/mcde: Use mode->clock instead of reverse calculating it from the vrefresh

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

htotal*vtotal*vrefresh ~= clock. So just say "clock" when we mean it.

Cc: Linus Walleij 
Cc: Sam Ravnborg 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/mcde/mcde_dsi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index 52031d826f2c..c07a8e273b6f 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -537,8 +537,7 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
 * porches and sync.
 */
/* (ps/s) / (pixels/s) = ps/pixels */
-   pclk = DIV_ROUND_UP_ULL(1,
-   (drm_mode_vrefresh(mode) * mode->htotal * 
mode->vtotal));
+   pclk = DIV_ROUND_UP_ULL(1, mode->clock);
dev_dbg(d->dev, "picoseconds between two pixels: %llu\n",
pclk);
 
-- 
2.24.1

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


[PATCH v2 01/17] drm: Nuke mode->hsync

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Let's just calculate the hsync rate on demand. No point in wasting
space storing it and risking the cached value getting out of sync
with reality.

v2: Move drm_mode_hsync() next to its only users
Drop the TODO

Reviewed-by: Emil Velikov  #v1
Signed-off-by: Ville Syrjälä 
---
 Documentation/gpu/todo.rst   | 12 -
 drivers/gpu/drm/drm_edid.c   |  8 ++
 drivers/gpu/drm/drm_modes.c  | 26 
 drivers/gpu/drm/i915/display/intel_display.c |  1 -
 include/drm/drm_modes.h  | 11 -
 5 files changed, 8 insertions(+), 50 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 439656f55c5d..658b52f7ffc6 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -347,18 +347,6 @@ Contact: Sean Paul
 
 Level: Starter
 
-Remove drm_display_mode.hsync
--
-
-We have drm_mode_hsync() to calculate this from hsync_start/end, since drivers
-shouldn't/don't use this, remove this member to avoid any temptations to use it
-in the future. If there is any debug code using drm_display_mode.hsync, convert
-it to use drm_mode_hsync() instead.
-
-Contact: Sean Paul
-
-Level: Starter
-
 connector register/unregister fixes
 ---
 
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 43b6ca364daa..3bd95c4b02eb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2380,6 +2380,14 @@ bad_std_timing(u8 a, u8 b)
   (a == 0x20 && b == 0x20);
 }
 
+static int drm_mode_hsync(const struct drm_display_mode *mode)
+{
+   if (mode->htotal <= 0)
+   return 0;
+
+   return DIV_ROUND_CLOSEST(mode->clock, mode->htotal);
+}
+
 /**
  * drm_mode_std - convert standard mode info (width, height, refresh) into mode
  * @connector: connector of for the EDID block
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d4d64518e11b..fec1c33b3045 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -747,32 +747,6 @@ void drm_mode_set_name(struct drm_display_mode *mode)
 }
 EXPORT_SYMBOL(drm_mode_set_name);
 
-/**
- * drm_mode_hsync - get the hsync of a mode
- * @mode: mode
- *
- * Returns:
- * @modes's hsync rate in kHz, rounded to the nearest integer. Calculates the
- * value first if it is not yet set.
- */
-int drm_mode_hsync(const struct drm_display_mode *mode)
-{
-   unsigned int calc_val;
-
-   if (mode->hsync)
-   return mode->hsync;
-
-   if (mode->htotal <= 0)
-   return 0;
-
-   calc_val = (mode->clock * 1000) / mode->htotal; /* hsync in Hz */
-   calc_val += 500;/* round to 1000Hz */
-   calc_val /= 1000;   /* truncate to kHz */
-
-   return calc_val;
-}
-EXPORT_SYMBOL(drm_mode_hsync);
-
 /**
  * drm_mode_vrefresh - get the vrefresh of a mode
  * @mode: mode
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 70ec301fe6e3..5ebb2df5f1f4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8870,7 +8870,6 @@ void intel_mode_from_pipe_config(struct drm_display_mode 
*mode,
 
mode->clock = pipe_config->hw.adjusted_mode.crtc_clock;
 
-   mode->hsync = drm_mode_hsync(mode);
mode->vrefresh = drm_mode_vrefresh(mode);
drm_mode_set_name(mode);
 }
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 99134d4f35eb..730fc31de4fb 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -390,16 +390,6 @@ struct drm_display_mode {
 */
int vrefresh;
 
-   /**
-* @hsync:
-*
-* Horizontal refresh rate, for debug output in human readable form. Not
-* used in a functional way.
-*
-* This value is in kHz.
-*/
-   int hsync;
-
/**
 * @picture_aspect_ratio:
 *
@@ -493,7 +483,6 @@ int of_get_drm_display_mode(struct device_node *np,
int index);
 
 void drm_mode_set_name(struct drm_display_mode *mode);
-int drm_mode_hsync(const struct drm_display_mode *mode);
 int drm_mode_vrefresh(const struct drm_display_mode *mode);
 void drm_mode_get_hv_timing(const struct drm_display_mode *mode,
int *hdisplay, int *vdisplay);
-- 
2.24.1

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


[PATCH v2 10/17] drm: Shrink mode->private_flags

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

gma500 needs 4 bits (to store a pixel multiplier) in the
mode->private_flags, i915 currently has three bits defined.
No one else uses this. Reduce the size to u8.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 917527eb8067..95c23f86ae0c 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -378,7 +378,7 @@ struct drm_display_mode {
 * by atomic drivers since they can store any additional data by
 * subclassing state structures.
 */
-   int private_flags;
+   u8 private_flags;
 
/**
 * @picture_aspect_ratio:
-- 
2.24.1

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


[PATCH v2 02/17] drm/i915: Introduce some local intel_dp variables

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

The drrs code dereferences mode->vrefresh via some really long chain
of structures/pointers. Couldn't get coccinelle to see through all
that so let's add some local variables to help it.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index db6ae8e9af6e..ffc2816787db 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7721,6 +7721,7 @@ static void intel_edp_drrs_downclock_work(struct 
work_struct *work)
 void intel_edp_drrs_invalidate(struct drm_i915_private *dev_priv,
   unsigned int frontbuffer_bits)
 {
+   struct intel_dp *intel_dp;
struct drm_crtc *crtc;
enum pipe pipe;
 
@@ -7730,12 +7731,14 @@ void intel_edp_drrs_invalidate(struct drm_i915_private 
*dev_priv,
cancel_delayed_work(_priv->drrs.work);
 
mutex_lock(_priv->drrs.mutex);
-   if (!dev_priv->drrs.dp) {
+
+   intel_dp = dev_priv->drrs.dp;
+   if (!intel_dp) {
mutex_unlock(_priv->drrs.mutex);
return;
}
 
-   crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
+   crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
pipe = to_intel_crtc(crtc)->pipe;
 
frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
@@ -7744,7 +7747,7 @@ void intel_edp_drrs_invalidate(struct drm_i915_private 
*dev_priv,
/* invalidate means busy screen hence upclock */
if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
intel_dp_set_drrs_state(dev_priv, to_intel_crtc(crtc)->config,
-   
dev_priv->drrs.dp->attached_connector->panel.fixed_mode->vrefresh);
+   
intel_dp->attached_connector->panel.fixed_mode->vrefresh);
 
mutex_unlock(_priv->drrs.mutex);
 }
@@ -7764,6 +7767,7 @@ void intel_edp_drrs_invalidate(struct drm_i915_private 
*dev_priv,
 void intel_edp_drrs_flush(struct drm_i915_private *dev_priv,
  unsigned int frontbuffer_bits)
 {
+   struct intel_dp *intel_dp;
struct drm_crtc *crtc;
enum pipe pipe;
 
@@ -7773,12 +,14 @@ void intel_edp_drrs_flush(struct drm_i915_private 
*dev_priv,
cancel_delayed_work(_priv->drrs.work);
 
mutex_lock(_priv->drrs.mutex);
-   if (!dev_priv->drrs.dp) {
+
+   intel_dp = dev_priv->drrs.dp;
+   if (!intel_dp) {
mutex_unlock(_priv->drrs.mutex);
return;
}
 
-   crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
+   crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
pipe = to_intel_crtc(crtc)->pipe;
 
frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
@@ -7787,7 +7793,7 @@ void intel_edp_drrs_flush(struct drm_i915_private 
*dev_priv,
/* flush means busy screen hence upclock */
if (frontbuffer_bits && dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
intel_dp_set_drrs_state(dev_priv, to_intel_crtc(crtc)->config,
-   
dev_priv->drrs.dp->attached_connector->panel.fixed_mode->vrefresh);
+   
intel_dp->attached_connector->panel.fixed_mode->vrefresh);
 
/*
 * flush also means no more activity hence schedule downclock, if all
-- 
2.24.1

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


[PATCH v2 00/17] drm: Put drm_display_mode on diet

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Refreshed version of the mode diet.

New unseen stuff at the end:
- Nuke private_flags entirely
- Replace export_head with a bool to shrink the struct
  below the magic two cachelines

I kept the intermediate "shrink private_flags to u8" step
because I didn't want to redo the pahole numbers.

Ville Syrjälä (17):
  drm: Nuke mode->hsync
  drm/i915: Introduce some local intel_dp variables
  drm: Nuke mode->vrefresh
  drm/msm/dpu: Stop copying around mode->private_flags
  drm: Shrink {width,height}_mm to u16
  drm: Shrink mode->type to u8
  drm: Make mode->flags u32
  drm: Shrink drm_display_mode timings
  drm: Flatten drm_mode_vrefresh()
  drm: Shrink mode->private_flags
  drm: pahole struct drm_display_mode
  drm/mcde: Use mode->clock instead of reverse calculating it from the
vrefresh
  drm/i915: Stop using mode->private_flags
  drm/i915: Replace I915_MODE_FLAG_INHERITED with a boolean
  drm/gma500: Stop using mode->private_flags
  drm: Nuke mode->private_flags
  drm: Replace mode->export_head with a boolean

 Documentation/gpu/todo.rst|  32 --
 drivers/gpu/drm/bridge/sii902x.c  |   2 +-
 drivers/gpu/drm/drm_client_modeset.c  |   2 +-
 drivers/gpu/drm/drm_connector.c   |  45 ++-
 drivers/gpu/drm/drm_edid.c| 336 +-
 drivers/gpu/drm/drm_modes.c   |  66 +---
 drivers/gpu/drm/drm_probe_helper.c|   3 -
 drivers/gpu/drm/exynos/exynos_hdmi.c  |   5 +-
 drivers/gpu/drm/exynos/exynos_mixer.c |   2 +-
 drivers/gpu/drm/gma500/psb_intel_drv.h|  19 -
 drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  11 +-
 drivers/gpu/drm/i2c/ch7006_mode.c |   1 -
 drivers/gpu/drm/i915/display/icl_dsi.c|  13 +-
 drivers/gpu/drm/i915/display/intel_atomic.c   |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |  27 +-
 .../drm/i915/display/intel_display_debugfs.c  |   4 +-
 .../drm/i915/display/intel_display_types.h|  11 +-
 drivers/gpu/drm/i915/display/intel_dp.c   |  24 +-
 drivers/gpu/drm/i915/display/intel_tv.c   |   7 +-
 drivers/gpu/drm/i915/display/vlv_dsi.c|   6 +-
 drivers/gpu/drm/i915/i915_irq.c   |   4 +-
 drivers/gpu/drm/mcde/mcde_dsi.c   |   7 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c   |   4 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c   |   2 +-
 drivers/gpu/drm/meson/meson_venc_cvbs.c   |   2 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  29 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  10 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |   5 +-
 drivers/gpu/drm/panel/panel-arm-versatile.c   |   4 -
 drivers/gpu/drm/panel/panel-boe-himax8279d.c  |   3 +-
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c|   6 +-
 drivers/gpu/drm/panel/panel-elida-kd35t133.c  |   3 +-
 .../gpu/drm/panel/panel-feixin-k101-im2ba02.c |   3 +-
 .../drm/panel/panel-feiyang-fy07024di26a30d.c |   3 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9322.c  |   7 -
 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c |   3 +-
 drivers/gpu/drm/panel/panel-innolux-p079zca.c |   4 +-
 .../gpu/drm/panel/panel-jdi-lt070me05000.c|   3 +-
 .../drm/panel/panel-kingdisplay-kd097d04.c|   3 +-
 .../drm/panel/panel-leadtek-ltk500hd1829.c|   3 +-
 drivers/gpu/drm/panel/panel-lg-lb035q02.c |   1 -
 drivers/gpu/drm/panel/panel-lg-lg4573.c   |   3 +-
 drivers/gpu/drm/panel/panel-nec-nl8048hl11.c  |   1 -
 drivers/gpu/drm/panel/panel-novatek-nt35510.c |   1 -
 drivers/gpu/drm/panel/panel-novatek-nt39016.c |   1 -
 .../drm/panel/panel-olimex-lcd-olinuxino.c|   1 -
 .../gpu/drm/panel/panel-orisetech-otm8009a.c  |   3 +-
 .../drm/panel/panel-osd-osd101t2587-53ts.c|   3 +-
 .../drm/panel/panel-panasonic-vvx10f034n00.c  |   3 +-
 .../drm/panel/panel-raspberrypi-touchscreen.c |   4 +-
 drivers/gpu/drm/panel/panel-raydium-rm67191.c |   3 +-
 drivers/gpu/drm/panel/panel-raydium-rm68200.c |   3 +-
 .../drm/panel/panel-rocktech-jh057n00900.c|   5 +-
 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |   1 -
 drivers/gpu/drm/panel/panel-samsung-s6d16d0.c |   6 -
 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c |   4 +-
 .../gpu/drm/panel/panel-samsung-s6e63j0x03.c  |   3 +-
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c |   3 +-
 .../panel/panel-samsung-s6e88a0-ams452ef01.c  |   1 -
 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c   |   3 +-
 .../gpu/drm/panel/panel-sharp-lq101r1sx01.c   |   3 +-
 .../gpu/drm/panel/panel-sharp-ls037v7dw01.c   |   1 -
 .../gpu/drm/panel/panel-sharp-ls043t1le01.c   |   3 +-
 drivers/gpu/drm/panel/panel-simple.c  |  87 +
 drivers/gpu/drm/panel/panel-sitronix-st7701.c |   2 +-
 .../gpu/drm/panel/panel-sitronix-st7789v.c|   3 +-
 drivers/gpu/drm/panel/panel-sony-acx424akp.c  |   2 -
 drivers/gpu/drm/panel/panel-sony-acx565akm.c  |   1 -
 drivers/gpu/drm/panel/panel-tpo-td028ttec1.c  |   1 -
 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c  |   1 -
 

[PATCH v2 05/17] drm: Shrink {width,height}_mm to u16

2020-04-03 Thread Ville Syrjala
From: Ville Syrjälä 

Instead of supporting ~2000km wide displayes let's limit ourselves
to ~65m. That seems plenty big enough to me.

Even with EDID_QUIRK_DETAILED_IN_CM EDIDs seem to be limited to
10*0xfff which fits into the 16 bits.

Reviewed-by: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 include/drm/drm_modes.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 8b05f3705d0e..3625e3681488 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -330,7 +330,7 @@ struct drm_display_mode {
 * Addressable size of the output in mm, projectors should set this to
 * 0.
 */
-   int width_mm;
+   u16 width_mm;
 
/**
 * @height_mm:
@@ -338,7 +338,7 @@ struct drm_display_mode {
 * Addressable size of the output in mm, projectors should set this to
 * 0.
 */
-   int height_mm;
+   u16 height_mm;
 
/**
 * @crtc_clock:
-- 
2.24.1

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


Re: [PATCH] drm/amdgpu: Fix oops when pp_funcs is unset in ACPI event

2020-04-03 Thread Alex Deucher
On Fri, Apr 3, 2020 at 11:18 AM Aaron Ma  wrote:
>
> On ARCTURUS and RENOIR, powerplay is not supported yet.
> When plug in or unplug power jack, ACPI event will issue.
> Then kernel NULL pointer BUG will be triggered.
> Check for NULL pointers before calling.
>
> Signed-off-by: Aaron Ma 

Applied.  Thanks!

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index f197f1be0969..611de69f9d48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -89,7 +89,8 @@ void amdgpu_pm_acpi_event_handler(struct amdgpu_device 
> *adev)
> adev->pm.ac_power = true;
> else
> adev->pm.ac_power = false;
> -   if (adev->powerplay.pp_funcs->enable_bapm)
> +   if (adev->powerplay.pp_funcs &&
> +   adev->powerplay.pp_funcs->enable_bapm)
> amdgpu_dpm_enable_bapm(adev, adev->pm.ac_power);
> mutex_unlock(>pm.mutex);
>
> --
> 2.17.1
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Curtaining API / Force blanking displays

2020-04-03 Thread Erik Jensen
First off, apologies if the functionality described already exists and I
just failed to find it, or if this isn't the correct venue for this
discussion. If so, pointers to the correct location would be appreciated.

I'm currently looking into the feasibility of developing a remote access
tool using kernel-level interfaces (e.g., drmModeGetFB and uinput) to
operate regardless of whether the user is using Xorg, a Wayland compositor,
or even a text console (assuming KMS is in use).

One of the requirements, however, is the remote user is able to "curtain"
their session in order to prevent individuals near the physical machine
from watching their session. Imagine a user working from home and
connecting to their workstation in a shared office space.

One possible solution I came up with would be a new kernel API to allow a
privileged process other than the DRM-Master to request that all displays
of a card be blanked or left in power saving mode. This wouldn't affect the
ability of the DRM-Master to change modes and layout configuration, but no
content would be visible on the physical displays until the curtaining
process ended the curtain or exited.

Is this (a) a good approach to solving this issue, (b) an API that, if
implemented, would be likely to be accepted into the kernel, and (c)
something that would be feasible to implement given the current
architecture? E.g., would it require changes in individual drivers, or
could it be managed solely in driver-independent kernel code?

I'm new to DRI development, so if it is something that folks would be open
to having, pointers to a good part of the code to look at to start
implementing such a feature would be appreciated.

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


[PATCH 3/4] drm/dp_mst: Increase ACT retry timeout to 3s

2020-04-03 Thread Lyude Paul
Currently we only poll for an ACT up to 30 times, with a busy-wait delay
of 100µs between each attempt - giving us a timeout of 2900µs. While
this might seem sensible, it would appear that in certain scenarios it
can take dramatically longer then that for us to receive an ACT. On one
of the EVGA MST hubs that I have available, I observed said hub
sometimes taking longer then a second before signalling the ACT. These
delays mostly seem to occur when previous sideband messages we've sent
are NAKd by the hub, however it wouldn't be particularly surprising if
it's possible to reproduce times like this simply by introducing branch
devices with large LCTs since payload allocations have to take effect on
every downstream device up to the payload's target.

So, instead of just retrying 30 times we poll for the ACT for up to 3ms,
and additionally use usleep_range() to avoid a very long and rude
busy-wait. Note that the previous retry count of 30 appears to have been
arbitrarily chosen, as I can't find any mention of a recommended timeout
or retry count for ACTs in the DisplayPort 2.0 specification. This also
goes for the range we were previously using for udelay(), although I
suspect that was just copied from the recommended delay for link
training on SST devices.

Signed-off-by: Lyude Paul 
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul 
Cc:  # v3.17+
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7aaf184a2e5f..f313407374ed 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4466,17 +4466,30 @@ static int drm_dp_dpcd_write_payload(struct 
drm_dp_mst_topology_mgr *mgr,
  * @mgr: manager to use
  *
  * Tries waiting for the MST hub to finish updating it's payload table by
- * polling for the ACT handled bit.
+ * polling for the ACT handled bit for up to 3 seconds (yes-some hubs really
+ * take that long).
  *
  * Returns:
  * 0 if the ACT was handled in time, negative error code on failure.
  */
 int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
 {
-   int count = 0, ret;
+   /*
+* There doesn't seem to be any recommended retry count or timeout in
+* the MST specification. Since some hubs have been observed to take
+* over 1 second to update their payload allocations under certain
+* conditions, we use a rather large timeout value.
+*/
+   const int timeout_ms = 3000;
+   unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+   int ret;
+   bool retrying = false;
u8 status;
 
do {
+   if (retrying)
+   usleep_range(100, 1000);
+
ret = drm_dp_dpcd_readb(mgr->aux,
DP_PAYLOAD_TABLE_UPDATE_STATUS,
);
@@ -4488,13 +4501,12 @@ int drm_dp_check_act_status(struct 
drm_dp_mst_topology_mgr *mgr)
 
if (status & DP_PAYLOAD_ACT_HANDLED)
break;
-   count++;
-   udelay(100);
-   } while (count < 30);
+   retrying = true;
+   } while (jiffies < timeout);
 
if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
-   DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
- status, count);
+   DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
+ status, timeout_ms);
return -EINVAL;
}
return 0;
-- 
2.25.1

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


[PATCH 4/4] drm/dp_mst: Print errors on ACT timeouts

2020-04-03 Thread Lyude Paul
Although it's not unexpected for drm_dp_check_act_status() to fail due
to DPCD read failures (as the hub may have just been unplugged
suddenly), timeouts are a bit more worrying as they either mean we need
a longer timeout value, or we aren't setting up payload allocations
properly. So, let's start printing errors on timeouts.

Signed-off-by: Lyude Paul 
Cc: Sean Paul 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f313407374ed..3d0d373f6f91 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4494,6 +4494,10 @@ int drm_dp_check_act_status(struct 
drm_dp_mst_topology_mgr *mgr)
DP_PAYLOAD_TABLE_UPDATE_STATUS,
);
if (ret < 0) {
+   /*
+* Failure here isn't unexpected - the hub may have
+* just been unplugged
+*/
DRM_DEBUG_KMS("failed to read payload table status 
%d\n",
  ret);
return ret;
@@ -4505,8 +4509,8 @@ int drm_dp_check_act_status(struct 
drm_dp_mst_topology_mgr *mgr)
} while (jiffies < timeout);
 
if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
-   DRM_DEBUG_KMS("failed to get ACT bit %d after %dms\n",
- status, timeout_ms);
+   DRM_ERROR("Failed to get ACT after %dms, last status: %02x\n",
+ timeout_ms, status);
return -EINVAL;
}
return 0;
-- 
2.25.1

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


[PATCH 0/4] drm/dp_mst: drm_dp_check_act_status() fixes

2020-04-03 Thread Lyude Paul
Noticed this while fixing some unrelated issues with NAKs being dropped
- we don't wait nearly long enough to receive ACTs from MST hubs in some
situations. Also, we take the time to refactor this function a bit.

This fixes some ACT timeouts I observed on an EVGA MST hub with i915.

Lyude Paul (4):
  drm/dp_mst: Improve kdocs for drm_dp_check_act_status()
  drm/dp_mst: Reformat drm_dp_check_act_status() a bit
  drm/dp_mst: Increase ACT retry timeout to 3s
  drm/dp_mst: Print errors on ACT timeouts

 drivers/gpu/drm/drm_dp_mst_topology.c | 50 ++-
 1 file changed, 34 insertions(+), 16 deletions(-)

-- 
2.25.1

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


[PATCH 2/4] drm/dp_mst: Reformat drm_dp_check_act_status() a bit

2020-04-03 Thread Lyude Paul
Just add a bit more line wrapping, get rid of some extraneous
whitespace, remove an unneeded goto label, and move around some variable
declarations. No functional changes here.

Signed-off-by: Lyude Paul 
[this isn't a fix, but it's needed for the fix that comes after this]
Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul 
Cc:  # v3.17+
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2b9ce965f044..7aaf184a2e5f 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct 
drm_dp_mst_topology_mgr *mgr,
  */
 int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
 {
+   int count = 0, ret;
u8 status;
-   int ret;
-   int count = 0;
 
do {
-   ret = drm_dp_dpcd_readb(mgr->aux, 
DP_PAYLOAD_TABLE_UPDATE_STATUS, );
-
+   ret = drm_dp_dpcd_readb(mgr->aux,
+   DP_PAYLOAD_TABLE_UPDATE_STATUS,
+   );
if (ret < 0) {
-   DRM_DEBUG_KMS("failed to read payload table status 
%d\n", ret);
-   goto fail;
+   DRM_DEBUG_KMS("failed to read payload table status 
%d\n",
+ ret);
+   return ret;
}
 
if (status & DP_PAYLOAD_ACT_HANDLED)
break;
count++;
udelay(100);
-
} while (count < 30);
 
if (!(status & DP_PAYLOAD_ACT_HANDLED)) {
-   DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n", 
status, count);
-   ret = -EINVAL;
-   goto fail;
+   DRM_DEBUG_KMS("failed to get ACT bit %d after %d retries\n",
+ status, count);
+   return -EINVAL;
}
return 0;
-fail:
-   return ret;
 }
 EXPORT_SYMBOL(drm_dp_check_act_status);
 
-- 
2.25.1

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


[PATCH 1/4] drm/dp_mst: Improve kdocs for drm_dp_check_act_status()

2020-04-03 Thread Lyude Paul
No functional changes.

Signed-off-by: Lyude Paul 
Cc: Sean Paul 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 10d0315af513..2b9ce965f044 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4462,10 +4462,14 @@ static int drm_dp_dpcd_write_payload(struct 
drm_dp_mst_topology_mgr *mgr,
 
 
 /**
- * drm_dp_check_act_status() - Check ACT handled status.
+ * drm_dp_check_act_status() - Polls for ACT handled status.
  * @mgr: manager to use
  *
- * Check the payload status bits in the DPCD for ACT handled completion.
+ * Tries waiting for the MST hub to finish updating it's payload table by
+ * polling for the ACT handled bit.
+ *
+ * Returns:
+ * 0 if the ACT was handled in time, negative error code on failure.
  */
 int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr)
 {
-- 
2.25.1

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


[PATCH] drm/dp_mst: Don't drop NAKs for down responses

2020-04-03 Thread Lyude Paul
It looks like that when we introduced the ability to handle multiple
down requests at once, we accidentally started dropping NAK replies -
causing sideband messages which got NAK'd to seemingly timeout and cause
all sorts of weirdness.

So, fix this by making sure we don't return from
drm_dp_mst_handle_down_rep() early, but instead treat NAKs like any
other message.

Signed-off-by: Lyude Paul 
Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies")
Cc: Wayne Lin 
Cc: Wayne Lin 
Cc: Sean Paul 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 10d0315af513..5449ada3e019 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3813,7 +3813,6 @@ static int drm_dp_mst_handle_down_rep(struct 
drm_dp_mst_topology_mgr *mgr)
  txmsg->reply.u.nak.reason,
  
drm_dp_mst_nak_reason_str(txmsg->reply.u.nak.reason),
  txmsg->reply.u.nak.nak_data);
-   goto out_clear_reply;
}
 
memset(msg, 0, sizeof(struct drm_dp_sideband_msg_rx));
-- 
2.25.1

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


Re: How to handle disconnection of eDP panels due to dynamic display mux switches

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 8:06 PM Daniel Dadap  wrote:
>
>
> On 4/3/20 2:16 AM, Daniel Vetter wrote:
> > On Fri, Apr 3, 2020 at 8:54 AM Daniel Dadap  wrote:
> >>
> >> On 4/2/20 6:39 AM, Lukas Wunner wrote:
> >>
> >>
> >> On Fri, Mar 27, 2020 at 04:25:19PM -0500, Daniel Dadap wrote:
>  A number of hybrid GPU notebook computer designs with dual (integrated 
>  plus
>  discrete) GPUs are equipped with multiplexers (muxes) that allow display
>  panels to be driven by either the integrated GPU or the discrete GPU.
>  Typically, this is a selection that can be made at boot time as a menu
>  option in the system firmware's setup screen, and the mux selection stays
>  fixed for as long as the system is running and persists across reboots 
>  until
>  it is explicitly changed. However, some muxed hybrid GPU systems have
>  dynamically switchable muxes which can be switched while the system is
>  running.
> >>> As you may be aware, there's drivers/gpu/vga/vga_switcheroo.c (of which
> >>> I'm listed as a reviewer in MAINTAINERS) to support such hardware.
> >>>
> >>> It also supports muxed configurations, including those that support
> >>> switching at runtime (and not only at boot) such as the MacBook Pro,
> >>> which uses drivers/platform/x86/apple-gmux.c to interface between
> >>> vga_switcheroo and the hardware mux.
> >>>
> >>> However, so far switching only actually works on LVDS-based MacBook Pros,
> >>> i.e. all pre-retina machines introduced between Late 2008 and Mid 2012,
> >>> because that hardware is capable of switching the DDC pins separately
> >>> from the display, so we lock and switch them when probing the EDID.
> >>
> >> I have observed that on at least some systems, the EDID for the internal
> >> panel can be read via the ACPI _DDC method regardless of whether it's
> >> actively muxed in. I don't know whether that's true for all systems
> >> where the DDC line can't be switched independently, but maybe
> >> vga_switcheroo could also export an interface for GPU drivers to cache
> >> EDIDs so that a muxed-away GPU can read an EDID that was previously read
> >> by another GPU? I guess the utility of that would depend on how
> >> prevalent the combination of no DDC muxing + no ACPI EDID reads turns
> >> out to be.
> >>
> >>
> >>> The retina machines introduced from Mid 2012 onward use eDP and run
> >>> into the issues you're describing:  The AUX channel cannot be switched
> >>> separately from the display, so link training fails unless the entire
> >>> display is switched.  Nevertheless macOS can switch the panel seamlessly.
> >>> So how are they doing it?
> >>>
> >>> Well, I don't own a retina MacBook Pro, hence never got very far with
> >>> supporting them, but I did some research and experiments in the 2015/2016
> >>> time frame which a colleague, Bruno Bierbaumer, tested on his machine:
> >>>
> >>> First of all, there's DPCD byte 3 bit 6 (NO_AUX_HANDSHAKE_LINK_TRAINING)
> >>> which is documented as follows:
> >>>
> >>>   Does not require AUX CH handshake when the link configuration is
> >>>   already known. [...] The known-good drive current and pre-emphasis
> >>>   level (or those used in the last "full" link training with AUX CH
> >>>   handshake) must be used when the link training is performed without
> >>>   AUX CH handshake.
> >>>
> >>> That bit is set on the MacBook Pros in question.
> >>
> >> I'll check one of the eDP-based systems I've been experimenting on to
> >> see if setting the VGA_SWITCHER_NEEDS_EDP_CONFIG capability in the
> >> handler is sufficient to make i915 avoid poking the AUX channel when
> >> it's mux-switched away. (This would be in addition to hacking the
> >> can_switch() callback in the GPU drivers to allow switching while there
> >> are still active KMS clients for the purposes of this experiment, unless
> >> somebody can point me to a tree with the WIP per-output switching Daniel
> >> Vetter mentioned.
> > Two things: I thought (but not sure) that for the output switching
> > muxes we'd run vgaswitcheroo in a different mode, where it doesn't
> > check whether whether the driver can be killed. Because it wont. On a
> > quick search only thing I've found is the ddc-only switching done by
> > vga_switcheroo_lock/unlock_ddc. Maybe misremembering, but I thought
> > there was more. But been a while I last looked at this all in detail.
> >
> > Wrt per-output switching WIP branch. That would be something you'd
> > need to type ofc, I was just laying out what I think would make sense
> > as a possible path to integrate this into upstream.
> > -Daniel
>
>
> Okay. I misunderstood. When you said that vga-switcheroo could switch
> individual outputs and do so without powering down the
> switched-away-from GPU, I took that to mean that this feature had
> already been implemented somewhere, despite appearances to the contrary
> upstream. I agree that adding per-output switching support to
> vga-switcheroo would be a 

Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Joe Perches
On Fri, 2020-04-03 at 13:47 +0300, Sakari Ailus wrote:
> On Fri, Apr 03, 2020 at 01:24:49PM +0300, Laurent Pinchart wrote:
> > On Fri, Apr 03, 2020 at 12:11:56PM +0300, Sakari Ailus wrote:
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.
[]
> > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
[]
> > > +static noinline_for_stack
> > > +char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> > > + struct printf_spec spec, const char *fmt)
[]
> > > + if (fmt[1] != 'c' || fmt[2] != 'c')

This could check outside a format string if
the %p4 is at the end of the format string.

printk("%p4", fourcc)

So this should verify

if (!(fmt[1] == 'c' && fmt[2] == 'c'))


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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 8:54 PM Joe Perches  wrote:
> On Fri, 2020-04-03 at 19:32 +0200, Mauro Carvalho Chehab wrote:
> > Em Fri, 03 Apr 2020 09:56:42 -0700
> > Joe Perches  escreveu:

> It _might_ be useful to use a CONFIG_MEDIA_SUPPORT guard
> in lib/vsprintf for this.

No need. FourCC, if Sakari makes it more generic, can be used for
other purposes, e.g. printing component names from the chips (not
related to media at all).

-- 
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 18/44] drm/st7586: Use devm_drm_dev_alloc

2020-04-03 Thread David Lechner

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Joe Perches
On Fri, 2020-04-03 at 19:32 +0200, Mauro Carvalho Chehab wrote:
> Em Fri, 03 Apr 2020 09:56:42 -0700
> Joe Perches  escreveu:
[]
> > How many instances of %p4cc could there be?
> 
> That's hard to know... there are several places printing it
> with different ways:
> 
>   $ git grep -i -E "(dev|pr)_(warn|dbg|info)" drivers/media|grep pixf|wc 
> -l
>   6
>   $ git grep -i -E "print" drivers/media|grep pixf|wc -l
>   1
>   $ git grep print_fourcc|wc -l
>   7
>   $ git grep -i -E "(dev|pr)_(warn|dbg|info)" drivers/media|grep 
> pixelf|wc -l
>   10
>   $ git grep -i -E "(dev|pr|v4l)_(warn|dbg|info)" drivers/media|grep 
> format|wc -l
>   60
> 
> I bet there are other places besides the above ones, but the thing is, as 
> we currently lack a standard way, drivers still have their own ideas
> about how to handle it. Each one does it differently.

My thought was ~100 uses was a minimum, rather like %pI6c.

That's pretty close already, so I suppose that's enough.

It _might_ be useful to use a CONFIG_MEDIA_SUPPORT guard
in lib/vsprintf for this.

cheers, Joe

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


Re: [PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:59 AM Daniel Vetter  wrote:
>
> Upcasting using a container_of macro is more typesafe, faster and
> easier for the compiler to optimize.
>
> Signed-off-by: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Gerd Hoffmann 
> Cc: Daniel Vetter 
> Cc: "Noralf Trønnes" 
> Cc: Sam Ravnborg 

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


Re: [PATCH 11/44] drm/v3d: Don't set drm_device->dev_private

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:58 AM Daniel Vetter  wrote:
>
> And switch the helper over to container_of, which is a bunch faster
> than chasing a pointer. Plus allows gcc to see through this maze.
>
> Signed-off-by: Daniel Vetter 

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


Re: [PATCH 12/44] drm/v3d: Use devm_drm_dev_alloc

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:58 AM Daniel Vetter  wrote:
>
> Also allows us to simplify the unroll code since the drm_dev_put
> disappears.
>
> Signed-off-by: Daniel Vetter 

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


Re: [PATCH 24/44] drm/hx8357d: Use devm_drm_dev_alloc

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:59 AM Daniel Vetter  wrote:
>
> Already using devm_drm_dev_init, so very simple replacment.
>
> Signed-off-by: Daniel Vetter 

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


Re: [PATCH 14/44] drm/v3d: Delete v3d_dev->pdev

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:58 AM Daniel Vetter  wrote:
>
> We already have it in v3d_dev->drm.dev with zero additional pointer
> chasing. Personally I don't like duplicated pointers like this
> because:
> - reviewers need to check whether the pointer is for the same or
> different objects if there's multiple
> - compilers have an easier time too
>
> To avoid having to pull in some big headers I implemented the casting
> function as a macro instead of a static inline. Typechecking thanks to
> container_of still assured.
>
> But also a bit a bikeshed, so feel free to ignore.
>
> Signed-off-by: Daniel Vetter 
> Cc: Eric Anholt 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 13/44] drm/v3d: Delete v3d_dev->dev

2020-04-03 Thread Eric Anholt
On Fri, Apr 3, 2020 at 6:58 AM Daniel Vetter  wrote:
>
> We already have it in v3d_dev->drm.dev with zero additional pointer
> chasing. Personally I don't like duplicated pointers like this
> because:
> - reviewers need to check whether the pointer is for the same or
>   different objects if there's multiple
> - compilers have an easier time too
>
> But also a bit a bikeshed, so feel free to ignore.
>
> Signed-off-by: Daniel Vetter 
> Cc: Eric Anholt 

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


Re: [PATCH 23/44] drm/ili9225: Use devm_drm_dev_alloc

2020-04-03 Thread David Lechner

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Mauro Carvalho Chehab
Em Fri, 03 Apr 2020 09:56:42 -0700
Joe Perches  escreveu:

> On Fri, 2020-04-03 at 14:10 +0200, Rasmus Villemoes wrote:
> > On 03/04/2020 11.11, Sakari Ailus wrote:  
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.  
> > 
> > This seems quite niche to me, I'm not sure that belongs in vsprintf.c.
> > What's wrong with having a
> > 
> > char *fourcc_string(char *buf, u32 x)
> > 
> > that formats x into buf and returns buf, so it can be used in a
> > 
> > char buf[8];
> > pr_debug("bla: %s\n", fourcc_string(buf, x))  
> 
> Nothing really, it's a number of uses question.
> 
> For networking code,  print_mac was used before %pM.
> 
> After Linus floated the idea of %p, %pM was
> introduced and all the DECLARE_MAC_BUF/print_mac
> calls were converted.
> 
> %pM did reduce overall object size a fair amount.
> 
> How many instances of %p4cc could there be?

That's hard to know... there are several places printing it
with different ways:

$ git grep -i -E "(dev|pr)_(warn|dbg|info)" drivers/media|grep pixf|wc 
-l
6
$ git grep -i -E "print" drivers/media|grep pixf|wc -l
1
$ git grep print_fourcc|wc -l
7
$ git grep -i -E "(dev|pr)_(warn|dbg|info)" drivers/media|grep 
pixelf|wc -l
10
$ git grep -i -E "(dev|pr|v4l)_(warn|dbg|info)" drivers/media|grep 
format|wc -l
60

I bet there are other places besides the above ones, but the thing is, as 
we currently lack a standard way, drivers still have their own ideas
about how to handle it. Each one does it differently.


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


Re: [PATCH 17/44] drm/st7735r: Use devm_drm_dev_alloc

2020-04-03 Thread David Lechner

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Aside: There was an oddity in the old code, we allocated priv but in
the error path we've freed priv->dbidev ...

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---


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


Re: [PATCH 22/44] drm/ili9341: Use devm_drm_dev_alloc

2020-04-03 Thread David Lechner

On 4/3/20 8:58 AM, Daniel Vetter wrote:

Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: "Noralf Trønnes" 
Cc: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Eric Anholt 
Cc: David Lechner 
---

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


Re: [PATCH v5 3/5] thermal: devfreq_cooling: Use PM QoS to set frequency limits

2020-04-03 Thread Matthias Kaehlcke
Hi Daniel,

On Fri, Apr 03, 2020 at 06:43:20PM +0200, Daniel Lezcano wrote:
> On 18/03/2020 12:45, Lukasz Luba wrote:
> > From: Matthias Kaehlcke 
> > 
> > Now that devfreq supports limiting the frequency range of a device
> > through PM QoS make use of it instead of disabling OPPs that should
> > not be used.
> > 
> > The switch from disabling OPPs to PM QoS introduces a subtle behavioral
> > change in case of conflicting requests (min > max): PM QoS gives
> > precedence to the MIN_FREQUENCY request, while higher OPPs disabled
> > with dev_pm_opp_disable() would override MIN_FREQUENCY.
> > 
> > Signed-off-by: Matthias Kaehlcke 
> > Reviewed-by: Lukasz Luba 
> > Reviewed-by: Chanwoo Choi 
> 
> This patch is standalone, right? If yes, I will apply it.

Yes, it is standalone, please apply

Thanks

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


Re: [PATCH] drm/legacy: Fix type for drm_local_map.offset

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 7:14 PM Linus Torvalds
 wrote:
>
> On Fri, Apr 3, 2020 at 1:29 AM Daniel Vetter  wrote:
> >
> > > Tested-by: Nathan Chancellor  # build
> >
> > This works too, missed it when replying to Linus
> >
> > Reviewed-by: Daniel Vetter 
> >
> > Linus I guess this one is better, but like I explained it really
> > doesn't matter what we do with drm legacy code, it's a horror show
> > that should be disabled on all modern distros anyway. We just keep it
> > because of "never break old uapi".
>
> Ok, That patch from Chris looks fine to me too.
>
> dma_addr_t and resource_size_t aren't the same, but at least
> dma_addr_t should always be the bigger one.
>
> And it does look like nothing else ever takes the address of this
> field, so the ones that might want just the resource_size_t part will
> at least have enough bits.
>
> So I think Chris' patch is the way to go. I'm assuming I'll get it
> through the normal drm tree channels, this doesn't sound _so_ urgent
> that I'd need to expedite that patch into my tree and apply it
> directly.

Ok, sounds good.

Chris can you pls push this to drm-misc-next-fixes? That should be
enough for the pull request train next week.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/legacy: Fix type for drm_local_map.offset

2020-04-03 Thread Linus Torvalds
On Fri, Apr 3, 2020 at 1:29 AM Daniel Vetter  wrote:
>
> > Tested-by: Nathan Chancellor  # build
>
> This works too, missed it when replying to Linus
>
> Reviewed-by: Daniel Vetter 
>
> Linus I guess this one is better, but like I explained it really
> doesn't matter what we do with drm legacy code, it's a horror show
> that should be disabled on all modern distros anyway. We just keep it
> because of "never break old uapi".

Ok, That patch from Chris looks fine to me too.

dma_addr_t and resource_size_t aren't the same, but at least
dma_addr_t should always be the bigger one.

And it does look like nothing else ever takes the address of this
field, so the ones that might want just the resource_size_t part will
at least have enough bits.

So I think Chris' patch is the way to go. I'm assuming I'll get it
through the normal drm tree channels, this doesn't sound _so_ urgent
that I'd need to expedite that patch into my tree and apply it
directly.

Thanks,

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Joe Perches
On Fri, 2020-04-03 at 14:10 +0200, Rasmus Villemoes wrote:
> On 03/04/2020 11.11, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.
> 
> This seems quite niche to me, I'm not sure that belongs in vsprintf.c.
> What's wrong with having a
> 
> char *fourcc_string(char *buf, u32 x)
> 
> that formats x into buf and returns buf, so it can be used in a
> 
> char buf[8];
> pr_debug("bla: %s\n", fourcc_string(buf, x))

Nothing really, it's a number of uses question.

For networking code,  print_mac was used before %pM.

After Linus floated the idea of %p, %pM was
introduced and all the DECLARE_MAC_BUF/print_mac
calls were converted.

%pM did reduce overall object size a fair amount.

How many instances of %p4cc could there be?


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


Re: amdgpu dropping load callback triggers WARN_ON in __drm_mode_object_add

2020-04-03 Thread Daniel Vetter
5 seconds on irc and I've found the splat ...

drm_properties need to be created at driver load time, upfront. You
can attach them to hotpluggable drm_connector objects later on, but
only before calling drm_connector_register().

The warning exists because i915 had the same bug :-)
-Daniel

On Fri, Apr 3, 2020 at 6:38 PM Daniel Vetter  wrote:
>
> On Fri, Apr 3, 2020 at 6:30 PM Michel Dänzer  wrote:
> >
> >
> > I'm getting the attached WARNING splats since amdgpu dropped its load
> > callback. They're from
> >
> > WARN_ON(!dev->driver->load && dev->registered && !obj_free_cb);
> >
> > in __drm_mode_object_add.
> >
> > I'm not sure how to address this, since obj_free_cb is always NULL here,
> > and I don't think MST connectors getting added after dev->registered is
> > set can be avoided?
>
> I need the full splat, otherwise can't tell you what's going wrong.
> There's 2 cases:
>
> - hotpluggable stuff, which has the obj_free_cb set. Essentially MST 
> connectors.
>
> - not-hotpluggable stuff, which is everything else. They don't have
> obj_free_cb set. It's a driver bug to register those after the overall
> drm_device has already been registered and published to userspace.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: amdgpu dropping load callback triggers WARN_ON in __drm_mode_object_add

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 6:30 PM Michel Dänzer  wrote:
>
>
> I'm getting the attached WARNING splats since amdgpu dropped its load
> callback. They're from
>
> WARN_ON(!dev->driver->load && dev->registered && !obj_free_cb);
>
> in __drm_mode_object_add.
>
> I'm not sure how to address this, since obj_free_cb is always NULL here,
> and I don't think MST connectors getting added after dev->registered is
> set can be avoided?

I need the full splat, otherwise can't tell you what's going wrong.
There's 2 cases:

- hotpluggable stuff, which has the obj_free_cb set. Essentially MST connectors.

- not-hotpluggable stuff, which is everything else. They don't have
obj_free_cb set. It's a driver bug to register those after the overall
drm_device has already been registered and published to userspace.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


amdgpu dropping load callback triggers WARN_ON in __drm_mode_object_add

2020-04-03 Thread Michel Dänzer

I'm getting the attached WARNING splats since amdgpu dropped its load
callback. They're from

WARN_ON(!dev->driver->load && dev->registered && !obj_free_cb);

in __drm_mode_object_add.

I'm not sure how to address this, since obj_free_cb is always NULL here,
and I don't think MST connectors getting added after dev->registered is
set can be avoided?


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
Apr  3 17:32:55 thor kernel: [1.593399][  T129] [ cut here 
]
Apr  3 17:32:55 thor kernel: [1.593409][  T129] WARNING: CPU: 6 PID: 129 at 
drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xa1/0xb0 [drm]
Apr  3 17:32:55 thor kernel: [1.593409][  T129] Modules linked in: 
crc32c_intel(E) amdgpu(E) mfd_core(E) i2c_algo_bit(E) aesni_intel(E) ttm(E) 
glue_helper(E) gpu_sched(E) crypto_simd(E) cryptd(E) drm_kms_helper(E) cec(E) 
xhci_pci(E) evdev(E) psmouse(E) rc_core(E) serio_raw(E) xhci_hcd(E) usbcore(E) 
drm(E) nvme(E) nvme_core(E) t10_pi(E)
Apr  3 17:32:55 thor kernel: [1.593409][  T129] CPU: 6 PID: 129 Comm: 
kworker/6:1 Tainted: GE 5.6.0+ #1
Apr  3 17:32:55 thor kernel: [1.593409][  T129] Hardware name: LENOVO 
20NFGE/20NFGE, BIOS R11ET30W (1.10 ) 10/11/2019
Apr  3 17:32:55 thor kernel: [1.593409][  T129] Workqueue: events_long 
drm_dp_mst_link_probe_work [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129] RIP: 
0010:__drm_mode_object_add+0xa1/0xb0 [drm]
Apr  3 17:32:55 thor kernel: [1.593409][  T129] Code: 00 00 4c 89 f7 e8 3f 
e4 46 ca 85 db b8 00 00 00 00 0f 4e c3 5b 5d 41 5c 41 5d 41 5e 41 5f c3 80 7f 
40 00 74 8c 4d 85 c0 75 87 <0f> 0b eb 83 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 
44 00 00 45 31
Apr  3 17:32:55 thor kernel: [1.593409][  T129] RSP: 0018:993f803e7ad8 
EFLAGS: 00010246
Apr  3 17:32:55 thor kernel: [1.593409][  T129] RAX: c0c39120 RBX: 
896166b1d000 RCX: 0001
Apr  3 17:32:55 thor kernel: [1.593409][  T129] RDX: b0b0b0b0 RSI: 
896164a7b590 RDI: 896166b1d000
Apr  3 17:32:55 thor kernel: [1.593409][  T129] RBP: 896164a7b590 R08: 
 R09: 896164a81600
Apr  3 17:32:55 thor kernel: [1.593409][  T129] R10:  R11: 
89626e0bedd7 R12: b0b0b0b0
Apr  3 17:32:55 thor kernel: [1.593409][  T129] R13:  R14: 
c0215091 R15: 0001
Apr  3 17:32:55 thor kernel: [1.593409][  T129] FS:  () 
GS:896170b8() knlGS:
Apr  3 17:32:55 thor kernel: [1.593409][  T129] CS:  0010 DS:  ES:  
CR0: 80050033
Apr  3 17:32:55 thor kernel: [1.593409][  T129] CR2: 7ffc35448de8 CR3: 
0003a6ddc000 CR4: 003406e0
Apr  3 17:32:55 thor kernel: [1.593409][  T129] Call Trace:
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_property_create+0xcd/0x1a0 [drm]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_property_create_range+0x1a/0x40 [drm]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_connector_attach_max_bpc_property+0x62/0x80 [drm]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
amdgpu_dm_connector_init_helper+0x1cd/0x280 [amdgpu]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
dm_dp_add_mst_connector+0xbb/0x1a0 [amdgpu]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_dp_mst_port_add_connector+0x108/0x1a0 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? 
dm_dp_aux_transfer+0x8f/0xf0 [amdgpu]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? 
drm_dp_dpcd_access+0xdf/0x110 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? 
drm_dp_mst_add_port+0x2f/0xe0 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_dp_send_link_address+0x368/0x990 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? 
preempt_count_sub+0x9b/0xd0
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_dp_check_and_send_link_address+0xad/0xd0 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
drm_dp_mst_link_probe_work+0x94/0x180 [drm_kms_helper]
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  
process_one_work+0x1ab/0x3a0
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  worker_thread+0x50/0x3a0
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  kthread+0xf9/0x130
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? 
process_one_work+0x3a0/0x3a0
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ? kthread_park+0x90/0x90
Apr  3 17:32:55 thor kernel: [1.593409][  T129]  ret_from_fork+0x22/0x40
Apr  3 17:32:55 thor kernel: [1.593409][  T129] ---[ end trace 
f1dd3fe3af522768 ]---
Apr  3 17:32:55 thor kernel: [1.593816][  T129] [ cut here 
]
Apr  3 17:32:55 thor kernel: [1.593829][  T129] WARNING: CPU: 6 

RE: mainline/master bisection: baseline.login on peach-pi

2020-04-03 Thread Deucher, Alexander
[AMD Official Use Only - Internal Distribution Only]

> -Original Message-
> From: Guillaume Tucker 
> Sent: Friday, April 3, 2020 10:14 AM
> To: Michael J. Ruhl ; Shane Francis
> ; Deucher, Alexander
> 
> Cc: kerne...@groups.io; dri-devel@lists.freedesktop.org; linux-
> ker...@vger.kernel.org; Tom Murphy ; Joerg Roedel
> ; David Airlie ; Maarten Lankhorst
> ; Daniel Vetter ;
> Maxime Ripard ; Enric Balletbo i Serra
> 
> Subject: Re: mainline/master bisection: baseline.login on peach-pi
> 
> Please see the bisection report below about a boot failure.
> 
> Reports aren't automatically sent to the public while we're trialing new
> bisection features on kernelci.org but this one looks valid.
> 
> This bisection was run with exynos_defconfig but the issue can also be
> reproduced with multi_v7_defconfig.  It doesn't appear to be affecting any
> other platforms on kernelci.org.  This looks like a DRM driver problem, the
> kernel image boots fine without the modules installed.  It actually started
> failing on Tuesday in mainline.

Fixed with this patch:
https://patchwork.freedesktop.org/patch/359081/
Just trying to get this into 5.7 and stable.  I was waiting for a 5.6 back 
merge to drm-misc-next-fixes, but I could send it as a separate PR if Dave or 
Daniel prefer.

Alex


> 
> Guillaume
> 
> On 02/04/2020 19:38, kernelci.org bot wrote:
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> > * This automated bisection report was sent to you on the basis  *
> > * that you may be involved with the breaking commit it has  *
> > * found.  No manual investigation has been done to verify it,   *
> > * and the root cause of the problem may be somewhere else.  *
> > *   *
> > * If you do send a fix, please include this trailer:*
> > *   Reported-by: "kernelci.org bot"   *
> > *   *
> > * Hope this helps!  *
> > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> >
> > mainline/master bisection: baseline.login on peach-pi
> >
> > Summary:
> >   Start:  56a451b780676 Merge tag 'ntb-5.7' of
> git://github.com/jonmason/ntb
> >   Plain log:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstor
> age.kernelci.org%2F%2Fmainline%2Fmaster%2Fv5.6-3277-
> g56a451b78067%2Farm%2Fexynos_defconfig%2Fgcc-8%2Flab-
> collabora%2Fbaseline-exynos5800-peach-
> pi.txtdata=02%7C01%7Calexander.deucher%40amd.com%7Ca1d322a4
> f72744bfe14208d7d7d9398f%7C3dd8961fe4884e608e11a82d994e183d%7C0%
> 7C0%7C637215200294572061sdata=pJ%2F8FHi6grBy4aGUuL%2F9%2Bj
> %2FVqrWJfStjBCaDUnBoUMI%3Dreserved=0
> >   HTML log:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstor
> age.kernelci.org%2F%2Fmainline%2Fmaster%2Fv5.6-3277-
> g56a451b78067%2Farm%2Fexynos_defconfig%2Fgcc-8%2Flab-
> collabora%2Fbaseline-exynos5800-peach-
> pi.htmldata=02%7C01%7Calexander.deucher%40amd.com%7Ca1d322
> a4f72744bfe14208d7d7d9398f%7C3dd8961fe4884e608e11a82d994e183d%7C0
> %7C0%7C637215200294572061sdata=pSoe45cMygTcp0UXe3DY%2F6jeI
> EChQ5FwsPO32A4%2Bhh8%3Dreserved=0
> >   Result: 42e67b479eab6 drm/prime: use dma length macro when
> mapping sg
> >
> > Checks:
> >   revert: PASS
> >   verify: PASS
> >
> > Parameters:
> >   Tree:   mainline
> >   URL:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git&
> amp;data=02%7C01%7Calexander.deucher%40amd.com%7Ca1d322a4f72744
> bfe14208d7d7d9398f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7
> C637215200294572061sdata=MW1XaSNAKSVByF8eOG2%2Bv59Mqjwi
> 7Cu6QaU87AZbka8%3Dreserved=0
> >   Branch: master
> >   Target: peach-pi
> >   CPU arch:   arm
> >   Lab:lab-collabora
> >   Compiler:   gcc-8
> >   Config: exynos_defconfig
> >   Test case:  baseline.login
> >
> > Breaking commit found:
> >
> > --
> > - commit 42e67b479eab6d26459b80b4867298232b0435e7
> > Author: Shane Francis 
> > Date:   Wed Mar 25 09:07:39 2020 +
> >
> > drm/prime: use dma length macro when mapping sg
> >
> > As dma_map_sg can reorganize scatter-gather lists in a
> > way that can cause some later segments to be empty we should
> > always use the sg_dma_len macro to fetch the actual length.
> >
> > This could now be 0 and not need to be mapped to a page or
> > address array
> >
> > Fixes: be62dbf554c5 ("iommu/amd: Convert AMD iommu driver to the
> dma-iommu api")
> > Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D206461data=02%7C01%7Cal
> exander.deucher%40amd.com%7Ca1d322a4f72744bfe14208d7d7d9398f%7C
> 3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637215200294572061
> 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 5:15 PM Daniel Vetter  wrote:
>
> On Fri, Apr 3, 2020 at 5:06 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Fri, Apr 03, 2020 at 04:47:04PM +0200, Daniel Vetter wrote:
> > > On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
> > >  wrote:
> > > >
> > > > On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > > > > In drm we've added nice drm_device (the main gpu driver thing, which
> > > > > also represents the userspace interfaces and has everything else
> > > > > dangling off it) init functions using devres, devm_drm_dev_init and
> > > > > soon devm_drm_dev_alloc (this patch series adds that).
> > > > >
> > > > > A slight trouble is that drm_device itself holds a reference on the
> > > > > struct device it's sitting on top (for sysfs links and dmesg debug and
> > > > > lots of other things), so there's a reference loop. For real drivers
> > > > > this is broken at remove/unplug time, where all devres resources are
> > > > > released device_release_driver(), before the final device reference is
> > > > > dropped. So far so good.
> > > > >
> > > > > There's 2 exceptions:
> > > > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> > > > >   platform device to make them look more like normal devices to
> > > > >   userspace. These aren't drivers in the driver model sense, we simple
> > > > >   create a platform_device and register it.
> > > >
> > > > That's a horrid abuse of platform devices, just use a "virtual" device
> > > > please, create/remove it when you need it, and all should be fine.
> > > >
> > > > > - drm/i915/selftests, where we create minimal mock devices, and again
> > > > >   the selftests aren't proper drivers in the driver model sense.
> > > >
> > > > Again, virtual devices are best to use for this.
> > >
> > > Hm yeah, I guess we should fix that. i915 selftests do use raw struct
> > > device though, and it's not really the problem.
> > >
> > > > > For these two cases the reference loop isn't broken, because devres is
> > > > > only cleaned up when the last device reference is dropped. But that's
> > > > > not happening, because the drm_device holds that last struct device
> > > > > reference.
> > > > >
> > > > > Thus far this wasn't a problem since the above cases simply
> > > > > hand-rolled their cleanup code. But I want to convert all drivers over
> > > > > to the devm_ versions, hence it would be really nice if these
> > > > > virtual/fake/mock uses-cases could also be managed with devres
> > > > > cleanup.
> > > > >
> > > > > I see three possible approaches:
> > > > >
> > > > > - Clean up devres from device_del (or platform_device_unregister) even
> > > > >   when no driver is bound. This seems like the simplest solution, but
> > > > >   also the one with the widest impact, and what this patch implements.
> > > > >   We might want to include more of the cleanup than just
> > > > >   devres_release_all, but this is all I need to get my use case going.
> > > >
> > > > After device_del, you should never be using that structure again anyway.
> > > > So why is there any "resource leak"?  You can't recycle the structure,
> > > > and you can't assign it to anything else, so eventually you have to do
> > > > a final put on the thing, which will free up the resources.
> > >
> > > I guess I should have spent more time explaining this. There's two
> > > references involved:
> > >
> > > - drm_device->dev points at the underlying struct device. The
> > > drm_device holds a reference until it's fully cleaned up, so that we
> > > can do nice stuff like use dev_ versions of printk functions, and you
> > > always know that there's not going to be a use-after free.
> > >
> > > - now the other dependency is that as long as the device exists (not
> > > just in memory, but in the device model, i.e. between device_add() and
> > > device_del()) the drm_device should exist. So what we do in the
> > > bus-specific remove/disconnect callback is that we call
> > > drm_dev_unregister(). This drops the drm_device refcount that the drm
> > > chardev was holding, to make sure that an open() on the chardev can
> > > actually get at the memory without going boom. Then after the
> > > drm_dev_unregister, again in the remove/disconnect callback of th
> > > driver, there's a drm_dev_put(). Which might or might not be the final
> > > drm_dev_put(), since if there's currently some open fd we keep the
> > > refcount elevated, to avoid oopses and fun stuff like that. And
> > > drm_device pointers get shared very widely, thanks to fun stuff like
> > > dma_buf buffer sharing and dma_fence hw syncpt sharing across
> > > processes and drivers.
> > >
> > > Once the final drm_dev_put() is called we also end up calling
> > > put_device() and everything is happy.
> > >
> > > So far so good.
> > >
> > > Now the problem is that refcount is hard, and most drm drivers get it
> > > wrong in some fashion or another, so I'm trying to solve all this with
> > > more magic.
> >
> > Wait, 

Re: [PATCH v3 1/4] drm/mediatek: Move tz_disabled from mtk_hdmi_phy to mtk_hdmi driver

2020-04-03 Thread Chun-Kuang Hu
Chunfeng Yun  於 2020年4月3日 週五 上午10:59寫道:
>
> On Thu, 2020-04-02 at 20:49 +0800, Chun-Kuang Hu wrote:
> > Hi, Matthias:
> >
> > Matthias Brugger  於 2020年4月1日 週三 下午11:53寫道:
> > >
> > >
> > >
> > > On 01/04/2020 04:16, Chunfeng Yun wrote:
> > > > On Tue, 2020-03-31 at 23:57 +0800, Chun-Kuang Hu wrote:
> > > >> From: CK Hu 
> > > >>
> > > >> tz_disabled is used to control mtk_hdmi output signal, but this 
> > > >> variable
> > > >> is stored in mtk_hdmi_phy and mtk_hdmi_phy does not use it. So move
> > > >> tz_disabled to mtk_hdmi where it's used.
> > > >>
> > > >> Signed-off-by: CK Hu 
> > > >> Signed-off-by: Chun-Kuang Hu 
> > > >> ---
> > > >>  drivers/gpu/drm/mediatek/mtk_hdmi.c   | 22 ---
> > > >>  drivers/gpu/drm/mediatek/mtk_hdmi_phy.h   |  1 -
> > > >>  .../gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c|  1 -
> > > >>  3 files changed, 19 insertions(+), 5 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
> > > >> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > >> index 5e4a4dbda443..878433c09c9b 100644
> > > >> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > >> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > >> @@ -144,11 +144,16 @@ struct hdmi_audio_param {
> > > >>  struct hdmi_codec_params codec_params;
> > > >>  };
> > > >>
> > > >> +struct mtk_hdmi_conf {
> > > >> +bool tz_disabled;
> > > >> +};
> > > >> +
> > > >>  struct mtk_hdmi {
> > > >>  struct drm_bridge bridge;
> > > >>  struct drm_bridge *next_bridge;
> > > >>  struct drm_connector conn;
> > > >>  struct device *dev;
> > > >> +const struct mtk_hdmi_conf *conf;
> > > >>  struct phy *phy;
> > > >>  struct device *cec_dev;
> > > >>  struct i2c_adapter *ddc_adpt;
> > > >> @@ -230,7 +235,6 @@ static void mtk_hdmi_hw_vid_black(struct mtk_hdmi 
> > > >> *hdmi, bool black)
> > > >>  static void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool 
> > > >> enable)
> > > >>  {
> > > >>  struct arm_smccc_res res;
> > > >> -struct mtk_hdmi_phy *hdmi_phy = phy_get_drvdata(hdmi->phy);
> > > >>
> > > >>  /*
> > > >>   * MT8173 HDMI hardware has an output control bit to 
> > > >> enable/disable HDMI
> > > >> @@ -238,7 +242,7 @@ static void mtk_hdmi_hw_make_reg_writable(struct 
> > > >> mtk_hdmi *hdmi, bool enable)
> > > >>   * The ARM trusted firmware provides an API for the HDMI driver 
> > > >> to set
> > > >>   * this control bit to enable HDMI output in supervisor mode.
> > > >>   */
> > > >> -if (hdmi_phy->conf && hdmi_phy->conf->tz_disabled)
> > > >> +if (hdmi->conf->tz_disabled)
> > >
> > > Wouldn't we need to check:
> > > if (hdmi->conf && hdmi->conf->tz_disabled)
> >
> > My design is: hdmi->conf would not be NULL.
> >
> > >
> > > >>  regmap_update_bits(hdmi->sys_regmap,
> > > >> hdmi->sys_offset + HDMI_SYS_CFG20,
> > > >> 0x80008005, enable ? 0x8005 : 
> > > >> 0x8000);
> > > >> @@ -1688,6 +1692,7 @@ static int mtk_drm_hdmi_probe(struct 
> > > >> platform_device *pdev)
> > > >>  return -ENOMEM;
> > > >>
> > > >>  hdmi->dev = dev;
> > > >> +hdmi->conf = of_device_get_match_data(dev);
> > > >>
> > > >>  ret = mtk_hdmi_dt_parse_pdata(hdmi, pdev);
> > > >>  if (ret)
> > > >> @@ -1765,8 +1770,19 @@ static int mtk_hdmi_resume(struct device *dev)
> > > >>  static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
> > > >>   mtk_hdmi_suspend, mtk_hdmi_resume);
> > > >>
> > > >> +static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701 = {
> > > >> +.tz_disabled = true,
> > > >> +};
> > > >> +
> > > >> +static const struct mtk_hdmi_conf mtk_hdmi_conf_mt8173;
> > > >> +
> > > >>  static const struct of_device_id mtk_drm_hdmi_of_ids[] = {
> > > >> -{ .compatible = "mediatek,mt8173-hdmi", },
> > > >> +{ .compatible = "mediatek,mt2701-hdmi",
> > > >> +  .data = _hdmi_conf_mt2701,
> > > >> +},
> > > >> +{ .compatible = "mediatek,mt8173-hdmi",
> > > >> +  .data = _hdmi_conf_mt8173,
> > >
> > > We don't have any data? Then we should set data to NULL instead.
> >
> > My design is data would not be NULL, so I need not to check whether it
> > is NULL in driver.
> But we don't need .data for mt8173, it's better to set it to NULL.

OK, in the view of reducing the code size, setting it to NULL would
make code size smaller.
I would modify this in next version.

Regards,
Chun-Kuang.

>
> >
> > Regards,
> > CK
> >
> > >
> > > Regards,
> > > Matthias
> > >
> > > >> +},
> > > >>  {}
> > > >>  };
> > > >>
> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h 
> > > >> b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
> > > >> index 2d8b3182470d..fc1c2efd1128 100644
> > > >> --- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
> > > >> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
> > > >> @@ -20,7 +20,6 @@
> > > >>  struct mtk_hdmi_phy;
> > > >>
> > > >>  struct mtk_hdmi_phy_conf {
> > > >> -bool 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 5:06 PM Greg Kroah-Hartman
 wrote:
>
> On Fri, Apr 03, 2020 at 04:47:04PM +0200, Daniel Vetter wrote:
> > On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
> >  wrote:
> > >
> > > On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > > > In drm we've added nice drm_device (the main gpu driver thing, which
> > > > also represents the userspace interfaces and has everything else
> > > > dangling off it) init functions using devres, devm_drm_dev_init and
> > > > soon devm_drm_dev_alloc (this patch series adds that).
> > > >
> > > > A slight trouble is that drm_device itself holds a reference on the
> > > > struct device it's sitting on top (for sysfs links and dmesg debug and
> > > > lots of other things), so there's a reference loop. For real drivers
> > > > this is broken at remove/unplug time, where all devres resources are
> > > > released device_release_driver(), before the final device reference is
> > > > dropped. So far so good.
> > > >
> > > > There's 2 exceptions:
> > > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> > > >   platform device to make them look more like normal devices to
> > > >   userspace. These aren't drivers in the driver model sense, we simple
> > > >   create a platform_device and register it.
> > >
> > > That's a horrid abuse of platform devices, just use a "virtual" device
> > > please, create/remove it when you need it, and all should be fine.
> > >
> > > > - drm/i915/selftests, where we create minimal mock devices, and again
> > > >   the selftests aren't proper drivers in the driver model sense.
> > >
> > > Again, virtual devices are best to use for this.
> >
> > Hm yeah, I guess we should fix that. i915 selftests do use raw struct
> > device though, and it's not really the problem.
> >
> > > > For these two cases the reference loop isn't broken, because devres is
> > > > only cleaned up when the last device reference is dropped. But that's
> > > > not happening, because the drm_device holds that last struct device
> > > > reference.
> > > >
> > > > Thus far this wasn't a problem since the above cases simply
> > > > hand-rolled their cleanup code. But I want to convert all drivers over
> > > > to the devm_ versions, hence it would be really nice if these
> > > > virtual/fake/mock uses-cases could also be managed with devres
> > > > cleanup.
> > > >
> > > > I see three possible approaches:
> > > >
> > > > - Clean up devres from device_del (or platform_device_unregister) even
> > > >   when no driver is bound. This seems like the simplest solution, but
> > > >   also the one with the widest impact, and what this patch implements.
> > > >   We might want to include more of the cleanup than just
> > > >   devres_release_all, but this is all I need to get my use case going.
> > >
> > > After device_del, you should never be using that structure again anyway.
> > > So why is there any "resource leak"?  You can't recycle the structure,
> > > and you can't assign it to anything else, so eventually you have to do
> > > a final put on the thing, which will free up the resources.
> >
> > I guess I should have spent more time explaining this. There's two
> > references involved:
> >
> > - drm_device->dev points at the underlying struct device. The
> > drm_device holds a reference until it's fully cleaned up, so that we
> > can do nice stuff like use dev_ versions of printk functions, and you
> > always know that there's not going to be a use-after free.
> >
> > - now the other dependency is that as long as the device exists (not
> > just in memory, but in the device model, i.e. between device_add() and
> > device_del()) the drm_device should exist. So what we do in the
> > bus-specific remove/disconnect callback is that we call
> > drm_dev_unregister(). This drops the drm_device refcount that the drm
> > chardev was holding, to make sure that an open() on the chardev can
> > actually get at the memory without going boom. Then after the
> > drm_dev_unregister, again in the remove/disconnect callback of th
> > driver, there's a drm_dev_put(). Which might or might not be the final
> > drm_dev_put(), since if there's currently some open fd we keep the
> > refcount elevated, to avoid oopses and fun stuff like that. And
> > drm_device pointers get shared very widely, thanks to fun stuff like
> > dma_buf buffer sharing and dma_fence hw syncpt sharing across
> > processes and drivers.
> >
> > Once the final drm_dev_put() is called we also end up calling
> > put_device() and everything is happy.
> >
> > So far so good.
> >
> > Now the problem is that refcount is hard, and most drm drivers get it
> > wrong in some fashion or another, so I'm trying to solve all this with
> > more magic.
>
> Wait, no.  Fix the drivers.  Seriously, don't try to "bust" the
> reference count logic here.

I guess still not clear. What I'm doing is fixing the drivers. But
because they all get it wrong, I'm trying to hide as much of the
refcounting as possible 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Greg Kroah-Hartman
On Fri, Apr 03, 2020 at 04:47:04PM +0200, Daniel Vetter wrote:
> On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > > In drm we've added nice drm_device (the main gpu driver thing, which
> > > also represents the userspace interfaces and has everything else
> > > dangling off it) init functions using devres, devm_drm_dev_init and
> > > soon devm_drm_dev_alloc (this patch series adds that).
> > >
> > > A slight trouble is that drm_device itself holds a reference on the
> > > struct device it's sitting on top (for sysfs links and dmesg debug and
> > > lots of other things), so there's a reference loop. For real drivers
> > > this is broken at remove/unplug time, where all devres resources are
> > > released device_release_driver(), before the final device reference is
> > > dropped. So far so good.
> > >
> > > There's 2 exceptions:
> > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> > >   platform device to make them look more like normal devices to
> > >   userspace. These aren't drivers in the driver model sense, we simple
> > >   create a platform_device and register it.
> >
> > That's a horrid abuse of platform devices, just use a "virtual" device
> > please, create/remove it when you need it, and all should be fine.
> >
> > > - drm/i915/selftests, where we create minimal mock devices, and again
> > >   the selftests aren't proper drivers in the driver model sense.
> >
> > Again, virtual devices are best to use for this.
> 
> Hm yeah, I guess we should fix that. i915 selftests do use raw struct
> device though, and it's not really the problem.
> 
> > > For these two cases the reference loop isn't broken, because devres is
> > > only cleaned up when the last device reference is dropped. But that's
> > > not happening, because the drm_device holds that last struct device
> > > reference.
> > >
> > > Thus far this wasn't a problem since the above cases simply
> > > hand-rolled their cleanup code. But I want to convert all drivers over
> > > to the devm_ versions, hence it would be really nice if these
> > > virtual/fake/mock uses-cases could also be managed with devres
> > > cleanup.
> > >
> > > I see three possible approaches:
> > >
> > > - Clean up devres from device_del (or platform_device_unregister) even
> > >   when no driver is bound. This seems like the simplest solution, but
> > >   also the one with the widest impact, and what this patch implements.
> > >   We might want to include more of the cleanup than just
> > >   devres_release_all, but this is all I need to get my use case going.
> >
> > After device_del, you should never be using that structure again anyway.
> > So why is there any "resource leak"?  You can't recycle the structure,
> > and you can't assign it to anything else, so eventually you have to do
> > a final put on the thing, which will free up the resources.
> 
> I guess I should have spent more time explaining this. There's two
> references involved:
> 
> - drm_device->dev points at the underlying struct device. The
> drm_device holds a reference until it's fully cleaned up, so that we
> can do nice stuff like use dev_ versions of printk functions, and you
> always know that there's not going to be a use-after free.
> 
> - now the other dependency is that as long as the device exists (not
> just in memory, but in the device model, i.e. between device_add() and
> device_del()) the drm_device should exist. So what we do in the
> bus-specific remove/disconnect callback is that we call
> drm_dev_unregister(). This drops the drm_device refcount that the drm
> chardev was holding, to make sure that an open() on the chardev can
> actually get at the memory without going boom. Then after the
> drm_dev_unregister, again in the remove/disconnect callback of th
> driver, there's a drm_dev_put(). Which might or might not be the final
> drm_dev_put(), since if there's currently some open fd we keep the
> refcount elevated, to avoid oopses and fun stuff like that. And
> drm_device pointers get shared very widely, thanks to fun stuff like
> dma_buf buffer sharing and dma_fence hw syncpt sharing across
> processes and drivers.
> 
> Once the final drm_dev_put() is called we also end up calling
> put_device() and everything is happy.
> 
> So far so good.
> 
> Now the problem is that refcount is hard, and most drm drivers get it
> wrong in some fashion or another, so I'm trying to solve all this with
> more magic.

Wait, no.  Fix the drivers.  Seriously, don't try to "bust" the
reference count logic here.

> Since all drivers need to have a drm_dev_put() at the end of their
> driver's remove/disconnect callback we've added a devm_drm_dev_init
> function which registers a devres action to do that drm_dev_put() at
> device_del time (which might or might not be the final drm_dev_put()).
> Nothing has changed thus far.
> 
> Now this works really well because when you have a real driver model
> 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Greg Kroah-Hartman
On Fri, Apr 03, 2020 at 04:51:33PM +0200, Daniel Vetter wrote:
> On Fri, Apr 3, 2020 at 4:47 PM Daniel Vetter  wrote:
> >
> > On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
> >  wrote:
> > >
> > > On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > > > In drm we've added nice drm_device (the main gpu driver thing, which
> > > > also represents the userspace interfaces and has everything else
> > > > dangling off it) init functions using devres, devm_drm_dev_init and
> > > > soon devm_drm_dev_alloc (this patch series adds that).
> > > >
> > > > A slight trouble is that drm_device itself holds a reference on the
> > > > struct device it's sitting on top (for sysfs links and dmesg debug and
> > > > lots of other things), so there's a reference loop. For real drivers
> > > > this is broken at remove/unplug time, where all devres resources are
> > > > released device_release_driver(), before the final device reference is
> > > > dropped. So far so good.
> > > >
> > > > There's 2 exceptions:
> > > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> > > >   platform device to make them look more like normal devices to
> > > >   userspace. These aren't drivers in the driver model sense, we simple
> > > >   create a platform_device and register it.
> > >
> > > That's a horrid abuse of platform devices, just use a "virtual" device
> > > please, create/remove it when you need it, and all should be fine.
> > >
> > > > - drm/i915/selftests, where we create minimal mock devices, and again
> > > >   the selftests aren't proper drivers in the driver model sense.
> > >
> > > Again, virtual devices are best to use for this.
> >
> > Hm yeah, I guess we should fix that. i915 selftests do use raw struct
> > device though, and it's not really the problem.
> >
> > > > For these two cases the reference loop isn't broken, because devres is
> > > > only cleaned up when the last device reference is dropped. But that's
> > > > not happening, because the drm_device holds that last struct device
> > > > reference.
> > > >
> > > > Thus far this wasn't a problem since the above cases simply
> > > > hand-rolled their cleanup code. But I want to convert all drivers over
> > > > to the devm_ versions, hence it would be really nice if these
> > > > virtual/fake/mock uses-cases could also be managed with devres
> > > > cleanup.
> > > >
> > > > I see three possible approaches:
> > > >
> > > > - Clean up devres from device_del (or platform_device_unregister) even
> > > >   when no driver is bound. This seems like the simplest solution, but
> > > >   also the one with the widest impact, and what this patch implements.
> > > >   We might want to include more of the cleanup than just
> > > >   devres_release_all, but this is all I need to get my use case going.
> > >
> > > After device_del, you should never be using that structure again anyway.
> > > So why is there any "resource leak"?  You can't recycle the structure,
> > > and you can't assign it to anything else, so eventually you have to do
> > > a final put on the thing, which will free up the resources.
> >
> > I guess I should have spent more time explaining this. There's two
> > references involved:
> >
> > - drm_device->dev points at the underlying struct device. The
> > drm_device holds a reference until it's fully cleaned up, so that we
> > can do nice stuff like use dev_ versions of printk functions, and you
> > always know that there's not going to be a use-after free.
> >
> > - now the other dependency is that as long as the device exists (not
> > just in memory, but in the device model, i.e. between device_add() and
> > device_del()) the drm_device should exist. So what we do in the
> > bus-specific remove/disconnect callback is that we call
> > drm_dev_unregister(). This drops the drm_device refcount that the drm
> > chardev was holding, to make sure that an open() on the chardev can
> > actually get at the memory without going boom. Then after the
> > drm_dev_unregister, again in the remove/disconnect callback of th
> > driver, there's a drm_dev_put(). Which might or might not be the final
> > drm_dev_put(), since if there's currently some open fd we keep the
> > refcount elevated, to avoid oopses and fun stuff like that. And
> > drm_device pointers get shared very widely, thanks to fun stuff like
> > dma_buf buffer sharing and dma_fence hw syncpt sharing across
> > processes and drivers.
> >
> > Once the final drm_dev_put() is called we also end up calling
> > put_device() and everything is happy.
> >
> > So far so good.
> >
> > Now the problem is that refcount is hard, and most drm drivers get it
> > wrong in some fashion or another, so I'm trying to solve all this with
> > more magic.
> >
> > Since all drivers need to have a drm_dev_put() at the end of their
> > driver's remove/disconnect callback we've added a devm_drm_dev_init
> > function which registers a devres action to do that drm_dev_put() at
> > device_del time (which might or 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 4:47 PM Daniel Vetter  wrote:
>
> On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > > In drm we've added nice drm_device (the main gpu driver thing, which
> > > also represents the userspace interfaces and has everything else
> > > dangling off it) init functions using devres, devm_drm_dev_init and
> > > soon devm_drm_dev_alloc (this patch series adds that).
> > >
> > > A slight trouble is that drm_device itself holds a reference on the
> > > struct device it's sitting on top (for sysfs links and dmesg debug and
> > > lots of other things), so there's a reference loop. For real drivers
> > > this is broken at remove/unplug time, where all devres resources are
> > > released device_release_driver(), before the final device reference is
> > > dropped. So far so good.
> > >
> > > There's 2 exceptions:
> > > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> > >   platform device to make them look more like normal devices to
> > >   userspace. These aren't drivers in the driver model sense, we simple
> > >   create a platform_device and register it.
> >
> > That's a horrid abuse of platform devices, just use a "virtual" device
> > please, create/remove it when you need it, and all should be fine.
> >
> > > - drm/i915/selftests, where we create minimal mock devices, and again
> > >   the selftests aren't proper drivers in the driver model sense.
> >
> > Again, virtual devices are best to use for this.
>
> Hm yeah, I guess we should fix that. i915 selftests do use raw struct
> device though, and it's not really the problem.
>
> > > For these two cases the reference loop isn't broken, because devres is
> > > only cleaned up when the last device reference is dropped. But that's
> > > not happening, because the drm_device holds that last struct device
> > > reference.
> > >
> > > Thus far this wasn't a problem since the above cases simply
> > > hand-rolled their cleanup code. But I want to convert all drivers over
> > > to the devm_ versions, hence it would be really nice if these
> > > virtual/fake/mock uses-cases could also be managed with devres
> > > cleanup.
> > >
> > > I see three possible approaches:
> > >
> > > - Clean up devres from device_del (or platform_device_unregister) even
> > >   when no driver is bound. This seems like the simplest solution, but
> > >   also the one with the widest impact, and what this patch implements.
> > >   We might want to include more of the cleanup than just
> > >   devres_release_all, but this is all I need to get my use case going.
> >
> > After device_del, you should never be using that structure again anyway.
> > So why is there any "resource leak"?  You can't recycle the structure,
> > and you can't assign it to anything else, so eventually you have to do
> > a final put on the thing, which will free up the resources.
>
> I guess I should have spent more time explaining this. There's two
> references involved:
>
> - drm_device->dev points at the underlying struct device. The
> drm_device holds a reference until it's fully cleaned up, so that we
> can do nice stuff like use dev_ versions of printk functions, and you
> always know that there's not going to be a use-after free.
>
> - now the other dependency is that as long as the device exists (not
> just in memory, but in the device model, i.e. between device_add() and
> device_del()) the drm_device should exist. So what we do in the
> bus-specific remove/disconnect callback is that we call
> drm_dev_unregister(). This drops the drm_device refcount that the drm
> chardev was holding, to make sure that an open() on the chardev can
> actually get at the memory without going boom. Then after the
> drm_dev_unregister, again in the remove/disconnect callback of th
> driver, there's a drm_dev_put(). Which might or might not be the final
> drm_dev_put(), since if there's currently some open fd we keep the
> refcount elevated, to avoid oopses and fun stuff like that. And
> drm_device pointers get shared very widely, thanks to fun stuff like
> dma_buf buffer sharing and dma_fence hw syncpt sharing across
> processes and drivers.
>
> Once the final drm_dev_put() is called we also end up calling
> put_device() and everything is happy.
>
> So far so good.
>
> Now the problem is that refcount is hard, and most drm drivers get it
> wrong in some fashion or another, so I'm trying to solve all this with
> more magic.
>
> Since all drivers need to have a drm_dev_put() at the end of their
> driver's remove/disconnect callback we've added a devm_drm_dev_init
> function which registers a devres action to do that drm_dev_put() at
> device_del time (which might or might not be the final drm_dev_put()).
> Nothing has changed thus far.
>
> Now this works really well because when you have a real driver model
> driver attached, then device_del ends up calling devres_release_all(),
> which ends up triggering the 

Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Daniel Vetter
On Fri, Apr 3, 2020 at 4:17 PM Greg Kroah-Hartman
 wrote:
>
> On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> > In drm we've added nice drm_device (the main gpu driver thing, which
> > also represents the userspace interfaces and has everything else
> > dangling off it) init functions using devres, devm_drm_dev_init and
> > soon devm_drm_dev_alloc (this patch series adds that).
> >
> > A slight trouble is that drm_device itself holds a reference on the
> > struct device it's sitting on top (for sysfs links and dmesg debug and
> > lots of other things), so there's a reference loop. For real drivers
> > this is broken at remove/unplug time, where all devres resources are
> > released device_release_driver(), before the final device reference is
> > dropped. So far so good.
> >
> > There's 2 exceptions:
> > - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
> >   platform device to make them look more like normal devices to
> >   userspace. These aren't drivers in the driver model sense, we simple
> >   create a platform_device and register it.
>
> That's a horrid abuse of platform devices, just use a "virtual" device
> please, create/remove it when you need it, and all should be fine.
>
> > - drm/i915/selftests, where we create minimal mock devices, and again
> >   the selftests aren't proper drivers in the driver model sense.
>
> Again, virtual devices are best to use for this.

Hm yeah, I guess we should fix that. i915 selftests do use raw struct
device though, and it's not really the problem.

> > For these two cases the reference loop isn't broken, because devres is
> > only cleaned up when the last device reference is dropped. But that's
> > not happening, because the drm_device holds that last struct device
> > reference.
> >
> > Thus far this wasn't a problem since the above cases simply
> > hand-rolled their cleanup code. But I want to convert all drivers over
> > to the devm_ versions, hence it would be really nice if these
> > virtual/fake/mock uses-cases could also be managed with devres
> > cleanup.
> >
> > I see three possible approaches:
> >
> > - Clean up devres from device_del (or platform_device_unregister) even
> >   when no driver is bound. This seems like the simplest solution, but
> >   also the one with the widest impact, and what this patch implements.
> >   We might want to include more of the cleanup than just
> >   devres_release_all, but this is all I need to get my use case going.
>
> After device_del, you should never be using that structure again anyway.
> So why is there any "resource leak"?  You can't recycle the structure,
> and you can't assign it to anything else, so eventually you have to do
> a final put on the thing, which will free up the resources.

I guess I should have spent more time explaining this. There's two
references involved:

- drm_device->dev points at the underlying struct device. The
drm_device holds a reference until it's fully cleaned up, so that we
can do nice stuff like use dev_ versions of printk functions, and you
always know that there's not going to be a use-after free.

- now the other dependency is that as long as the device exists (not
just in memory, but in the device model, i.e. between device_add() and
device_del()) the drm_device should exist. So what we do in the
bus-specific remove/disconnect callback is that we call
drm_dev_unregister(). This drops the drm_device refcount that the drm
chardev was holding, to make sure that an open() on the chardev can
actually get at the memory without going boom. Then after the
drm_dev_unregister, again in the remove/disconnect callback of th
driver, there's a drm_dev_put(). Which might or might not be the final
drm_dev_put(), since if there's currently some open fd we keep the
refcount elevated, to avoid oopses and fun stuff like that. And
drm_device pointers get shared very widely, thanks to fun stuff like
dma_buf buffer sharing and dma_fence hw syncpt sharing across
processes and drivers.

Once the final drm_dev_put() is called we also end up calling
put_device() and everything is happy.

So far so good.

Now the problem is that refcount is hard, and most drm drivers get it
wrong in some fashion or another, so I'm trying to solve all this with
more magic.

Since all drivers need to have a drm_dev_put() at the end of their
driver's remove/disconnect callback we've added a devm_drm_dev_init
function which registers a devres action to do that drm_dev_put() at
device_del time (which might or might not be the final drm_dev_put()).
Nothing has changed thus far.

Now this works really well because when you have a real driver model
driver attached, then device_del ends up calling devres_release_all(),
which ends up triggering the multi-stage cleanup of drm_devices. But
if you do _not_ have a real driver attached, then device_del does
nothing wrt devres cleanup. Instead this is delayed until the final
put_device().

Unfortunately that final put_device() will never happen, 

Re: [PATCH 2/2] drm: delete drm_pci.h

2020-04-03 Thread Sam Ravnborg
Hi Daniel.

On Fri, Apr 03, 2020 at 01:06:10PM +0200, Daniel Vetter wrote:
> It's empty!
> 
> After more than 20 years of OS abstraction layer for pci devices, it's
> kinda gone now.
> 
> Signed-off-by: Daniel Vetter 
Looks good, and survived my build testing.

Acked-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_bufs.c   |  2 +-
>  drivers/gpu/drm/drm_dma.c|  2 +-
>  drivers/gpu/drm/drm_pci.c|  1 -
>  drivers/gpu/drm/i915/gem/i915_gem_phys.c |  2 --
>  drivers/gpu/drm/r128/ati_pcigart.c   |  3 +-
>  drivers/gpu/drm/radeon/radeon_drv.c  |  2 +-
>  include/drm/drm_pci.h| 37 
>  7 files changed, 5 insertions(+), 44 deletions(-)
>  delete mode 100644 include/drm/drm_pci.h
> 
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index dcabf5698333..ef26ac57f039 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -43,7 +44,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  
>  #include "drm_legacy.h"
> diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
> index a7add55a85b4..d07ba54ec945 100644
> --- a/drivers/gpu/drm/drm_dma.c
> +++ b/drivers/gpu/drm/drm_dma.c
> @@ -34,9 +34,9 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include 
> -#include 
>  #include 
>  
>  #include "drm_legacy.h"
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 131b7a139fda..75e2b7053f35 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -30,7 +30,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  
>  #include "drm_internal.h"
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> index 698e22420dc5..7fe9831aa9ba 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
> @@ -10,8 +10,6 @@
>  
>  #include  /* for drm_legacy.h! */
>  #include 
> -#include  /* for drm_pci.h! */
> -#include 
>  
>  #include "gt/intel_gt.h"
>  #include "i915_drv.h"
> diff --git a/drivers/gpu/drm/r128/ati_pcigart.c 
> b/drivers/gpu/drm/r128/ati_pcigart.c
> index 9b4072f97215..3e76ae5a17ee 100644
> --- a/drivers/gpu/drm/r128/ati_pcigart.c
> +++ b/drivers/gpu/drm/r128/ati_pcigart.c
> @@ -32,9 +32,10 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #include "ati_pcigart.h"
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 59f8186a2415..bbb0883e8ce6 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -44,7 +45,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
> deleted file mode 100644
> index 1bf31131960e..
> --- a/include/drm/drm_pci.h
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/*
> - * Internal Header for the Direct Rendering Manager
> - *
> - * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> - * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> - * Copyright (c) 2009-2010, Code Aurora Forum.
> - * All rights reserved.
> - *
> - * Author: Rickard E. (Rik) Faith 
> - * Author: Gareth Hughes 
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
> - */
> -
> -#ifndef _DRM_PCI_H_
> -#define _DRM_PCI_H_
> -
> -#include 
> -
> -#endif /* _DRM_PCI_H_ */
> -- 
> 2.25.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 

[PATCH v3 3/3] MAINTAINERS: Update feiyang, st7701 panel bindings converted as YAML

2020-04-03 Thread Jagan Teki
The feiyang,fy07024di26a30d.txt and sitronix,st7701.txt has been
converted to YAML schemas, update MAINTAINERS to match them again.

Signed-off-by: Jagan Teki 
---
Changes for v3:
- none

 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ba8b584bf95..b987f2588e6a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5273,7 +5273,7 @@ DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD 
PANELS
 M: Jagan Teki 
 S: Maintained
 F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
-F: 
Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
+F: 
Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml
 
 DRM DRIVER FOR GRAIN MEDIA GM12U320 PROJECTORS
 M: Hans de Goede 
@@ -5416,7 +5416,7 @@ DRM DRIVER FOR SITRONIX ST7701 PANELS
 M: Jagan Teki 
 S: Maintained
 F: drivers/gpu/drm/panel/panel-sitronix-st7701.c
-F: Documentation/devicetree/bindings/display/panel/sitronix,st7701.txt
+F: Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
 
 DRM DRIVER FOR SITRONIX ST7586 PANELS
 M: David Lechner 
-- 
2.17.1

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


[PATCH v3 2/3] dt-bindings: display: panel: Convert sitronix, st7701 to DT schema

2020-04-03 Thread Jagan Teki
Convert the sitronix,st7701 panel bindings to DT schema.

Signed-off-by: Jagan Teki 
---
Changes for v3:
- update the licence, used (GPL-2.0-only OR BSD-2-Clause) since
  I'm the author for old binding
- use panel-common.yaml
- mark true for common properties 
- use maxItems: 1 for reg
- update example
Changes for v2:
- fix dt_binding_check 

 .../display/panel/sitronix,st7701.txt | 30 
 .../display/panel/sitronix,st7701.yaml| 69 +++
 2 files changed, 69 insertions(+), 30 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/display/panel/sitronix,st7701.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml

diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7701.txt 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7701.txt
deleted file mode 100644
index ccd17597f1f6..
--- a/Documentation/devicetree/bindings/display/panel/sitronix,st7701.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Sitronix ST7701 based LCD panels
-
-ST7701 designed for small and medium sizes of TFT LCD display, is
-capable of supporting up to 480RGBX864 in resolution. It provides
-several system interfaces like MIPI/RGB/SPI.
-
-Techstar TS8550B is 480x854, 2-lane MIPI DSI LCD panel which has
-inbuilt ST7701 chip.
-
-Required properties:
-- compatible: must be "sitronix,st7701" and one of
-  * "techstar,ts8550b"
-- reset-gpios: a GPIO phandle for the reset pin
-
-Required properties for techstar,ts8550b:
-- reg: DSI virtual channel used by that screen
-- VCC-supply: analog regulator for MIPI circuit
-- IOVCC-supply: I/O system regulator
-
-Optional properties:
-- backlight: phandle for the backlight control.
-
-panel@0 {
-   compatible = "techstar,ts8550b", "sitronix,st7701";
-   reg = <0>;
-   VCC-supply = <_dldo2>;
-   IOVCC-supply = <_dldo2>;
-   reset-gpios = < 3 24 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD24 */
-   backlight = <>;
-};
diff --git 
a/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml 
b/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
new file mode 100644
index ..6dff59fe4be1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7701.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/sitronix,st7701.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sitronix ST7701 based LCD panels
+
+maintainers:
+  - Jagan Teki 
+
+description: |
+  ST7701 designed for small and medium sizes of TFT LCD display, is
+  capable of supporting up to 480RGBX864 in resolution. It provides
+  several system interfaces like MIPI/RGB/SPI.
+
+  Techstar TS8550B is 480x854, 2-lane MIPI DSI LCD panel which has
+  inbuilt ST7701 chip.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+items:
+  - enum:
+  - techstar,ts8550b
+  - const: sitronix,st7701
+
+  reg:
+description: DSI virtual channel used by that screen
+maxItems: 1
+
+  VCC-supply:
+description: analog regulator for MIPI circuit
+
+  IOVCC-supply:
+description: I/O system regulator
+
+  reset-gpios: true
+
+  backlight: true
+
+required:
+  - compatible
+  - reg
+  - VCC-supply
+  - IOVCC-supply
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "techstar,ts8550b", "sitronix,st7701";
+reg = <0>;
+VCC-supply = <_dldo2>;
+IOVCC-supply = <_dldo2>;
+reset-gpios = < 3 24 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD24 */
+backlight = <>;
+};
+};
-- 
2.17.1

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


[PATCH v3 1/3] dt-bindings: display: panel: Convert feiyang, fy07024di26a30d to DT schema

2020-04-03 Thread Jagan Teki
Convert the feiyang,fy07024di26a30d panel bindings to DT schema.

Signed-off-by: Jagan Teki 
---
Changes for v3:
- update the licence, used (GPL-2.0-only OR BSD-2-Clause) since
  I'm the author for old binding
- use panel-common.yaml
- mark true for common properties 
- use maxItems: 1 for reg
- update example
Changes for v2:
- fix dt_binding_check 

 .../display/panel/feiyang,fy07024di26a30d.txt | 20 ---
 .../panel/feiyang,fy07024di26a30d.yaml| 58 +++
 2 files changed, 58 insertions(+), 20 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml

diff --git 
a/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt 
b/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
deleted file mode 100644
index 82caa7b65ae8..
--- 
a/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-Feiyang FY07024DI26A30-D 7" MIPI-DSI LCD Panel
-
-Required properties:
-- compatible: must be "feiyang,fy07024di26a30d"
-- reg: DSI virtual channel used by that screen
-- avdd-supply: analog regulator dc1 switch
-- dvdd-supply: 3v3 digital regulator
-- reset-gpios: a GPIO phandle for the reset pin
-
-Optional properties:
-- backlight: phandle for the backlight control.
-
-panel@0 {
-   compatible = "feiyang,fy07024di26a30d";
-   reg = <0>;
-   avdd-supply = <_dc1sw>;
-   dvdd-supply = <_dldo2>;
-   reset-gpios = < 3 24 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD24 */
-   backlight = <>;
-};
diff --git 
a/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml 
b/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml
new file mode 100644
index ..95acf9e96f1c
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/feiyang,fy07024di26a30d.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Feiyang FY07024DI26A30-D 7" MIPI-DSI LCD Panel
+
+maintainers:
+  - Jagan Teki 
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+const: feiyang,fy07024di26a30d
+
+  reg:
+description: DSI virtual channel used by that screen
+maxItems: 1
+
+  avdd-supply:
+description: analog regulator dc1 switch
+
+  dvdd-supply:
+description: 3v3 digital regulator
+
+  reset-gpios: true
+
+  backlight: true
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - dvdd-supply
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "feiyang,fy07024di26a30d";
+reg = <0>;
+avdd-supply = <_dc1sw>;
+dvdd-supply = <_dldo2>;
+reset-gpios = < 3 24 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD24 */
+backlight = <>;
+};
+};
-- 
2.17.1

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


Re: KMS enums and bitfields UAPI

2020-04-03 Thread Adam Jackson
On Fri, 2020-04-03 at 15:24 +0300, Pekka Paalanen wrote:
> On Fri, 03 Apr 2020 10:15:21 + Simon Ser  wrote:
> 
> > Additionally, I've heard Pekka saying that it would be nice to have 
> > constants
> > for property names in the UAPI headers. Indeed, this would prevent
> > hard-to-debug typo issues. I have a very good example of such a typo issue 
> > that
> > took literally hours to debug, with X11 atoms which also use free-form 
> > strings
> > like KMS properties [3].
> > 
> > If we have constants for property names in UAPI headers, it wouldn't be a 
> > big
> > hurdle to also have constants for enum values alongside.
> 
> To clarify, the property names would be of the form
> 
> #define DRM_KMS_PROPERTY_KERBLAH "KerBlah"
> 
> while enum values would be integers, i.e. the raw values.
> 
> Daniel Stone did have a good counter-argument to defining property
> names: userspace would be full of
> 
> #ifndef DRM_KMS_PROPERTY_KERBLAH
> #define DRM_KMS_PROPERTY_KERBLAH "KerBleh"
> #endif
> 
> anyway as long as they cannot rely on the headers to be recent enough.
> (The typo is intentional.)

Why not put this in some external registry and regularly sync it into
drm-next? This seems like an xorgproto kind of problem to me.

- ajax

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


Re: [PATCH v2 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2020-04-03 Thread Mauro Carvalho Chehab
Em Fri, 3 Apr 2020 14:10:53 +0200
Rasmus Villemoes  escreveu:

> On 03/04/2020 11.11, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.  
> 
> This seems quite niche to me, I'm not sure that belongs in vsprintf.c.

It is used on different subsystems. At least media, drm and input (yes, 
there are some input multi-touch devices with return images using
"GREY" fourcc - see drivers/input/touchscreen/sur40.c).

> What's wrong with having a
> 
> char *fourcc_string(char *buf, u32 x)
> 
> that formats x into buf and returns buf, so it can be used in a
> 
> char buf[8];
> pr_debug("bla: %s\n", fourcc_string(buf, x))
> 
> Or, for that matter, since it's for debugging, why not just print x with
> 0x%08x?

That's about what it has been done so far, using different solutions
on different places. Some display hex values, others display fourcc
(usually ignoring the BE case). We'd like to have a common solution
that won't be subsystem-specific and will handle it on a proper unified
way.

With regards to ex values, see for example the GREY format:

V4L2_PIX_FMT_GREY ('GREY')

when someone reads 'GREY', this is easily understandable as a grey image
format, even by someone that it is not familiar with 4cc codes. Same is
true for several other widely used formats, like BGR and RGB.

If you see its hexa representation, 0x47524559 is a lot more obscure.

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


Re: [PATCH 01/44] drivers/base: Always release devres on device_del

2020-04-03 Thread Greg Kroah-Hartman
On Fri, Apr 03, 2020 at 03:57:45PM +0200, Daniel Vetter wrote:
> In drm we've added nice drm_device (the main gpu driver thing, which
> also represents the userspace interfaces and has everything else
> dangling off it) init functions using devres, devm_drm_dev_init and
> soon devm_drm_dev_alloc (this patch series adds that).
> 
> A slight trouble is that drm_device itself holds a reference on the
> struct device it's sitting on top (for sysfs links and dmesg debug and
> lots of other things), so there's a reference loop. For real drivers
> this is broken at remove/unplug time, where all devres resources are
> released device_release_driver(), before the final device reference is
> dropped. So far so good.
> 
> There's 2 exceptions:
> - drm/vkms|vgem: Virtual drivers for which we create a fake/virtual
>   platform device to make them look more like normal devices to
>   userspace. These aren't drivers in the driver model sense, we simple
>   create a platform_device and register it.

That's a horrid abuse of platform devices, just use a "virtual" device
please, create/remove it when you need it, and all should be fine.

> - drm/i915/selftests, where we create minimal mock devices, and again
>   the selftests aren't proper drivers in the driver model sense.

Again, virtual devices are best to use for this.

> For these two cases the reference loop isn't broken, because devres is
> only cleaned up when the last device reference is dropped. But that's
> not happening, because the drm_device holds that last struct device
> reference.
> 
> Thus far this wasn't a problem since the above cases simply
> hand-rolled their cleanup code. But I want to convert all drivers over
> to the devm_ versions, hence it would be really nice if these
> virtual/fake/mock uses-cases could also be managed with devres
> cleanup.
> 
> I see three possible approaches:
> 
> - Clean up devres from device_del (or platform_device_unregister) even
>   when no driver is bound. This seems like the simplest solution, but
>   also the one with the widest impact, and what this patch implements.
>   We might want to include more of the cleanup than just
>   devres_release_all, but this is all I need to get my use case going.

After device_del, you should never be using that structure again anyway.
So why is there any "resource leak"?  You can't recycle the structure,
and you can't assign it to anything else, so eventually you have to do
a final put on the thing, which will free up the resources.

And then all should be fine, right?  But, by putting the freeing here,
you can still have a "live" device that thinks it has resources availble
that it can access, but yet they are now gone.  Yeah, it's probably not
ever going to really happen, but the lifecycles of dynamic devices are
tough to "prove" at times, and I worry that freeing things this early is
going to cause odd disconnect issues.

thanks,

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


Re: mainline/master bisection: baseline.login on peach-pi

2020-04-03 Thread Guillaume Tucker
Please see the bisection report below about a boot failure.

Reports aren't automatically sent to the public while we're
trialing new bisection features on kernelci.org but this one
looks valid.

This bisection was run with exynos_defconfig but the issue can
also be reproduced with multi_v7_defconfig.  It doesn't appear to
be affecting any other platforms on kernelci.org.  This looks
like a DRM driver problem, the kernel image boots fine without
the modules installed.  It actually started failing on Tuesday in
mainline.

Guillaume

On 02/04/2020 19:38, kernelci.org bot wrote:
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * This automated bisection report was sent to you on the basis  *
> * that you may be involved with the breaking commit it has  *
> * found.  No manual investigation has been done to verify it,   *
> * and the root cause of the problem may be somewhere else.  *
> *   *
> * If you do send a fix, please include this trailer:*
> *   Reported-by: "kernelci.org bot"   *
> *   *
> * Hope this helps!  *
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> 
> mainline/master bisection: baseline.login on peach-pi
> 
> Summary:
>   Start:  56a451b780676 Merge tag 'ntb-5.7' of 
> git://github.com/jonmason/ntb
>   Plain log:  
> https://storage.kernelci.org//mainline/master/v5.6-3277-g56a451b78067/arm/exynos_defconfig/gcc-8/lab-collabora/baseline-exynos5800-peach-pi.txt
>   HTML log:   
> https://storage.kernelci.org//mainline/master/v5.6-3277-g56a451b78067/arm/exynos_defconfig/gcc-8/lab-collabora/baseline-exynos5800-peach-pi.html
>   Result: 42e67b479eab6 drm/prime: use dma length macro when mapping sg
> 
> Checks:
>   revert: PASS
>   verify: PASS
> 
> Parameters:
>   Tree:   mainline
>   URL:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>   Branch: master
>   Target: peach-pi
>   CPU arch:   arm
>   Lab:lab-collabora
>   Compiler:   gcc-8
>   Config: exynos_defconfig
>   Test case:  baseline.login
> 
> Breaking commit found:
> 
> ---
> commit 42e67b479eab6d26459b80b4867298232b0435e7
> Author: Shane Francis 
> Date:   Wed Mar 25 09:07:39 2020 +
> 
> drm/prime: use dma length macro when mapping sg
> 
> As dma_map_sg can reorganize scatter-gather lists in a
> way that can cause some later segments to be empty we should
> always use the sg_dma_len macro to fetch the actual length.
> 
> This could now be 0 and not need to be mapped to a page or
> address array
> 
> Fixes: be62dbf554c5 ("iommu/amd: Convert AMD iommu driver to the 
> dma-iommu api")
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=206461
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=206895
> Bug: https://gitlab.freedesktop.org/drm/amd/issues/1056
> Signed-off-by: Shane Francis 
> Reviewed-by: Michael J. Ruhl 
> Signed-off-by: Alex Deucher 
> Link: 
> https://patchwork.freedesktop.org/patch/msgid/20200325090741.21957-2-bigbeesh...@gmail.com
> Cc: sta...@vger.kernel.org
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 86d9b0e45c8c6..1de2cde2277ca 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -967,7 +967,7 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table 
> *sgt, struct page **pages,
>  
>   index = 0;
>   for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> - len = sg->length;
> + len = sg_dma_len(sg);
>   page = sg_page(sg);
>   addr = sg_dma_address(sg);
> ---
> 
> 
> Git bisection log:
> 
> ---
> git bisect start
> # good: [8b614cb8f1dcac8ca77cf4dd85f46ef3055f8238] Merge tag 
> '5.6-rc4-smb3-fixes' of git://git.samba.org/sfrench/cifs-2.6
> git bisect good 8b614cb8f1dcac8ca77cf4dd85f46ef3055f8238
> # bad: [56a451b780676bc1cdac011735fe2869fa2e9abf] Merge tag 'ntb-5.7' of 
> git://github.com/jonmason/ntb
> git bisect bad 56a451b780676bc1cdac011735fe2869fa2e9abf
> # bad: [59838093be51ee9447f6ad05483d697b6fa0368d] Merge tag 
> 'driver-core-5.7-rc1' of 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
> git bisect bad 59838093be51ee9447f6ad05483d697b6fa0368d
> # bad: [32db9f10d52c97ffc407c7dad81c6fafcad730b2] Merge tag 
> 'arm-soc-fixes-5.6' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
> git bisect bad 32db9f10d52c97ffc407c7dad81c6fafcad730b2
> # good: [78511edc2dd4c7b9f74f3b544093c854b7bd7744] Merge tag 'pm-5.6-rc6' of 
> 

Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

2020-04-03 Thread Michel Dänzer
On 2020-03-01 6:46 a.m., Marek Olšák wrote:
> For Mesa, we could run CI only when Marge pushes, so that it's a strictly
> pre-merge CI.

Thanks for the suggestion! I implemented something like this for Mesa:

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


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 38/44] drm/armada: Don't use drm_device->dev_private

2020-04-03 Thread Daniel Vetter
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Signed-off-by: Daniel Vetter 
Cc: Russell King 
---
 drivers/gpu/drm/armada/armada_crtc.c| 4 ++--
 drivers/gpu/drm/armada/armada_debugfs.c | 2 +-
 drivers/gpu/drm/armada/armada_drm.h | 2 ++
 drivers/gpu/drm/armada/armada_drv.c | 4 +---
 drivers/gpu/drm/armada/armada_fbdev.c   | 4 ++--
 drivers/gpu/drm/armada/armada_gem.c | 4 ++--
 drivers/gpu/drm/armada/armada_overlay.c | 8 
 7 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c 
b/drivers/gpu/drm/armada/armada_crtc.c
index c2b92acd1e9a..8686e50226a5 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -757,7 +757,7 @@ static int armada_drm_crtc_cursor_move(struct drm_crtc 
*crtc, int x, int y)
 static void armada_drm_crtc_destroy(struct drm_crtc *crtc)
 {
struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc);
-   struct armada_private *priv = crtc->dev->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(crtc->dev);
 
if (dcrtc->cursor_obj)
drm_gem_object_put_unlocked(>cursor_obj->obj);
@@ -901,7 +901,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, 
struct device *dev,
struct resource *res, int irq, const struct armada_variant *variant,
struct device_node *port)
 {
-   struct armada_private *priv = drm->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(drm);
struct armada_crtc *dcrtc;
struct drm_plane *primary;
void __iomem *base;
diff --git a/drivers/gpu/drm/armada/armada_debugfs.c 
b/drivers/gpu/drm/armada/armada_debugfs.c
index c6fc2f1d58e9..29f4b52e3c8d 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -19,7 +19,7 @@ static int armada_debugfs_gem_linear_show(struct seq_file *m, 
void *data)
 {
struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
-   struct armada_private *priv = dev->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(dev);
struct drm_printer p = drm_seq_file_printer(m);
 
mutex_lock(>linear_lock);
diff --git a/drivers/gpu/drm/armada/armada_drm.h 
b/drivers/gpu/drm/armada/armada_drm.h
index a11bdaccbb33..6a5a87932576 100644
--- a/drivers/gpu/drm/armada/armada_drm.h
+++ b/drivers/gpu/drm/armada/armada_drm.h
@@ -73,6 +73,8 @@ struct armada_private {
 #endif
 };
 
+#define drm_to_armada_dev(dev) container_of(dev, struct armada_private, drm)
+
 int armada_fbdev_init(struct drm_device *);
 void armada_fbdev_fini(struct drm_device *);
 
diff --git a/drivers/gpu/drm/armada/armada_drv.c 
b/drivers/gpu/drm/armada/armada_drv.c
index 2546ff9d1c92..2a9ee76ee585 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -106,8 +106,6 @@ static int armada_drm_bind(struct device *dev)
return ret;
}
 
-   priv->drm.dev_private = priv;
-
dev_set_drvdata(dev, >drm);
 
/* Mode setting support */
@@ -169,7 +167,7 @@ static int armada_drm_bind(struct device *dev)
 static void armada_drm_unbind(struct device *dev)
 {
struct drm_device *drm = dev_get_drvdata(dev);
-   struct armada_private *priv = drm->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(drm);
 
drm_kms_helper_poll_fini(>drm);
armada_fbdev_fini(>drm);
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index f2dc371bd8e5..c9a414b3a8c4 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -117,7 +117,7 @@ static const struct drm_fb_helper_funcs 
armada_fb_helper_funcs = {
 
 int armada_fbdev_init(struct drm_device *dev)
 {
-   struct armada_private *priv = dev->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(dev);
struct drm_fb_helper *fbh;
int ret;
 
@@ -151,7 +151,7 @@ int armada_fbdev_init(struct drm_device *dev)
 
 void armada_fbdev_fini(struct drm_device *dev)
 {
-   struct armada_private *priv = dev->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(dev);
struct drm_fb_helper *fbh = priv->fbdev;
 
if (fbh) {
diff --git a/drivers/gpu/drm/armada/armada_gem.c 
b/drivers/gpu/drm/armada/armada_gem.c
index 976685f2939e..2c7d5f71e715 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -39,7 +39,7 @@ static size_t roundup_gem_size(size_t size)
 void armada_gem_free_object(struct drm_gem_object *obj)
 {
struct armada_gem_object *dobj = drm_to_armada_gem(obj);
-   struct armada_private *priv = obj->dev->dev_private;
+   struct armada_private *priv = drm_to_armada_dev(obj->dev);
 
DRM_DEBUG_DRIVER("release obj %p\n", dobj);
 
@@ -77,7 +77,7 @@ void 

[PATCH 44/44] drm/managed: Cleanup of unused functions and polishing docs

2020-04-03 Thread Daniel Vetter
Following functions are only used internally, not by drivers:
- drm_dev_init
- devm_drm_dev_init
- drmm_add_final_kfree

Also, now that we have a very slick and polished way to allocate a
drm_device with devm_drm_dev_alloc, update all the docs to reflect the
new reality. Mostly this consists of deleting old and misleading
hints. Two main ones:

- it is no longer required that the drm_device base class is first in
  the structure. devm_drm_dev_alloc can cope with it being anywhere

- obviously embedded no needs devm_drm_dev_alloc

Signed-off-by: Daniel Vetter 
---
 .../driver-api/driver-model/devres.rst|   2 +-
 drivers/gpu/drm/drm_drv.c | 119 --
 drivers/gpu/drm/drm_internal.h|   1 +
 drivers/gpu/drm/drm_managed.c |  15 +--
 include/drm/drm_device.h  |   2 +-
 include/drm/drm_drv.h |  20 +--
 6 files changed, 34 insertions(+), 125 deletions(-)

diff --git a/Documentation/driver-api/driver-model/devres.rst 
b/Documentation/driver-api/driver-model/devres.rst
index 46c13780994c..74a4a3fa8c52 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -263,7 +263,7 @@ DMA
   dmam_pool_destroy()
 
 DRM
-  devm_drm_dev_init()
+  devm_drm_dev_alloc()
 
 GPIO
   devm_gpiod_get()
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9e60b784b3ac..64e20c630aa7 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -240,13 +240,13 @@ void drm_minor_release(struct drm_minor *minor)
  * DOC: driver instance overview
  *
  * A device instance for a drm driver is represented by  drm_device. 
This
- * is initialized with drm_dev_init(), usually from bus-specific ->probe()
- * callbacks implemented by the driver. The driver then needs to initialize all
- * the various subsystems for the drm device like memory management, vblank
- * handling, modesetting support and intial output configuration plus obviously
- * initialize all the corresponding hardware bits. Finally when everything is 
up
- * and running and ready for userspace the device instance can be published
- * using drm_dev_register().
+ * is allocated and initialized with devm_drm_dev_alloc(), usually from
+ * bus-specific ->probe() callbacks implemented by the driver. The driver then
+ * needs to initialize all the various subsystems for the drm device like 
memory
+ * management, vblank handling, modesetting support and intial output
+ * configuration plus obviously initialize all the corresponding hardware bits.
+ * Finally when everything is up and running and ready for userspace the device
+ * instance can be published using drm_dev_register().
  *
  * There is also deprecated support for initalizing device instances using
  * bus-specific helpers and the _driver.load callback. But due to
@@ -274,7 +274,7 @@ void drm_minor_release(struct drm_minor *minor)
  *
  * The following example shows a typical structure of a DRM display driver.
  * The example focus on the probe() function and the other functions that is
- * almost always present and serves as a demonstration of devm_drm_dev_init().
+ * almost always present and serves as a demonstration of devm_drm_dev_alloc().
  *
  * .. code-block:: c
  *
@@ -294,22 +294,12 @@ void drm_minor_release(struct drm_minor *minor)
  * struct drm_device *drm;
  * int ret;
  *
- * // devm_kzalloc() can't be used here because the drm_device '
- * // lifetime can exceed the device lifetime if driver unbind
- * // happens when userspace still has open file descriptors.
- * priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- * if (!priv)
- * return -ENOMEM;
- *
+ * priv = devm_drm_dev_alloc(>dev, _drm_driver,
+ *   struct driver_device, drm);
+ * if (IS_ERR(priv))
+ * return PTR_ERR(priv);
  * drm = >drm;
  *
- * ret = devm_drm_dev_init(>dev, drm, _drm_driver);
- * if (ret) {
- * kfree(priv);
- * return ret;
- * }
- * drmm_add_final_kfree(drm, priv);
- *
  * ret = drmm_mode_config_init(drm);
  * if (ret)
  * return ret;
@@ -550,9 +540,9 @@ static void drm_fs_inode_free(struct inode *inode)
  * following guidelines apply:
  *
  *  - The entire device initialization procedure should be run from the
- *_master_ops.master_bind callback, starting with drm_dev_init(),
- *then binding all components with component_bind_all() and finishing with
- *drm_dev_register().
+ *_master_ops.master_bind callback, starting with
+ *devm_drm_dev_alloc(), then binding all components with
+ *component_bind_all() and finishing with drm_dev_register().
  *
  *  - The opaque pointer passed to all 

[PATCH 37/44] drm/armada: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Also remove the now no longer needed build bug on since that's already
not needed anymore with drmm_add_final_kfree. Conversion to managed
drm_device cleanup is easy, the final drm_dev_put() is already the
last thing in both the bind unbind as in the unbind flow.

Also, this relies on component.c correctly wrapping bind in
separate devres groups, which it does.

Signed-off-by: Daniel Vetter 
Cc: Russell King 
---
 drivers/gpu/drm/armada/armada_drv.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c 
b/drivers/gpu/drm/armada/armada_drv.c
index dd9ed71ed942..2546ff9d1c92 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -87,24 +87,13 @@ static int armada_drm_bind(struct device *dev)
 "armada-drm"))
return -EBUSY;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-   if (!priv)
-   return -ENOMEM;
-
-   /*
-* The drm_device structure must be at the start of
-* armada_private for drm_dev_put() to work correctly.
-*/
-   BUILD_BUG_ON(offsetof(struct armada_private, drm) != 0);
-
-   ret = drm_dev_init(>drm, _drm_driver, dev);
-   if (ret) {
-   dev_err(dev, "[" DRM_NAME ":%s] drm_dev_init failed: %d\n",
-   __func__, ret);
-   kfree(priv);
-   return ret;
+   priv = devm_drm_dev_alloc(dev, _drm_driver,
+ struct armada_private, drm);
+   if (IS_ERR(priv)) {
+   dev_err(dev, "[" DRM_NAME ":%s] devm_drm_dev_alloc failed: 
%li\n",
+   __func__, PTR_ERR(priv));
+   return PTR_ERR(priv);
}
-   drmm_add_final_kfree(>drm, priv);
 
/* Remove early framebuffers */
ret = drm_fb_helper_remove_conflicting_framebuffers(NULL,
@@ -174,7 +163,6 @@ static int armada_drm_bind(struct device *dev)
  err_kms:
drm_mode_config_cleanup(>drm);
drm_mm_takedown(>linear);
-   drm_dev_put(>drm);
return ret;
 }
 
@@ -194,8 +182,6 @@ static void armada_drm_unbind(struct device *dev)
 
drm_mode_config_cleanup(>drm);
drm_mm_takedown(>linear);
-
-   drm_dev_put(>drm);
 }
 
 static int compare_of(struct device *dev, void *data)
-- 
2.25.1

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


[PATCH 41/44] drm/i915: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Luckily we're already well set up in the main driver, with
drm_dev_put() being the last thing in both the unload error case and
the pci remove function.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.c | 17 -
 drivers/gpu/drm/i915/i915_pci.c |  2 --
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7a3b4b98572..9c0ff25c5d41 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -879,19 +879,11 @@ i915_driver_create(struct pci_dev *pdev, const struct 
pci_device_id *ent)
(struct intel_device_info *)ent->driver_data;
struct intel_device_info *device_info;
struct drm_i915_private *i915;
-   int err;
 
-   i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-   if (!i915)
-   return ERR_PTR(-ENOMEM);
-
-   err = drm_dev_init(>drm, , >dev);
-   if (err) {
-   kfree(i915);
-   return ERR_PTR(err);
-   }
-
-   drmm_add_final_kfree(>drm, i915);
+   i915 = devm_drm_dev_alloc(>dev, ,
+ struct drm_i915_private, drm);
+   if (IS_ERR(i915))
+   return i915;
 
i915->drm.pdev = pdev;
pci_set_drvdata(pdev, i915);
@@ -1008,7 +1000,6 @@ int i915_driver_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
pci_disable_device(pdev);
 out_fini:
i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
-   drm_dev_put(>drm);
return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 2c80a0194c80..0f8b439d6fd5 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -920,8 +920,6 @@ static void i915_pci_remove(struct pci_dev *pdev)
 
i915_driver_remove(i915);
pci_set_drvdata(pdev, NULL);
-
-   drm_dev_put(>drm);
 }
 
 /* is device_id present in comma separated list of ids */
-- 
2.25.1

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


[PATCH 43/44] drm/i915/selftests: align more to real device lifetimes

2020-04-03 Thread Daniel Vetter
The big change is device_add so that device_del can auto-cleanup
devres resources. This allows us to use devm_drm_dev_alloc, which
removes the last user of drm_dev_init.

Signed-off-by: Daniel Vetter 
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 31 +--
 .../gpu/drm/i915/selftests/mock_gem_device.h  |  2 +-
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c 
b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 03607647cdeb..ea73d1f7cf12 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -123,12 +123,6 @@ struct drm_i915_private *mock_gem_device(void)
pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
if (!pdev)
return NULL;
-   i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-   if (!i915) {
-   kfree(pdev);
-   return NULL;
-   }
-
device_initialize(>dev);
pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
pdev->dev.release = release_dev;
@@ -139,8 +133,23 @@ struct drm_i915_private *mock_gem_device(void)
/* hack to disable iommu for the fake device; force identity mapping */
pdev->dev.archdata.iommu = (void *)-1;
 #endif
+   err = device_add(>dev);
+   if (err) {
+   kfree(pdev);
+   return NULL;
+   }
+
+   i915 = devm_drm_dev_alloc(>dev, _driver,
+ struct drm_i915_private, drm);
+   if (err) {
+   pr_err("Failed to allocate mock GEM device: err=%d\n", err);
+   put_device(>dev);
+
+   return NULL;
+   }
 
pci_set_drvdata(pdev, i915);
+   i915->drm.pdev = pdev;
 
dev_pm_domain_set(>dev, _domain);
pm_runtime_enable(>dev);
@@ -148,16 +157,6 @@ struct drm_i915_private *mock_gem_device(void)
if (pm_runtime_enabled(>dev))
WARN_ON(pm_runtime_get_sync(>dev));
 
-   err = drm_dev_init(>drm, _driver, >dev);
-   if (err) {
-   pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-   put_device(>dev);
-   kfree(i915);
-
-   return NULL;
-   }
-   i915->drm.pdev = pdev;
-   drmm_add_final_kfree(>drm, i915);
 
intel_runtime_pm_init_early(>runtime_pm);
 
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.h 
b/drivers/gpu/drm/i915/selftests/mock_gem_device.h
index 2e3c7585a7bb..4f309a05c85a 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.h
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.h
@@ -9,7 +9,7 @@ void mock_device_flush(struct drm_i915_private *i915);
 
 static inline void mock_destroy_device(struct drm_i915_private *i915)
 {
-   drm_dev_put(>drm);
+   device_del(i915->drm.dev);
 }
 
 #endif /* !__MOCK_GEM_DEVICE_H__ */
-- 
2.25.1

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


[PATCH 39/44] drm/cirrus: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Gerd Hoffmann 
Cc: Daniel Vetter 
Cc: Sam Ravnborg 
Cc: "Noralf Trønnes" 
Cc: Rob Herring 
Cc: Thomas Zimmermann 
Cc: virtualizat...@lists.linux-foundation.org
Cc: Emil Velikov 
---
 drivers/gpu/drm/cirrus/cirrus.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index a36269717c3b..4b65637147ba 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -567,18 +567,13 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
return ret;
 
ret = -ENOMEM;
-   cirrus = kzalloc(sizeof(*cirrus), GFP_KERNEL);
-   if (cirrus == NULL)
-   return ret;
+   cirrus = devm_drm_dev_alloc(>dev, _driver,
+   struct cirrus_device, dev);
+   if (IS_ERR(cirrus))
+   return PTR_ERR(cirrus);
 
dev = >dev;
-   ret = devm_drm_dev_init(>dev, dev, _driver);
-   if (ret) {
-   kfree(cirrus);
-   return ret;
-   }
dev->dev_private = cirrus;
-   drmm_add_final_kfree(dev, cirrus);
 
cirrus->vram = devm_ioremap(>dev, pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
-- 
2.25.1

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


[PATCH 35/44] drm/ingenic: Don't set drm_device->dev_private

2020-04-03 Thread Daniel Vetter
Entirely not used, just copypasta.

Signed-off-by: Daniel Vetter 
Cc: Paul Cercueil 
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
b/drivers/gpu/drm/ingenic/ingenic-drm.c
index bb62d8e93985..3386270cb8a3 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -622,7 +622,6 @@ static int ingenic_drm_probe(struct platform_device *pdev)
priv->soc_info = soc_info;
priv->dev = dev;
drm = >drm;
-   drm->dev_private = priv;
 
platform_set_drvdata(pdev, priv);
 
-- 
2.25.1

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


[PATCH 36/44] drm/komeda: use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Komeda uses the component framework, which does open/close a new
devres group around all the bind callbacks. Which means we can use
devm_ functions for managing the drm_device cleanup, with leaking
stuff in case of deferred probes or other reasons to unbind
components, or the component_master.

Also note that this fixes a double-free in the probe unroll code, bot
drm_dev_put and kfree(kms) result in the kms allocation getting freed.

Aside: komeda_bind could be cleaned up a lot, devm_kfree is a bit
redundant. Plus I'm not clear on why there's suballocations for
mdrv->mdev and mdrv->kms. Plus I'm not sure the lifetimes are correct
with all that devm_kzalloc usage ... That structure layout is also the
reason why komeda still uses drm_device->dev_private and can't easily
be replaced with a proper container_of upcasting. I'm pretty sure that
there's endless amounts of hotunplug/hotremove bugs in there with all
the unprotected dereferencing of drm_device->dev_private.

Signed-off-by: Daniel Vetter 
Cc: "James (Qian) Wang" 
Cc: Liviu Dudau 
Cc: Mihail Atanassov 
---
 drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 16dfd5cdb66c..6b85d5f4caa8 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -261,18 +261,16 @@ static void komeda_kms_mode_config_init(struct 
komeda_kms_dev *kms,
 
 struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev)
 {
-   struct komeda_kms_dev *kms = kzalloc(sizeof(*kms), GFP_KERNEL);
+   struct komeda_kms_dev *kms;
struct drm_device *drm;
int err;
 
-   if (!kms)
-   return ERR_PTR(-ENOMEM);
+   kms = devm_drm_dev_alloc(mdev->dev, _kms_driver,
+struct komeda_kms_dev, base);
+   if (IS_ERR(kms))
+   return kms;
 
drm = >base;
-   err = drm_dev_init(drm, _kms_driver, mdev->dev);
-   if (err)
-   goto free_kms;
-   drmm_add_final_kfree(drm, kms);
 
drm->dev_private = mdev;
 
@@ -329,9 +327,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev 
*mdev)
drm_mode_config_cleanup(drm);
komeda_kms_cleanup_private_objs(kms);
drm->dev_private = NULL;
-   drm_dev_put(drm);
-free_kms:
-   kfree(kms);
return ERR_PTR(err);
 }
 
@@ -348,5 +343,4 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)
drm_mode_config_cleanup(drm);
komeda_kms_cleanup_private_objs(kms);
drm->dev_private = NULL;
-   drm_dev_put(drm);
 }
-- 
2.25.1

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


[PATCH 42/44] drm/i915/selftest: Create mock_destroy_device

2020-04-03 Thread Daniel Vetter
Just some prep work before we rework the lifetime handling, which
requires replacing all the drm_dev_put in selftests by something else.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/gem/selftests/huge_pages.c   | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c| 2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_evict.c   | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c | 2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +-
 drivers/gpu/drm/i915/selftests/intel_memory_region.c  | 2 +-
 drivers/gpu/drm/i915/selftests/mock_gem_device.c  | 2 +-
 drivers/gpu/drm/i915/selftests/mock_gem_device.h  | 5 +
 13 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 2d0fd50c5312..d19bb011fc6b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1789,7 +1789,7 @@ int i915_gem_huge_page_mock_selftests(void)
i915_vm_put(>vm);
 
 out_unlock:
-   drm_dev_put(_priv->drm);
+   mock_destroy_device(dev_priv);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index f4f933240b39..d9d96d23e37e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1986,7 +1986,7 @@ int i915_gem_context_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 2a52b92586b9..0845ce1ae37c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -272,7 +272,7 @@ int i915_gem_dmabuf_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
index 2b6db6f799de..085644edebfc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
@@ -85,7 +85,7 @@ int i915_gem_object_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
index 34932871b3a5..2a9709eb5a42 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
@@ -73,6 +73,6 @@ int i915_gem_phys_mock_selftests(void)
 
err = i915_subtests(tests, i915);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c 
b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index c2578a0f2f14..1c0865227714 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -157,7 +157,7 @@ static int mock_hwsp_freelist(void *arg)
__mock_hwsp_record(, na, NULL);
kfree(state.history);
 err_put:
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 028baae9631f..f88473d396f4 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -536,7 +536,7 @@ int i915_gem_evict_mock_selftests(void)
with_intel_runtime_pm(>runtime_pm, wakeref)
err = i915_subtests(tests, >gt);
 
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 5d2a02fcf595..035e4f77f06f 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1714,7 +1714,7 @@ int i915_gem_gtt_mock_selftests(void)
mock_fini_ggtt(ggtt);
kfree(ggtt);
 out_put:
-   drm_dev_put(>drm);
+   mock_destroy_device(i915);
return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c 
b/drivers/gpu/drm/i915/selftests/i915_request.c
index 1dab0360f76a..6bc6a2fc83ab 100644
--- 

[PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private

2020-04-03 Thread Daniel Vetter
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Gerd Hoffmann 
Cc: Daniel Vetter 
Cc: "Noralf Trønnes" 
Cc: Sam Ravnborg 
Cc: Eric Anholt 
Cc: Thomas Zimmermann 
Cc: virtualizat...@lists.linux-foundation.org
---
 drivers/gpu/drm/cirrus/cirrus.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 4b65637147ba..744a8e337e41 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -59,6 +59,8 @@ struct cirrus_device {
void __iomem   *mmio;
 };
 
+#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev)
+
 /* -- */
 /*
  * The meat of this driver. The core passes us a mode and we have to program
@@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
   struct drm_rect *rect)
 {
-   struct cirrus_device *cirrus = fb->dev->dev_private;
+   struct cirrus_device *cirrus = to_cirrus(fb->dev);
void *vmap;
int idx, ret;
 
@@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct 
drm_simple_display_pipe *pipe,
   struct drm_crtc_state *crtc_state,
   struct drm_plane_state *plane_state)
 {
-   struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+   struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
 
cirrus_mode_set(cirrus, _state->mode, plane_state->fb);
cirrus_fb_blit_fullscreen(plane_state->fb);
@@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct 
drm_simple_display_pipe *pipe,
 static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
   struct drm_plane_state *old_state)
 {
-   struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+   struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
struct drm_plane_state *state = pipe->plane.state;
struct drm_crtc *crtc = >crtc;
struct drm_rect rect;
@@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
return PTR_ERR(cirrus);
 
dev = >dev;
-   dev->dev_private = cirrus;
 
cirrus->vram = devm_ioremap(>dev, pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
-- 
2.25.1

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


[PATCH 18/44] drm/st7586: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---
 drivers/gpu/drm/tiny/st7586.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index c3295c717ba6..2a1fae422f7a 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -317,18 +317,13 @@ static int st7586_probe(struct spi_device *spi)
size_t bufsize;
int ret;
 
-   dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL);
-   if (!dbidev)
-   return -ENOMEM;
+   dbidev = devm_drm_dev_alloc(dev, _driver,
+   struct mipi_dbi_dev, drm);
+   if (IS_ERR(dbidev))
+   return PTR_ERR(dbidev);
 
dbi = >dbi;
drm = >drm;
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(dbidev);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, dbidev);
 
bufsize = (st7586_mode.vdisplay + 2) / 3 * st7586_mode.hdisplay;
 
-- 
2.25.1

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


[PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Also need to remove the drm_dev_put from the remove hook.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Gerd Hoffmann 
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-de...@lists.freedesktop.org
---
 drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
 drivers/gpu/drm/qxl/qxl_drv.h |  3 +--
 drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 09102e2efabc..6b4ae4c5fb76 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
return -EINVAL; /* TODO: ENODEV ? */
}
 
-   qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
-   if (!qdev)
+   qdev = devm_drm_dev_alloc(>dev, _driver,
+ struct qxl_device, ddev);
+   if (IS_ERR(qdev)) {
+   pr_err("Unable to init drm dev");
return -ENOMEM;
+   }
 
ret = pci_enable_device(pdev);
if (ret)
-   goto free_dev;
+   return ret;
 
ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
if (ret)
@@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
}
}
 
-   ret = qxl_device_init(qdev, _driver, pdev);
+   ret = qxl_device_init(qdev, pdev);
if (ret)
goto put_vga;
 
@@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
vga_put(pdev, VGA_RSRC_LEGACY_IO);
 disable_pci:
pci_disable_device(pdev);
-free_dev:
-   kfree(qdev);
+
return ret;
 }
 
@@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
-   drm_dev_put(dev);
 }
 
 DEFINE_DRM_GEM_FOPS(qxl_fops);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 435126facc9b..86ac191d9205 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -276,8 +276,7 @@ struct qxl_device {
 extern const struct drm_ioctl_desc qxl_ioctls[];
 extern int qxl_max_ioctl;
 
-int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
-   struct pci_dev *pdev);
+int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
 void qxl_device_fini(struct qxl_device *qdev);
 
 int qxl_modeset_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 9eed1a375f24..91a34dd835d7 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
 }
 
 int qxl_device_init(struct qxl_device *qdev,
-   struct drm_driver *drv,
struct pci_dev *pdev)
 {
int r, sb;
 
-   r = drm_dev_init(>ddev, drv, >dev);
-   if (r) {
-   pr_err("Unable to init drm dev");
-   goto error;
-   }
-
qdev->ddev.pdev = pdev;
pci_set_drvdata(pdev, >ddev);
qdev->ddev.dev_private = qdev;
-   drmm_add_final_kfree(>ddev, qdev);
 
mutex_init(>gem.mutex);
mutex_init(>update_area_mutex);
@@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base, 
pci_resource_len(pdev, 0));
if (!qdev->vram_mapping) {
pr_err("Unable to create vram_mapping");
-   r = -ENOMEM;
-   goto error;
+   return -ENOMEM;
}
 
if (pci_resource_len(pdev, 4) > 0) {
@@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
io_mapping_free(qdev->surface_mapping);
 vram_mapping_free:
io_mapping_free(qdev->vram_mapping);
-error:
return r;
 }
 
-- 
2.25.1

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


[PATCH 34/44] drm/ingenic: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Paul Cercueil 
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
b/drivers/gpu/drm/ingenic/ingenic-drm.c
index a9bc6623b488..bb62d8e93985 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -614,9 +614,10 @@ static int ingenic_drm_probe(struct platform_device *pdev)
return -EINVAL;
}
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-   if (!priv)
-   return -ENOMEM;
+   priv = devm_drm_dev_alloc(dev, _drm_driver_data,
+ struct ingenic_drm, drm);
+   if (IS_ERR(priv))
+   return PTR_ERR(priv);
 
priv->soc_info = soc_info;
priv->dev = dev;
@@ -625,13 +626,6 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, priv);
 
-   ret = devm_drm_dev_init(dev, drm, _drm_driver_data);
-   if (ret) {
-   kfree(priv);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, priv);
-
ret = drmm_mode_config_init(drm);
if (ret)
return ret;
-- 
2.25.1

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


[PATCH 23/44] drm/ili9225: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---
 drivers/gpu/drm/tiny/ili9225.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 118477af4491..d1a5ab6747d5 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -376,18 +376,13 @@ static int ili9225_probe(struct spi_device *spi)
u32 rotation = 0;
int ret;
 
-   dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL);
-   if (!dbidev)
-   return -ENOMEM;
+   dbidev = devm_drm_dev_alloc(dev, _driver,
+   struct mipi_dbi_dev, drm);
+   if (IS_ERR(dbidev))
+   return PTR_ERR(dbidev);
 
dbi = >dbi;
drm = >drm;
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(dbidev);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, dbidev);
 
dbi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(dbi->reset)) {
-- 
2.25.1

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


[PATCH 12/44] drm/v3d: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Also allows us to simplify the unroll code since the drm_dev_put
disappears.

Signed-off-by: Daniel Vetter 
Cc: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_drv.c | 31 ++-
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index ead62a15d48f..f57d408ef371 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -251,29 +251,23 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
u32 ident1;
 
 
-   v3d = kzalloc(sizeof(*v3d), GFP_KERNEL);
-   if (!v3d)
-   return -ENOMEM;
+   v3d = devm_drm_dev_alloc(dev, _drm_driver, struct v3d_dev, drm);
+   if (IS_ERR(v3d))
+   return PTR_ERR(v3d);
+
v3d->dev = dev;
v3d->pdev = pdev;
drm = >drm;
 
-   ret = drm_dev_init(>drm, _drm_driver, dev);
-   if (ret) {
-   kfree(v3d);
-   return ret;
-   }
-
platform_set_drvdata(pdev, drm);
-   drmm_add_final_kfree(drm, v3d);
 
ret = map_regs(v3d, >hub_regs, "hub");
if (ret)
-   goto dev_destroy;
+   return ret;
 
ret = map_regs(v3d, >core_regs[0], "core0");
if (ret)
-   goto dev_destroy;
+   return ret;
 
mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
dev->coherent_dma_mask =
@@ -291,29 +285,28 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
ret = PTR_ERR(v3d->reset);
 
if (ret == -EPROBE_DEFER)
-   goto dev_destroy;
+   return ret;
 
v3d->reset = NULL;
ret = map_regs(v3d, >bridge_regs, "bridge");
if (ret) {
dev_err(dev,
"Failed to get reset control or bridge regs\n");
-   goto dev_destroy;
+   return ret;
}
}
 
if (v3d->ver < 41) {
ret = map_regs(v3d, >gca_regs, "gca");
if (ret)
-   goto dev_destroy;
+   return ret;
}
 
v3d->mmu_scratch = dma_alloc_wc(dev, 4096, >mmu_scratch_paddr,
GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
if (!v3d->mmu_scratch) {
dev_err(dev, "Failed to allocate MMU scratch page\n");
-   ret = -ENOMEM;
-   goto dev_destroy;
+   return -ENOMEM;
}
 
pm_runtime_use_autosuspend(dev);
@@ -340,8 +333,6 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
v3d_gem_destroy(drm);
 dma_free:
dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
-dev_destroy:
-   drm_dev_put(drm);
return ret;
 }
 
@@ -354,8 +345,6 @@ static int v3d_platform_drm_remove(struct platform_device 
*pdev)
 
v3d_gem_destroy(drm);
 
-   drm_dev_put(drm);
-
dma_free_wc(v3d->dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
 
return 0;
-- 
2.25.1

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


[PATCH 24/44] drm/hx8357d: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Eric Anholt 
---
 drivers/gpu/drm/tiny/hx8357d.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index af7f3d10aac3..b4bc358a3269 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -226,17 +226,12 @@ static int hx8357d_probe(struct spi_device *spi)
u32 rotation = 0;
int ret;
 
-   dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL);
-   if (!dbidev)
-   return -ENOMEM;
+   dbidev = devm_drm_dev_alloc(dev, _driver,
+   struct mipi_dbi_dev, drm);
+   if (IS_ERR(dbidev))
+   return PTR_ERR(dbidev);
 
drm = >drm;
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(dbidev);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, dbidev);
 
dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
if (IS_ERR(dc)) {
-- 
2.25.1

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


[PATCH 17/44] drm/st7735r: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Aside: There was an oddity in the old code, we allocated priv but in
the error path we've freed priv->dbidev ...

Signed-off-by: Daniel Vetter 
Cc: David Lechner 
---
 drivers/gpu/drm/tiny/st7735r.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
index 631801c36f46..ccbf49a53202 100644
--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -195,21 +195,16 @@ static int st7735r_probe(struct spi_device *spi)
if (!cfg)
cfg = (void *)spi_get_device_id(spi)->driver_data;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-   if (!priv)
-   return -ENOMEM;
+   priv = devm_drm_dev_alloc(dev, _driver,
+ struct st7735r_priv, dbidev.drm);
+   if (IS_ERR(priv))
+   return PTR_ERR(priv);
 
dbidev = >dbidev;
priv->cfg = cfg;
 
dbi = >dbi;
drm = >drm;
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(dbidev);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, dbidev);
 
dbi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(dbi->reset)) {
-- 
2.25.1

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


[PATCH 19/44] drm/repaper: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: "Noralf Trønnes" 
---
 drivers/gpu/drm/tiny/repaper.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 862c3ee6055d..1c0e7169545b 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -1002,19 +1002,13 @@ static int repaper_probe(struct spi_device *spi)
}
}
 
-   epd = kzalloc(sizeof(*epd), GFP_KERNEL);
-   if (!epd)
-   return -ENOMEM;
+   epd = devm_drm_dev_alloc(dev, _driver,
+struct repaper_epd, drm);
+   if (IS_ERR(epd))
+   return PTR_ERR(epd);
 
drm = >drm;
 
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(epd);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, epd);
-
ret = drmm_mode_config_init(drm);
if (ret)
return ret;
-- 
2.25.1

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


[PATCH 15/44] drm/udl: Use demv_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Also init the fbdev emulation before we register the device, that way
we can rely on the auto-cleanup and simplify the probe error code even
more.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Sean Paul 
Cc: Thomas Zimmermann 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Cc: Sam Ravnborg 
Cc: Thomas Gleixner 
---
 drivers/gpu/drm/udl/udl_drv.c | 36 +++
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1ce2d865c36d..4ba5149fdd57 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -57,27 +57,20 @@ static struct udl_device *udl_driver_create(struct 
usb_interface *interface)
struct udl_device *udl;
int r;
 
-   udl = kzalloc(sizeof(*udl), GFP_KERNEL);
-   if (!udl)
-   return ERR_PTR(-ENOMEM);
-
-   r = drm_dev_init(>drm, , >dev);
-   if (r) {
-   kfree(udl);
-   return ERR_PTR(r);
-   }
+   udl = devm_drm_dev_alloc(>dev, ,
+struct udl_device, drm);
+   if (IS_ERR(udl))
+   return udl;
 
udl->udev = udev;
udl->drm.dev_private = udl;
-   drmm_add_final_kfree(>drm, udl);
 
r = udl_init(udl);
-   if (r) {
-   drm_dev_put(>drm);
+   if (r)
return ERR_PTR(r);
-   }
 
usb_set_intfdata(interface, udl);
+
return udl;
 }
 
@@ -91,23 +84,17 @@ static int udl_usb_probe(struct usb_interface *interface,
if (IS_ERR(udl))
return PTR_ERR(udl);
 
+   r = drm_fbdev_generic_setup(>drm, 0);
+   if (r)
+   return r;
+
r = drm_dev_register(>drm, 0);
if (r)
-   goto err_free;
+   return r;
 
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
 
-   r = drm_fbdev_generic_setup(>drm, 0);
-   if (r)
-   goto err_drm_dev_unregister;
-
return 0;
-
-err_drm_dev_unregister:
-   drm_dev_unregister(>drm);
-err_free:
-   drm_dev_put(>drm);
-   return r;
 }
 
 static void udl_usb_disconnect(struct usb_interface *interface)
@@ -117,7 +104,6 @@ static void udl_usb_disconnect(struct usb_interface 
*interface)
drm_kms_helper_poll_fini(dev);
udl_drop_usb(dev);
drm_dev_unplug(dev);
-   drm_dev_put(dev);
 }
 
 /*
-- 
2.25.1

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


[PATCH 32/44] drm/mcde: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Linus Walleij 
---
 drivers/gpu/drm/mcde/mcde_drv.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index 88cc6b4a7a64..bdb525e3c5d7 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -307,24 +307,16 @@ static int mcde_probe(struct platform_device *pdev)
int ret;
int i;
 
-   mcde = kzalloc(sizeof(*mcde), GFP_KERNEL);
-   if (!mcde)
-   return -ENOMEM;
-   mcde->dev = dev;
-
-   ret = devm_drm_dev_init(dev, >drm, _drm_driver);
-   if (ret) {
-   kfree(mcde);
-   return ret;
-   }
+   mcde = devm_drm_dev_alloc(dev, _drm_driver, struct mcde, drm);
+   if (IS_ERR(mcde))
+   return PTR_ERR(mcde);
drm = >drm;
drm->dev_private = mcde;
-   drmm_add_final_kfree(drm, mcde);
+   mcde->dev = dev;
platform_set_drvdata(pdev, drm);
 
/* Enable continuous updates: this is what Linux' framebuffer expects */
mcde->oneshot_mode = false;
-   drm->dev_private = mcde;
 
/* First obtain and turn on the main power */
mcde->epod = devm_regulator_get(dev, "epod");
-- 
2.25.1

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


[PATCH 26/44] drm/gm12u320: Don't use drm_device->dev_private

2020-04-03 Thread Daniel Vetter
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Signed-off-by: Daniel Vetter 
Cc: Hans de Goede 
---
 drivers/gpu/drm/tiny/gm12u320.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 907739a67bf6..cc397671f689 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -98,6 +98,8 @@ struct gm12u320_device {
} fb_update;
 };
 
+#define to_gm12u320(__dev) container_of(__dev, struct gm12u320_device, dev)
+
 static const char cmd_data[CMD_SIZE] = {
0x55, 0x53, 0x42, 0x43, 0x00, 0x00, 0x00, 0x00,
0x68, 0xfc, 0x00, 0x00, 0x00, 0x00, 0x10, 0xff,
@@ -408,7 +410,7 @@ static void gm12u320_fb_update_work(struct work_struct 
*work)
 static void gm12u320_fb_mark_dirty(struct drm_framebuffer *fb,
   struct drm_rect *dirty)
 {
-   struct gm12u320_device *gm12u320 = fb->dev->dev_private;
+   struct gm12u320_device *gm12u320 = to_gm12u320(fb->dev);
struct drm_framebuffer *old_fb = NULL;
bool wakeup = false;
 
@@ -558,7 +560,7 @@ static void gm12u320_pipe_enable(struct 
drm_simple_display_pipe *pipe,
 struct drm_plane_state *plane_state)
 {
struct drm_rect rect = { 0, 0, GM12U320_USER_WIDTH, GM12U320_HEIGHT };
-   struct gm12u320_device *gm12u320 = pipe->crtc.dev->dev_private;
+   struct gm12u320_device *gm12u320 = to_gm12u320(pipe->crtc.dev);
 
gm12u320->fb_update.draw_status_timeout = FIRST_FRAME_TIMEOUT;
gm12u320_fb_mark_dirty(plane_state->fb, );
@@ -566,7 +568,7 @@ static void gm12u320_pipe_enable(struct 
drm_simple_display_pipe *pipe,
 
 static void gm12u320_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-   struct gm12u320_device *gm12u320 = pipe->crtc.dev->dev_private;
+   struct gm12u320_device *gm12u320 = to_gm12u320(pipe->crtc.dev);
 
gm12u320_stop_fb_update(gm12u320);
 }
@@ -641,7 +643,6 @@ static int gm12u320_usb_probe(struct usb_interface 
*interface,
mutex_init(>fb_update.lock);
 
dev = >dev;
-   dev->dev_private = gm12u320;
 
ret = drmm_mode_config_init(dev);
if (ret)
@@ -706,7 +707,7 @@ static __maybe_unused int gm12u320_suspend(struct 
usb_interface *interface,
 static __maybe_unused int gm12u320_resume(struct usb_interface *interface)
 {
struct drm_device *dev = usb_get_intfdata(interface);
-   struct gm12u320_device *gm12u320 = dev->dev_private;
+   struct gm12u320_device *gm12u320 = to_gm12u320(dev);
 
gm12u320_set_ecomode(gm12u320);
 
-- 
2.25.1

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


[PATCH 13/44] drm/v3d: Delete v3d_dev->dev

2020-04-03 Thread Daniel Vetter
We already have it in v3d_dev->drm.dev with zero additional pointer
chasing. Personally I don't like duplicated pointers like this
because:
- reviewers need to check whether the pointer is for the same or
  different objects if there's multiple
- compilers have an easier time too

But also a bit a bikeshed, so feel free to ignore.

Signed-off-by: Daniel Vetter 
Cc: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 12 ++--
 drivers/gpu/drm/v3d/v3d_drv.c | 12 ++--
 drivers/gpu/drm/v3d/v3d_drv.h |  2 --
 drivers/gpu/drm/v3d/v3d_gem.c | 17 +
 drivers/gpu/drm/v3d/v3d_irq.c | 12 ++--
 drivers/gpu/drm/v3d/v3d_mmu.c | 10 +-
 drivers/gpu/drm/v3d/v3d_sched.c   | 10 +-
 7 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c 
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 2b0ea5f8febd..e76b24bb8828 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -132,7 +132,7 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void 
*unused)
u32 ident0, ident1, ident2, ident3, cores;
int ret, core;
 
-   ret = pm_runtime_get_sync(v3d->dev);
+   ret = pm_runtime_get_sync(v3d->drm.dev);
if (ret < 0)
return ret;
 
@@ -187,8 +187,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void 
*unused)
   (misccfg & V3D_MISCCFG_OVRTMUOUT) != 0);
}
 
-   pm_runtime_mark_last_busy(v3d->dev);
-   pm_runtime_put_autosuspend(v3d->dev);
+   pm_runtime_mark_last_busy(v3d->drm.dev);
+   pm_runtime_put_autosuspend(v3d->drm.dev);
 
return 0;
 }
@@ -219,7 +219,7 @@ static int v3d_measure_clock(struct seq_file *m, void 
*unused)
int measure_ms = 1000;
int ret;
 
-   ret = pm_runtime_get_sync(v3d->dev);
+   ret = pm_runtime_get_sync(v3d->drm.dev);
if (ret < 0)
return ret;
 
@@ -245,8 +245,8 @@ static int v3d_measure_clock(struct seq_file *m, void 
*unused)
   cycles / (measure_ms * 1000),
   (cycles / (measure_ms * 100)) % 10);
 
-   pm_runtime_mark_last_busy(v3d->dev);
-   pm_runtime_put_autosuspend(v3d->dev);
+   pm_runtime_mark_last_busy(v3d->drm.dev);
+   pm_runtime_put_autosuspend(v3d->drm.dev);
 
return 0;
 }
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index f57d408ef371..37cb880f2826 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -105,7 +105,7 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void 
*data,
if (args->value != 0)
return -EINVAL;
 
-   ret = pm_runtime_get_sync(v3d->dev);
+   ret = pm_runtime_get_sync(v3d->drm.dev);
if (ret < 0)
return ret;
if (args->param >= DRM_V3D_PARAM_V3D_CORE0_IDENT0 &&
@@ -114,8 +114,8 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void 
*data,
} else {
args->value = V3D_READ(offset);
}
-   pm_runtime_mark_last_busy(v3d->dev);
-   pm_runtime_put_autosuspend(v3d->dev);
+   pm_runtime_mark_last_busy(v3d->drm.dev);
+   pm_runtime_put_autosuspend(v3d->drm.dev);
return 0;
}
 
@@ -237,7 +237,7 @@ map_regs(struct v3d_dev *v3d, void __iomem **regs, const 
char *name)
struct resource *res =
platform_get_resource_byname(v3d->pdev, IORESOURCE_MEM, name);
 
-   *regs = devm_ioremap_resource(v3d->dev, res);
+   *regs = devm_ioremap_resource(v3d->drm.dev, res);
return PTR_ERR_OR_ZERO(*regs);
 }
 
@@ -255,7 +255,6 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
if (IS_ERR(v3d))
return PTR_ERR(v3d);
 
-   v3d->dev = dev;
v3d->pdev = pdev;
drm = >drm;
 
@@ -345,7 +344,8 @@ static int v3d_platform_drm_remove(struct platform_device 
*pdev)
 
v3d_gem_destroy(drm);
 
-   dma_free_wc(v3d->dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
+   dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch,
+   v3d->mmu_scratch_paddr);
 
return 0;
 }
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 112d80aed5f6..4d2d1f2fe1af 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -14,7 +14,6 @@
 #include "uapi/drm/v3d_drm.h"
 
 struct clk;
-struct device;
 struct platform_device;
 struct reset_control;
 
@@ -47,7 +46,6 @@ struct v3d_dev {
int ver;
bool single_irq_line;
 
-   struct device *dev;
struct platform_device *pdev;
void __iomem *hub_regs;
void __iomem *core_regs[3];
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 549dde83408b..09a7639cf161 100644
--- 

[PATCH 14/44] drm/v3d: Delete v3d_dev->pdev

2020-04-03 Thread Daniel Vetter
We already have it in v3d_dev->drm.dev with zero additional pointer
chasing. Personally I don't like duplicated pointers like this
because:
- reviewers need to check whether the pointer is for the same or
different objects if there's multiple
- compilers have an easier time too

To avoid having to pull in some big headers I implemented the casting
function as a macro instead of a static inline. Typechecking thanks to
container_of still assured.

But also a bit a bikeshed, so feel free to ignore.

Signed-off-by: Daniel Vetter 
Cc: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_drv.c | 3 +--
 drivers/gpu/drm/v3d/v3d_drv.h | 3 ++-
 drivers/gpu/drm/v3d/v3d_irq.c | 8 +---
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 37cb880f2826..82a7dfdd14c2 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -235,7 +235,7 @@ static int
 map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
 {
struct resource *res =
-   platform_get_resource_byname(v3d->pdev, IORESOURCE_MEM, name);
+   platform_get_resource_byname(v3d_to_pdev(v3d), IORESOURCE_MEM, 
name);
 
*regs = devm_ioremap_resource(v3d->drm.dev, res);
return PTR_ERR_OR_ZERO(*regs);
@@ -255,7 +255,6 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
if (IS_ERR(v3d))
return PTR_ERR(v3d);
 
-   v3d->pdev = pdev;
drm = >drm;
 
platform_set_drvdata(pdev, drm);
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 4d2d1f2fe1af..935f23b524b2 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -46,7 +46,6 @@ struct v3d_dev {
int ver;
bool single_irq_line;
 
-   struct platform_device *pdev;
void __iomem *hub_regs;
void __iomem *core_regs[3];
void __iomem *bridge_regs;
@@ -128,6 +127,8 @@ v3d_has_csd(struct v3d_dev *v3d)
return v3d->ver >= 41;
 }
 
+#define v3d_to_pdev(v3d) to_platform_device(v3d->drm.dev)
+
 /* The per-fd struct, which tracks the MMU mappings. */
 struct v3d_file_priv {
struct v3d_dev *v3d;
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index f4ce6d057c90..51b65263c657 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -217,7 +217,7 @@ v3d_irq_init(struct v3d_dev *v3d)
V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS);
V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS);
 
-   irq1 = platform_get_irq(v3d->pdev, 1);
+   irq1 = platform_get_irq(v3d_to_pdev(v3d), 1);
if (irq1 == -EPROBE_DEFER)
return irq1;
if (irq1 > 0) {
@@ -226,7 +226,8 @@ v3d_irq_init(struct v3d_dev *v3d)
   "v3d_core0", v3d);
if (ret)
goto fail;
-   ret = devm_request_irq(v3d->drm.dev, 
platform_get_irq(v3d->pdev, 0),
+   ret = devm_request_irq(v3d->drm.dev,
+  platform_get_irq(v3d_to_pdev(v3d), 0),
   v3d_hub_irq, IRQF_SHARED,
   "v3d_hub", v3d);
if (ret)
@@ -234,7 +235,8 @@ v3d_irq_init(struct v3d_dev *v3d)
} else {
v3d->single_irq_line = true;
 
-   ret = devm_request_irq(v3d->drm.dev, 
platform_get_irq(v3d->pdev, 0),
+   ret = devm_request_irq(v3d->drm.dev,
+  platform_get_irq(v3d_to_pdev(v3d), 0),
   v3d_irq, IRQF_SHARED,
   "v3d", v3d);
if (ret)
-- 
2.25.1

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


[PATCH 25/44] drm/gm12u320: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Hans de Goede 
---
 drivers/gpu/drm/tiny/gm12u320.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 6f0ea2827d62..907739a67bf6 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -631,22 +631,17 @@ static int gm12u320_usb_probe(struct usb_interface 
*interface,
if (interface->cur_altsetting->desc.bInterfaceNumber != 0)
return -ENODEV;
 
-   gm12u320 = kzalloc(sizeof(*gm12u320), GFP_KERNEL);
-   if (gm12u320 == NULL)
-   return -ENOMEM;
+   gm12u320 = devm_drm_dev_alloc(>dev, _drm_driver,
+ struct gm12u320_device, dev);
+   if (IS_ERR(gm12u320))
+   return PTR_ERR(gm12u320);
 
gm12u320->udev = interface_to_usbdev(interface);
INIT_DELAYED_WORK(>fb_update.work, gm12u320_fb_update_work);
mutex_init(>fb_update.lock);
 
dev = >dev;
-   ret = devm_drm_dev_init(>dev, dev, _drm_driver);
-   if (ret) {
-   kfree(gm12u320);
-   return ret;
-   }
dev->dev_private = gm12u320;
-   drmm_add_final_kfree(dev, gm12u320);
 
ret = drmm_mode_config_init(dev);
if (ret)
-- 
2.25.1

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


[PATCH 29/44] drm/tidss: Delete tidss->saved_state

2020-04-03 Thread Daniel Vetter
Not used anymore since the switch to suspend/resume helpers.

Signed-off-by: Daniel Vetter 
Cc: Jyri Sarha 
Cc: Tomi Valkeinen 
---
 drivers/gpu/drm/tidss/tidss_drv.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_drv.h 
b/drivers/gpu/drm/tidss/tidss_drv.h
index b23cd95c8d78..3b0a3d87b7c4 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.h
+++ b/drivers/gpu/drm/tidss/tidss_drv.h
@@ -29,8 +29,6 @@ struct tidss_device {
 
spinlock_t wait_lock;   /* protects the irq masks */
dispc_irq_t irq_mask;   /* enabled irqs in addition to wait_list */
-
-   struct drm_atomic_state *saved_state;
 };
 
 #define to_tidss(__dev) container_of(__dev, struct tidss_device, ddev)
-- 
2.25.1

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


[PATCH 21/44] drm/ili9486: Use devm_drm_dev_alloc

2020-04-03 Thread Daniel Vetter
Already using devm_drm_dev_init, so very simple replacment.

Signed-off-by: Daniel Vetter 
Cc: Kamlesh Gurudasani 
---
 drivers/gpu/drm/tiny/ili9486.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index c4079bf9e2c8..2702ea557d29 100644
--- a/drivers/gpu/drm/tiny/ili9486.c
+++ b/drivers/gpu/drm/tiny/ili9486.c
@@ -197,18 +197,13 @@ static int ili9486_probe(struct spi_device *spi)
u32 rotation = 0;
int ret;
 
-   dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL);
-   if (!dbidev)
-   return -ENOMEM;
+   dbidev = devm_drm_dev_alloc(dev, _driver,
+   struct mipi_dbi_dev, drm);
+   if (IS_ERR(dbidev))
+   return PTR_ERR(dbidev);
 
dbi = >dbi;
drm = >drm;
-   ret = devm_drm_dev_init(dev, drm, _driver);
-   if (ret) {
-   kfree(dbidev);
-   return ret;
-   }
-   drmm_add_final_kfree(drm, dbidev);
 
dbi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(dbi->reset)) {
-- 
2.25.1

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


[PATCH 31/44] drm/qxl: Don't use drm_device->dev_private

2020-04-03 Thread Daniel Vetter
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Gerd Hoffmann 
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-de...@lists.freedesktop.org
---
 drivers/gpu/drm/qxl/qxl_debugfs.c |  7 +++
 drivers/gpu/drm/qxl/qxl_display.c | 32 +++
 drivers/gpu/drm/qxl/qxl_drv.c |  8 
 drivers/gpu/drm/qxl/qxl_drv.h |  4 ++--
 drivers/gpu/drm/qxl/qxl_dumb.c|  2 +-
 drivers/gpu/drm/qxl/qxl_gem.c |  2 +-
 drivers/gpu/drm/qxl/qxl_ioctl.c   | 14 +++---
 drivers/gpu/drm/qxl/qxl_irq.c |  2 +-
 drivers/gpu/drm/qxl/qxl_kms.c |  1 -
 drivers/gpu/drm/qxl/qxl_object.c  |  2 +-
 drivers/gpu/drm/qxl/qxl_release.c |  2 +-
 drivers/gpu/drm/qxl/qxl_ttm.c |  2 +-
 12 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c 
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 88123047fdd4..524d35b648d8 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -39,7 +39,7 @@ static int
 qxl_debugfs_irq_received(struct seq_file *m, void *data)
 {
struct drm_info_node *node = (struct drm_info_node *) m->private;
-   struct qxl_device *qdev = node->minor->dev->dev_private;
+   struct qxl_device *qdev = to_qxl(node->minor->dev);
 
seq_printf(m, "%d\n", atomic_read(>irq_received));
seq_printf(m, "%d\n", atomic_read(>irq_received_display));
@@ -53,7 +53,7 @@ static int
 qxl_debugfs_buffers_info(struct seq_file *m, void *data)
 {
struct drm_info_node *node = (struct drm_info_node *) m->private;
-   struct qxl_device *qdev = node->minor->dev->dev_private;
+   struct qxl_device *qdev = to_qxl(node->minor->dev);
struct qxl_bo *bo;
 
list_for_each_entry(bo, >gem.objects, list) {
@@ -83,8 +83,7 @@ void
 qxl_debugfs_init(struct drm_minor *minor)
 {
 #if defined(CONFIG_DEBUG_FS)
-   struct qxl_device *dev =
-   (struct qxl_device *) minor->dev->dev_private;
+   struct qxl_device *dev = to_qxl(minor->dev);
 
drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
 minor->debugfs_root, minor);
diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index 09583a08e141..1082cd5d2fd4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -221,7 +221,7 @@ static int qxl_add_mode(struct drm_connector *connector,
bool preferred)
 {
struct drm_device *dev = connector->dev;
-   struct qxl_device *qdev = dev->dev_private;
+   struct qxl_device *qdev = to_qxl(dev);
struct drm_display_mode *mode = NULL;
int rc;
 
@@ -242,7 +242,7 @@ static int qxl_add_mode(struct drm_connector *connector,
 static int qxl_add_monitors_config_modes(struct drm_connector *connector)
 {
struct drm_device *dev = connector->dev;
-   struct qxl_device *qdev = dev->dev_private;
+   struct qxl_device *qdev = to_qxl(dev);
struct qxl_output *output = drm_connector_to_qxl_output(connector);
int h = output->index;
struct qxl_head *head;
@@ -310,7 +310,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc 
*crtc,
const char *reason)
 {
struct drm_device *dev = crtc->dev;
-   struct qxl_device *qdev = dev->dev_private;
+   struct qxl_device *qdev = to_qxl(dev);
struct qxl_crtc *qcrtc = to_qxl_crtc(crtc);
struct qxl_head head;
int oldcount, i = qcrtc->index;
@@ -400,7 +400,7 @@ static int qxl_framebuffer_surface_dirty(struct 
drm_framebuffer *fb,
 unsigned int num_clips)
 {
/* TODO: vmwgfx where this was cribbed from had locking. Why? */
-   struct qxl_device *qdev = fb->dev->dev_private;
+   struct qxl_device *qdev = to_qxl(fb->dev);
struct drm_clip_rect norect;
struct qxl_bo *qobj;
bool is_primary;
@@ -462,7 +462,7 @@ static const struct drm_crtc_helper_funcs 
qxl_crtc_helper_funcs = {
 static int qxl_primary_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
 {
-   struct qxl_device *qdev = plane->dev->dev_private;
+   struct qxl_device *qdev = to_qxl(plane->dev);
struct qxl_bo *bo;
 
if (!state->crtc || !state->fb)
@@ -476,7 +476,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane,
 static int qxl_primary_apply_cursor(struct drm_plane *plane)
 {
struct drm_device *dev = plane->dev;
-   struct qxl_device *qdev = dev->dev_private;
+   struct qxl_device *qdev = to_qxl(dev);
struct drm_framebuffer *fb = plane->state->fb;
struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
struct qxl_cursor_cmd *cmd;
@@ -523,7 +523,7 @@ static int qxl_primary_apply_cursor(struct 

  1   2   >