Re: [Freedreno] [bug report] drm/msm/a6xx: Fix llcc configuration for a660 gpu

2021-10-13 Thread Akhil P Oommen

On 10/12/2021 3:30 PM, Dan Carpenter wrote:

Hello Akhil P Oommen,

The patch a6f24383f6c0: "drm/msm/a6xx: Fix llcc configuration for
a660 gpu" from Jul 30, 2021, leads to the following Smatch static
checker warning:

drivers/gpu/drm/msm/adreno/a6xx_gpu.c:1480 a6xx_llc_activate()
error: uninitialized symbol 'gpu_scid'.

drivers/gpu/drm/msm/adreno/a6xx_gpu.c
 1423 static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
 1424 {
 1425 struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
 1426 struct msm_gpu *gpu = &adreno_gpu->base;
 1427 u32 gpu_scid, cntl1_regval = 0;
 1428
 1429 if (IS_ERR(a6xx_gpu->llc_mmio))
 1430 return;
 1431
 1432 if (!llcc_slice_activate(a6xx_gpu->llc_slice)) {
 1433 gpu_scid = llcc_get_slice_id(a6xx_gpu->llc_slice);
 1434
 1435 gpu_scid &= 0x1f;
 1436 cntl1_regval = (gpu_scid << 0) | (gpu_scid << 5) | 
(gpu_scid << 10) |
 1437(gpu_scid << 15) | (gpu_scid << 20);
 1438 }

gpu_scid not initialized on the else path.

 1439
 1440 /*
 1441  * For targets with a MMU500, activate the slice but don't 
program the
 1442  * register.  The XBL will take care of that.
 1443  */
 1444 if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) {
 1445 if (!a6xx_gpu->have_mmu500) {
 1446 u32 gpuhtw_scid = 
llcc_get_slice_id(a6xx_gpu->htw_llc_slice);
 1447
 1448 gpuhtw_scid &= 0x1f;
 1449 cntl1_regval |= FIELD_PREP(GENMASK(29, 25), 
gpuhtw_scid);
 1450 }
 1451 }
 1452
 1453 if (!cntl1_regval)
 1454 return;
 1455
 1456 /*
 1457  * Program the slice IDs for the various GPU blocks and GPU 
MMU
 1458  * pagetables
 1459  */
 1460 if (!a6xx_gpu->have_mmu500) {
 1461 a6xx_llc_write(a6xx_gpu,
 1462 REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1, 
cntl1_regval);
 1463
 1464 /*
 1465  * Program cacheability overrides to not allocate 
cache
 1466  * lines on a write miss
 1467  */
 1468 a6xx_llc_rmw(a6xx_gpu,
 1469 REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF, 
0x03);
 1470 return;
 1471 }
 1472
 1473 gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL1, GENMASK(24, 0), 
cntl1_regval);
 1474
 1475 /* On A660, the SCID programming for UCHE traffic is done in
 1476  * A6XX_GBIF_SCACHE_CNTL0[14:10]
 1477  */
 1478 if (adreno_is_a660_family(adreno_gpu))
 1479 gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL0, (0x1f << 10) 
|
--> 1480 (1 << 8), (gpu_scid << 10) | (1 << 8));
 ^^
Used here.

 1481 }

regards,
dan carpenter



Thanks, Dan. I will fix this in the stack I share.

-Akhil.


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-10-13 Thread Rob Clark
On Wed, Oct 13, 2021 at 7:16 AM Maxime Ripard  wrote:
>
> Hi Caleb,
>
> On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> > Hi,
> >
> > On 30/09/2021 20:49, Amit Pundir wrote:
> > > On Thu, 30 Sept 2021 at 04:50, Rob Clark  wrote:
> > > >
> > > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz  
> > > > wrote:
> > > > >
> > > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  
> > > > > wrote:
> > > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz 
> > > > > >  wrote:
> > > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  
> > > > > > > wrote:
> > > > > > > > The best practice to avoid those issues is to register its 
> > > > > > > > functions only after
> > > > > > > > all its dependencies are live. We also shouldn't wait any 
> > > > > > > > longer than we should
> > > > > > > > to play nice with the other components that are waiting for us, 
> > > > > > > > so in our case
> > > > > > > > that would mean moving the DSI device registration to the 
> > > > > > > > bridge probe.
> > > > > > > >
> > > > > > > > I also had a look at all the DSI hosts, and it seems that 
> > > > > > > > exynos, kirin and msm
> > > > > > > > would be affected by this and wouldn't probe anymore after 
> > > > > > > > those changes.
> > > > > > > > Exynos and kirin seems to be simple enough for a mechanical 
> > > > > > > > change (that still
> > > > > > > > requires to be tested), but the changes in msm seemed to be far 
> > > > > > > > more important
> > > > > > > > and I wasn't confortable doing them.
> > > > > > >
> > > > > > >
> > > > > > > Hey Maxime,
> > > > > > >Sorry for taking so long to get to this, but now that plumbers 
> > > > > > > is
> > > > > > > over I've had a chance to check it out on kirin
> > > > > > >
> > > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > > > 
> > > > > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > >
> > > > > > > But trying to boot hikey with that, I see the following loop 
> > > > > > > indefinitely:
> > > > > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > > > > regulator
> > > > > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > > > > regulator
> > > > > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > > > > regulator
> > > > > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using 
> > > > > > > dummy regulator
> > > > > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > > > > regulator
> > > > > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > > > > regulator
> > > > > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > > > > >
> > > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > > I'll retest and let you know.
> > > > >
> > > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > > still seeing the same thing.  :(
> > > > >
> > > > > Will dig a bit and let you know when I find more.
> > > >
> > > > Did you have a chance to test it on anything using drm/msm with DSI
> > > > panels?  That would at least confirm that I didn't miss anything in
> > > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > >
> > > Hi, smoke tested
> > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > > regressions in my limited testing so far including video (youtube)
> > > playback.
> > Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> > FBDEV_EMULATION (so we can get a working framebuffer console) which was
> > otherwise broken on 5.15.
> >
> > However it spits out some warnings during boot: 
> > https://p.calebs.dev/gucysowyna.yaml
>
> Thanks for testing. It looks like the runtime_pm ordering between the
> msm devices changed a bit with the conversion Rob did.
>
> Rob, do you know what could be going on?
>

Not entirely sure.. I didn't see that first splat, but maybe I was
missing some debug config? (The 2nd one is kind of "normal", I think
related to bootloader leaving the display on)

BR,
-R


[Freedreno] [PATCH v4 20/20] drm: cleanup: remove acquire_ctx from drm_mode_config

2021-10-13 Thread Fernando Ramos
The previous patch removed drm_modeset_{lock,unlock}_all, which were the
only users of this field inside the drm_mode_config structure.

Signed-off-by: Fernando Ramos 
---
 include/drm/drm_mode_config.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..b214b07157f2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -383,16 +383,6 @@ struct drm_mode_config {
 */
struct drm_modeset_lock connection_mutex;
 
-   /**
-* @acquire_ctx:
-*
-* Global implicit acquire context used by atomic drivers for legacy
-* IOCTLs. Deprecated, since implicit locking contexts make it
-* impossible to use driver-private &struct drm_modeset_lock. Users of
-* this must hold @mutex.
-*/
-   struct drm_modeset_acquire_ctx *acquire_ctx;
-
/**
 * @idr_mutex:
 *
-- 
2.33.0



[Freedreno] [PATCH v4 19/20] drm: cleanup: remove drm_modeset_(un)lock_all()

2021-10-13 Thread Fernando Ramos
Functions drm_modeset_lock_all() and drm_modeset_unlock_all() are no
longer used anywhere and can be removed.

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/drm_modeset_lock.c | 94 +-
 include/drm/drm_modeset_lock.h |  2 -
 2 files changed, 3 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c 
b/drivers/gpu/drm/drm_modeset_lock.c
index 4d32b61fa1fd..b2b84ca2b738 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -117,93 +117,6 @@ static void __stack_depot_print(depot_stack_handle_t 
stack_depot)
 }
 #endif /* CONFIG_DRM_DEBUG_MODESET_LOCK */
 
-/**
- * drm_modeset_lock_all - take all modeset locks
- * @dev: DRM device
- *
- * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped by calling the
- * drm_modeset_unlock_all() function.
- *
- * This function is deprecated. It allocates a lock acquisition context and
- * stores it in &drm_device.mode_config. This facilitate conversion of
- * existing code because it removes the need to manually deal with the
- * acquisition context, but it is also brittle because the context is global
- * and care must be taken not to nest calls. New code should use the
- * drm_modeset_lock_all_ctx() function and pass in the context explicitly.
- */
-void drm_modeset_lock_all(struct drm_device *dev)
-{
-   struct drm_mode_config *config = &dev->mode_config;
-   struct drm_modeset_acquire_ctx *ctx;
-   int ret;
-
-   ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
-   if (WARN_ON(!ctx))
-   return;
-
-   mutex_lock(&config->mutex);
-
-   drm_modeset_acquire_init(ctx, 0);
-
-retry:
-   ret = drm_modeset_lock_all_ctx(dev, ctx);
-   if (ret < 0) {
-   if (ret == -EDEADLK) {
-   drm_modeset_backoff(ctx);
-   goto retry;
-   }
-
-   drm_modeset_acquire_fini(ctx);
-   kfree(ctx);
-   return;
-   }
-   ww_acquire_done(&ctx->ww_ctx);
-
-   WARN_ON(config->acquire_ctx);
-
-   /*
-* We hold the locks now, so it is safe to stash the acquisition
-* context for drm_modeset_unlock_all().
-*/
-   config->acquire_ctx = ctx;
-
-   drm_warn_on_modeset_not_all_locked(dev);
-}
-EXPORT_SYMBOL(drm_modeset_lock_all);
-
-/**
- * drm_modeset_unlock_all - drop all modeset locks
- * @dev: DRM device
- *
- * This function drops all modeset locks taken by a previous call to the
- * drm_modeset_lock_all() function.
- *
- * This function is deprecated. It uses the lock acquisition context stored
- * in &drm_device.mode_config. This facilitates conversion of existing
- * code because it removes the need to manually deal with the acquisition
- * context, but it is also brittle because the context is global and care must
- * be taken not to nest calls. New code should pass the acquisition context
- * directly to the drm_modeset_drop_locks() function.
- */
-void drm_modeset_unlock_all(struct drm_device *dev)
-{
-   struct drm_mode_config *config = &dev->mode_config;
-   struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
-
-   if (WARN_ON(!ctx))
-   return;
-
-   config->acquire_ctx = NULL;
-   drm_modeset_drop_locks(ctx);
-   drm_modeset_acquire_fini(ctx);
-
-   kfree(ctx);
-
-   mutex_unlock(&dev->mode_config.mutex);
-}
-EXPORT_SYMBOL(drm_modeset_unlock_all);
-
 /**
  * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
  * @dev: device
@@ -425,10 +338,9 @@ EXPORT_SYMBOL(drm_modeset_unlock);
  * This function takes all modeset locks, suitable where a more fine-grained
  * scheme isn't (yet) implemented.
  *
- * Unlike drm_modeset_lock_all(), it doesn't take the &drm_mode_config.mutex
- * since that lock isn't required for modeset state changes. Callers which
- * need to grab that lock too need to do so outside of the acquire context
- * @ctx.
+ * It doesn't take the &drm_mode_config.mutex since that lock isn't required 
for
+ * modeset state changes. Callers which need to grab that lock too need to do 
so
+ * outside of the acquire context @ctx.
  *
  * Locks acquired with this function should be released by calling the
  * drm_modeset_drop_locks() function on @ctx.
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index b84693fbd2b5..96b853530120 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -140,8 +140,6 @@ struct drm_device;
 struct drm_crtc;
 struct drm_plane;
 
-void drm_modeset_lock_all(struct drm_device *dev);
-void drm_modeset_unlock_all(struct drm_device *dev);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 
 int drm_modeset_lock_all_ctx(struct drm_device *dev,
-- 
2.33.0



[Freedreno] [PATCH v4 18/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3]

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

NOTE:

While this change is similar to the one done two commits ago, it
contains an important extra nuances that I'm going to explain next.

The only difference between the old drm_modeset_{lock,unlock}_all()
functions and the new DRM_MODESET_LOCK_ALL_{BEGIN,END}() macros is that
the former use a global context stored in dev->mode_config.acquire_ctx
while the latter depend on a user provided one (typically in the stack).

This means that as long as no one accesses the global
dev->mode_config.acquire_ctx context in the block that runs between
lock/BEGIN and unlock/END, the code should be equivalent before and
after my changes.

Turns out that, while not obvious at first sight, the call to
dm_restore_drm_connector_state() done between drm_modset_lock_all() and
drm_modeset_unlock_all() ends up using that global context structure
stored in dev.

To fix this we need to update some function prototypes to accept the
new stack allocated variable as an argument.

Signed-off-by: Fernando Ramos 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 27 ---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 13 ++---
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 10ed1f8ad514..7a3c5def9fb9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -81,6 +81,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
@@ -2906,6 +2907,8 @@ static void handle_hpd_irq_helper(struct 
amdgpu_dm_connector *aconnector)
struct amdgpu_device *adev = drm_to_adev(dev);
struct dm_connector_state *dm_con_state = 
to_dm_connector_state(connector->state);
struct dm_crtc_state *dm_crtc_state = NULL;
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
if (adev->dm.disable_hpd_irq)
return;
@@ -2947,9 +2950,9 @@ static void handle_hpd_irq_helper(struct 
amdgpu_dm_connector *aconnector)
goto out;
}
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+   dm_restore_drm_connector_state(dev, connector, &ctx);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
drm_kms_helper_hotplug_event(dev);
@@ -3070,6 +3073,7 @@ static void handle_hpd_rx_irq(void *param)
struct drm_connector *connector = &aconnector->base;
struct drm_device *dev = connector->dev;
struct dc_link *dc_link = aconnector->dc_link;
+   struct drm_modeset_acquire_ctx ctx;
bool is_mst_root_connector = aconnector->mst_mgr.mst_state;
bool result = false;
enum dc_connection_type new_connection_type = dc_connection_none;
@@ -3079,6 +3083,7 @@ static void handle_hpd_rx_irq(void *param)
bool has_left_work = false;
int idx = aconnector->base.index;
struct hpd_rx_irq_offload_work_queue *offload_wq = 
&adev->dm.hpd_rx_offload_wq[idx];
+   int ret;
 
memset(&hpd_irq_data, 0, sizeof(hpd_irq_data));
 
@@ -3153,9 +3158,9 @@ static void handle_hpd_rx_irq(void *param)
goto finish;
}
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+   dm_restore_drm_connector_state(dev, connector, &ctx);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
drm_kms_helper_hotplug_event(dev);
}
@@ -9703,7 +9708,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
 }
 
 
-static int dm_force_atomic_commit(struct drm_connector *connector)
+static int dm_force_atomic_commit(struct drm_connector *connector,
+ struct drm_modeset_acquire_ctx *ctx)
 {
int ret = 0;
struct drm_device *ddev = connector->dev;
@@ -9717,7 +9723,7 @@ static int dm_force_atomic_commit(struct drm_connector 
*connector)
if (!state)
return -ENOMEM;
 
-   state->acquire_ctx = ddev->mode_config.acquire_ctx;
+   state->acquire_ctx = ctx;
 
/* Construct an atomic state to restore previous display setting */
 
@@ -9764,7 +9770,8 @@ static int dm_force_atomic_commit(struct drm_connector 
*connector)
  * same port and when running without usermode desktop manager supprot
  */
 void dm_restore_drm_connector_state(struct drm_device *dev,
-  

[Freedreno] [PATCH v4 16/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index dc50c05f23fc..0ea7bdbc8482 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static void amdgpu_display_flip_callback(struct dma_fence *f,
 struct dma_fence_cb *cb)
@@ -1574,16 +1575,21 @@ int amdgpu_display_suspend_helper(struct amdgpu_device 
*adev)
struct drm_crtc *crtc;
struct drm_connector *connector;
struct drm_connector_list_iter iter;
-   int r;
+   struct drm_modeset_acquire_ctx ctx;
+   int r, ret;
 
/* turn off display hw */
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_connector_list_iter_begin(dev, &iter);
drm_for_each_connector_iter(connector, &iter)
drm_helper_connector_dpms(connector,
  DRM_MODE_DPMS_OFF);
drm_connector_list_iter_end(&iter);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+   if (ret)
+   return ret;
+
/* unpin the front buffers and cursors */
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
@@ -1621,7 +1627,8 @@ int amdgpu_display_resume_helper(struct amdgpu_device 
*adev)
struct drm_connector *connector;
struct drm_connector_list_iter iter;
struct drm_crtc *crtc;
-   int r;
+   struct drm_modeset_acquire_ctx ctx;
+   int r, ret;
 
/* pin cursors */
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -1643,7 +1650,7 @@ int amdgpu_display_resume_helper(struct amdgpu_device 
*adev)
drm_helper_resume_force_mode(dev);
 
/* turn on display hw */
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
drm_connector_list_iter_begin(dev, &iter);
drm_for_each_connector_iter(connector, &iter)
@@ -1651,8 +1658,8 @@ int amdgpu_display_resume_helper(struct amdgpu_device 
*adev)
  DRM_MODE_DPMS_ON);
drm_connector_list_iter_end(&iter);
 
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
-   return 0;
+   return ret;
 }
 
-- 
2.33.0



[Freedreno] [PATCH v4 17/20] drm/amd: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2]

2021-10-13 Thread Fernando Ramos
Refactor places using drm_modeset_{lock,unlock}_all() so that they only
appear once per function.

This is needed so that in the next commit I can replace those functions
by the new macros (which use labels that can only appear once per
function).

Signed-off-by: Fernando Ramos 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 43 ---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 18 +++-
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f35561b5a465..10ed1f8ad514 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2935,13 +2935,6 @@ static void handle_hpd_irq_helper(struct 
amdgpu_dm_connector *aconnector)
if (aconnector->base.force && new_connection_type == 
dc_connection_none) {
emulated_link_detect(aconnector->dc_link);
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
-
-   if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-   drm_kms_helper_hotplug_event(dev);
-
} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
if (new_connection_type == dc_connection_none &&
aconnector->dc_link->type == dc_connection_none &&
@@ -2950,13 +2943,18 @@ static void handle_hpd_irq_helper(struct 
amdgpu_dm_connector *aconnector)
 
amdgpu_dm_update_connector_after_detect(aconnector);
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
-
-   if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-   drm_kms_helper_hotplug_event(dev);
+   } else {
+   goto out;
}
+
+   drm_modeset_lock_all(dev);
+   dm_restore_drm_connector_state(dev, connector);
+   drm_modeset_unlock_all(dev);
+
+   if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
+   drm_kms_helper_hotplug_event(dev);
+
+out:
mutex_unlock(&aconnector->hpd_lock);
 
 }
@@ -3144,12 +3142,6 @@ static void handle_hpd_rx_irq(void *param)
 
amdgpu_dm_update_connector_after_detect(aconnector);
 
-
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
-
-   drm_kms_helper_hotplug_event(dev);
} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
 
if (aconnector->fake_enable)
@@ -3157,14 +3149,17 @@ static void handle_hpd_rx_irq(void *param)
 
amdgpu_dm_update_connector_after_detect(aconnector);
 
+   } else {
+   goto finish;
+   }
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
+   drm_modeset_lock_all(dev);
+   dm_restore_drm_connector_state(dev, connector);
+   drm_modeset_unlock_all(dev);
 
-   drm_kms_helper_hotplug_event(dev);
-   }
+   drm_kms_helper_hotplug_event(dev);
}
+finish:
 #ifdef CONFIG_DRM_AMD_DC_HDCP
if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
if (adev->dm.hdcp_workqueue)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 814f67d86a3c..7751038d5788 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1237,12 +1237,6 @@ static ssize_t trigger_hotplug(struct file *f, const 
char __user *buf,
goto unlock;
 
amdgpu_dm_update_connector_after_detect(aconnector);
-
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
-
-   drm_kms_helper_hotplug_event(dev);
} else if (param[0] == 0) {
if (!aconnector->dc_link)
goto unlock;
@@ -1260,13 +1254,15 @@ static ssize_t trigger_hotplug(struct file *f, const 
char __user *buf,
 
amdgpu_dm_update_connector_after_detect(aconnector);
 
-   drm_modeset_lock_all(dev);
-   dm_restore_drm_connector_state(dev, connector);
-   drm_modeset_unlock_all(dev);
-
-   drm_kms_helper_hotplug_event(dev);
+   } else {
+   goto unlock;
}
 
+   drm_modeset_lock_all(dev);
+   dm_restore_drm_connector_state(dev, connector);

[Freedreno] [PATCH v4 15/20] drm/gma500: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/gma500/psb_device.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_device.c 
b/drivers/gpu/drm/gma500/psb_device.c
index 3030f18ba022..021a7238508f 100644
--- a/drivers/gpu/drm/gma500/psb_device.c
+++ b/drivers/gpu/drm/gma500/psb_device.c
@@ -8,6 +8,7 @@
 #include 
 
 #include 
+#include 
 
 #include "gma_device.h"
 #include "intel_bios.h"
@@ -169,8 +170,10 @@ static int psb_save_display_registers(struct drm_device 
*dev)
 {
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
struct gma_connector *connector;
struct psb_state *regs = &dev_priv->regs.psb;
+   int ret;
 
/* Display arbitration control + watermarks */
regs->saveDSPARB = PSB_RVDC32(DSPARB);
@@ -183,7 +186,7 @@ static int psb_save_display_registers(struct drm_device 
*dev)
regs->saveCHICKENBIT = PSB_RVDC32(DSPCHICKENBIT);
 
/* Save crtc and output state */
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
if (drm_helper_crtc_in_use(crtc))
dev_priv->ops->save_crtc(crtc);
@@ -193,8 +196,9 @@ static int psb_save_display_registers(struct drm_device 
*dev)
if (connector->save)
connector->save(&connector->base);
 
-   drm_modeset_unlock_all(dev);
-   return 0;
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+   return ret;
 }
 
 /**
@@ -207,8 +211,10 @@ static int psb_restore_display_registers(struct drm_device 
*dev)
 {
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
struct gma_connector *connector;
struct psb_state *regs = &dev_priv->regs.psb;
+   int ret;
 
/* Display arbitration + watermarks */
PSB_WVDC32(regs->saveDSPARB, DSPARB);
@@ -223,7 +229,7 @@ static int psb_restore_display_registers(struct drm_device 
*dev)
/*make sure VGA plane is off. it initializes to on after reset!*/
PSB_WVDC32(0x8000, VGACNTRL);
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
if (drm_helper_crtc_in_use(crtc))
dev_priv->ops->restore_crtc(crtc);
@@ -232,8 +238,8 @@ static int psb_restore_display_registers(struct drm_device 
*dev)
if (connector->restore)
connector->restore(&connector->base);
 
-   drm_modeset_unlock_all(dev);
-   return 0;
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+   return ret;
 }
 
 static int psb_power_down(struct drm_device *dev)
-- 
2.33.0



[Freedreno] [PATCH v4 14/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 3]

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

NOTE:

While the previous two commits were a simple "search and replace", this
time I had to do a bit of refactoring as only one call to
DRM_MODESET_LOCK_ALL_BEGIN() is allowed inside one same function.

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/i915/display/intel_overlay.c | 40 ++--
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c 
b/drivers/gpu/drm/i915/display/intel_overlay.c
index c0ee135e5499..c623738c59c8 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1105,6 +1105,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, 
void *data,
struct drm_crtc *drmmode_crtc;
struct intel_crtc *crtc;
struct drm_i915_gem_object *new_bo;
+   struct drm_modeset_acquire_ctx ctx;
int ret;
 
overlay = dev_priv->overlay;
@@ -1113,24 +1114,24 @@ int intel_overlay_put_image_ioctl(struct drm_device 
*dev, void *data,
return -ENODEV;
}
 
-   if (!(params->flags & I915_OVERLAY_ENABLE)) {
-   drm_modeset_lock_all(dev);
-   ret = intel_overlay_switch_off(overlay);
-   drm_modeset_unlock_all(dev);
+   if (params->flags & I915_OVERLAY_ENABLE) {
 
-   return ret;
-   }
+   drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
+   if (!drmmode_crtc)
+   return -ENOENT;
+   crtc = to_intel_crtc(drmmode_crtc);
 
-   drmmode_crtc = drm_crtc_find(dev, file_priv, params->crtc_id);
-   if (!drmmode_crtc)
-   return -ENOENT;
-   crtc = to_intel_crtc(drmmode_crtc);
+   new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
+   if (!new_bo)
+   return -ENOENT;
+   }
 
-   new_bo = i915_gem_object_lookup(file_priv, params->bo_handle);
-   if (!new_bo)
-   return -ENOENT;
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
-   drm_modeset_lock_all(dev);
+   if (!(params->flags & I915_OVERLAY_ENABLE)) {
+   ret = intel_overlay_switch_off(overlay);
+   goto out_unlock;
+   }
 
if (i915_gem_object_is_tiled(new_bo)) {
drm_dbg_kms(&dev_priv->drm,
@@ -1195,14 +1196,11 @@ int intel_overlay_put_image_ioctl(struct drm_device 
*dev, void *data,
if (ret != 0)
goto out_unlock;
 
-   drm_modeset_unlock_all(dev);
-   i915_gem_object_put(new_bo);
-
-   return 0;
-
 out_unlock:
-   drm_modeset_unlock_all(dev);
-   i915_gem_object_put(new_bo);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+   if (params->flags & I915_OVERLAY_ENABLE)
+   i915_gem_object_put(new_bo);
 
return ret;
 }
-- 
2.33.0



[Freedreno] [PATCH v4 13/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN() [part 2]

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

NOTE:

I separated this change from the rest of modifications to the i915
driver to point out something special explained next.

The only difference between the old drm_modeset_{lock,unlock}_all()
functions and the new DRM_MODESET_LOCK_ALL_{BEGIN,END}() macros is that
the former use a global context stored in dev->mode_config.acquire_ctx
while the latter depend on a user provided one (typically in the stack).

This means that as long as no one accesses the global
dev->mode_config.acquire_ctx context in the block that runs between
lock/BEGIN and unlock/END, the code should be equivalent before and
after my changes.

The only place where I had to take special action to preserve this
condition was here, where I need to modify the old call to
intel_modeset_setup_hw_state() to use the new stack allocated context
structure instead of the global one.

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/i915/display/intel_display.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index c7d9a58f9f98..75b45c01c573 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10992,6 +10992,7 @@ int intel_modeset_init_noirq(struct drm_i915_private 
*i915)
 int intel_modeset_init_nogem(struct drm_i915_private *i915)
 {
struct drm_device *dev = &i915->drm;
+   struct drm_modeset_acquire_ctx ctx;
enum pipe pipe;
struct intel_crtc *crtc;
int ret;
@@ -11043,10 +11044,10 @@ int intel_modeset_init_nogem(struct drm_i915_private 
*i915)
intel_vga_disable(i915);
intel_setup_outputs(i915);
 
-   drm_modeset_lock_all(dev);
-   intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+   intel_modeset_setup_hw_state(dev, &ctx);
intel_acpi_assign_connector_fwnodes(i915);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
for_each_intel_crtc(dev, crtc) {
if (!to_intel_crtc_state(crtc->base.state)->uapi.active)
-- 
2.33.0



[Freedreno] [PATCH v4 12/20] drm/i915: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/i915/display/intel_audio.c| 16 ---
 .../drm/i915/display/intel_display_debugfs.c  | 46 ---
 drivers/gpu/drm/i915/display/intel_overlay.c  |  6 ++-
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  7 ++-
 drivers/gpu/drm/i915/i915_drv.c   | 13 --
 5 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c 
b/drivers/gpu/drm/i915/display/intel_audio.c
index 03e8c05a74f6..37699f13b21f 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -26,6 +26,7 @@
 
 #include 
 #include 
+#include 
 
 #include "i915_drv.h"
 #include "intel_atomic.h"
@@ -1225,7 +1226,8 @@ static int i915_audio_component_bind(struct device 
*i915_kdev,
 {
struct i915_audio_component *acomp = data;
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
-   int i;
+   struct drm_modeset_acquire_ctx ctx;
+   int i, ret;
 
if (drm_WARN_ON(&dev_priv->drm, acomp->base.ops || acomp->base.dev))
return -EEXIST;
@@ -1235,16 +1237,16 @@ static int i915_audio_component_bind(struct device 
*i915_kdev,
 DL_FLAG_STATELESS)))
return -ENOMEM;
 
-   drm_modeset_lock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
acomp->base.ops = &i915_audio_component_ops;
acomp->base.dev = i915_kdev;
BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
acomp->aud_sample_rate[i] = 0;
dev_priv->audio_component = acomp;
-   drm_modeset_unlock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
-   return 0;
+   return ret;
 }
 
 static void i915_audio_component_unbind(struct device *i915_kdev,
@@ -1252,12 +1254,14 @@ static void i915_audio_component_unbind(struct device 
*i915_kdev,
 {
struct i915_audio_component *acomp = data;
struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
-   drm_modeset_lock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
acomp->base.ops = NULL;
acomp->base.dev = NULL;
dev_priv->audio_component = NULL;
-   drm_modeset_unlock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
device_link_remove(hda_kdev, i915_kdev);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index bc5113589f0a..3205ceb0ab70 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 
 #include "i915_debugfs.h"
 #include "intel_de.h"
@@ -1059,11 +1060,13 @@ static int i915_display_info(struct seq_file *m, void 
*unused)
struct intel_crtc *crtc;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
+   struct drm_modeset_acquire_ctx ctx;
intel_wakeref_t wakeref;
+   int ret;
 
wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
seq_printf(m, "CRTC info\n");
seq_printf(m, "-\n");
@@ -1078,20 +1081,21 @@ static int i915_display_info(struct seq_file *m, void 
*unused)
intel_connector_info(m, connector);
drm_connector_list_iter_end(&conn_iter);
 
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
 
-   return 0;
+   return ret;
 }
 
 static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
struct drm_device *dev = &dev_priv->drm;
-   int i;
+   struct drm_modeset_acquire_ctx ctx;
+   int i, ret;
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
seq_printf(m, "PLL refclks: non-SSC: %d kHz, SSC: %d kHz\n",
   dev_priv->dpll.ref_clks.nssc,
@@ -1134,9 +1138,9 @@ static int i915_shared_dplls_info(struct seq_file *m, 
void *unused)
seq_printf(m, " mg_pll_tdc_coldst_bias: 0x%08x\n",
   pll->state.hw_state.mg_pll_tdc_coldst_bias);
}
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
-   return 0;
+   return ret;
 }
 
 static int i915_ipc_status_show(struct seq_file *m, void *data)
@@ -119

[Freedreno] [PATCH v4 11/20] drm/msm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 768012243b44..b89687074890 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "dpu_kms.h"
 #include "dpu_hw_lm.h"
@@ -1172,14 +1173,15 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
struct drm_display_mode *mode;
struct drm_framebuffer *fb;
struct drm_plane_state *state;
+   struct drm_modeset_acquire_ctx ctx;
struct dpu_crtc_state *cstate;
 
-   int i, out_width;
+   int i, out_width, ret;
 
dpu_crtc = s->private;
crtc = &dpu_crtc->base;
 
-   drm_modeset_lock_all(crtc->dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, ret);
cstate = to_dpu_crtc_state(crtc->state);
 
mode = &crtc->state->adjusted_mode;
@@ -1263,9 +1265,9 @@ static int _dpu_debugfs_status_show(struct seq_file *s, 
void *data)
dpu_crtc->vblank_cb_time = ktime_set(0, 0);
}
 
-   drm_modeset_unlock_all(crtc->dev);
+   DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, ret);
 
-   return 0;
+   return ret;
 }
 
 DEFINE_SHOW_ATTRIBUTE(_dpu_debugfs_status);
-- 
2.33.0



[Freedreno] [PATCH v4 10/20] drm/nouveau: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..86e18a844953 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -667,16 +668,18 @@ nv50_audio_component_bind(struct device *kdev, struct 
device *hda_kdev,
struct drm_device *drm_dev = dev_get_drvdata(kdev);
struct nouveau_drm *drm = nouveau_drm(drm_dev);
struct drm_audio_component *acomp = data;
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
return -ENOMEM;
 
-   drm_modeset_lock_all(drm_dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
acomp->ops = &nv50_audio_component_ops;
acomp->dev = kdev;
drm->audio.component = acomp;
-   drm_modeset_unlock_all(drm_dev);
-   return 0;
+   DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
+   return ret;
 }
 
 static void
@@ -686,12 +689,14 @@ nv50_audio_component_unbind(struct device *kdev, struct 
device *hda_kdev,
struct drm_device *drm_dev = dev_get_drvdata(kdev);
struct nouveau_drm *drm = nouveau_drm(drm_dev);
struct drm_audio_component *acomp = data;
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
-   drm_modeset_lock_all(drm_dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret);
drm->audio.component = NULL;
acomp->ops = NULL;
acomp->dev = NULL;
-   drm_modeset_unlock_all(drm_dev);
+   DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret);
 }
 
 static const struct component_ops nv50_audio_component_bind_ops = {
-- 
2.33.0



[Freedreno] [PATCH v4 09/20] drm/omapdrm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/omapdrm/omap_fb.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c 
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 190afc564914..fa7636c13c19 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "omap_dmm_tiler.h"
 #include "omap_drv.h"
@@ -62,15 +63,17 @@ static int omap_framebuffer_dirty(struct drm_framebuffer 
*fb,
  unsigned num_clips)
 {
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
-   drm_modeset_lock_all(fb->dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, 0, ret);
 
drm_for_each_crtc(crtc, fb->dev)
omap_crtc_flush(crtc);
 
-   drm_modeset_unlock_all(fb->dev);
+   DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
 
-   return 0;
+   return ret;
 }
 
 static const struct drm_framebuffer_funcs omap_framebuffer_funcs = {
-- 
2.33.0



[Freedreno] [PATCH v4 07/20] drm/shmobile: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/shmobile/shmob_drm_drv.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c 
b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 80078a9fd7f6..1877feff2e6b 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -156,10 +156,12 @@ static int shmob_drm_pm_suspend(struct device *dev)
 static int shmob_drm_pm_resume(struct device *dev)
 {
struct shmob_drm_device *sdev = dev_get_drvdata(dev);
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
-   drm_modeset_lock_all(sdev->ddev);
+   DRM_MODESET_LOCK_ALL_BEGIN(sdev->ddev, ctx, 0, ret);
shmob_drm_crtc_resume(&sdev->crtc);
-   drm_modeset_unlock_all(sdev->ddev);
+   DRM_MODESET_LOCK_ALL_END(sdev->ddev, ctx, ret);
 
drm_kms_helper_poll_enable(sdev->ddev);
return 0;
-- 
2.33.0



[Freedreno] [PATCH v4 06/20] drm/tegra: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
Reported-by: kernel test robot 
---
 drivers/gpu/drm/tegra/dsi.c  |  6 --
 drivers/gpu/drm/tegra/hdmi.c |  6 --
 drivers/gpu/drm/tegra/sor.c  | 11 +++
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index f46d377f0c30..28050c188c1c 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "dc.h"
 #include "drm.h"
@@ -202,10 +203,11 @@ static int tegra_dsi_show_regs(struct seq_file *s, void 
*data)
struct tegra_dsi *dsi = node->info_ent->data;
struct drm_crtc *crtc = dsi->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+   struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
 
-   drm_modeset_lock_all(drm);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -220,7 +222,7 @@ static int tegra_dsi_show_regs(struct seq_file *s, void 
*data)
}
 
 unlock:
-   drm_modeset_unlock_all(drm);
+   DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
 }
 
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..a62de7f92414 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "hda.h"
 #include "hdmi.h"
@@ -1031,10 +1032,11 @@ static int tegra_hdmi_show_regs(struct seq_file *s, 
void *data)
struct tegra_hdmi *hdmi = node->info_ent->data;
struct drm_crtc *crtc = hdmi->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+   struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
 
-   drm_modeset_lock_all(drm);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1049,7 +1051,7 @@ static int tegra_hdmi_show_regs(struct seq_file *s, void 
*data)
}
 
 unlock:
-   drm_modeset_unlock_all(drm);
+   DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
 }
 
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 0ea320c1092b..3d1c8b3d1358 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "dc.h"
 #include "dp.h"
@@ -1490,10 +1491,11 @@ static int tegra_sor_show_crc(struct seq_file *s, void 
*data)
struct tegra_sor *sor = node->info_ent->data;
struct drm_crtc *crtc = sor->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+   struct drm_modeset_acquire_ctx ctx;
int err = 0;
u32 value;
 
-   drm_modeset_lock_all(drm);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1522,7 +1524,7 @@ static int tegra_sor_show_crc(struct seq_file *s, void 
*data)
seq_printf(s, "%08x\n", value);
 
 unlock:
-   drm_modeset_unlock_all(drm);
+   DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
 }
 
@@ -1652,10 +1654,11 @@ static int tegra_sor_show_regs(struct seq_file *s, void 
*data)
struct tegra_sor *sor = node->info_ent->data;
struct drm_crtc *crtc = sor->output.encoder.crtc;
struct drm_device *drm = node->minor->dev;
+   struct drm_modeset_acquire_ctx ctx;
unsigned int i;
int err = 0;
 
-   drm_modeset_lock_all(drm);
+   DRM_MODESET_LOCK_ALL_BEGIN(drm, ctx, 0, err);
 
if (!crtc || !crtc->state->active) {
err = -EBUSY;
@@ -1670,7 +1673,7 @@ static int tegra_sor_show_regs(struct seq_file *s, void 
*data)
}
 
 unlock:
-   drm_modeset_unlock_all(drm);
+   DRM_MODESET_LOCK_ALL_END(drm, ctx, err);
return err;
 }
 
-- 
2.33.0



[Freedreno] [PATCH v4 08/20] drm/radeon: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/radeon/radeon_device.c | 21 +++--
 drivers/gpu/drm/radeon/radeon_dp_mst.c | 10 ++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index 4f0fbf667431..7e31e5ce7f61 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1559,7 +1560,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool 
suspend,
struct pci_dev *pdev;
struct drm_crtc *crtc;
struct drm_connector *connector;
-   int i, r;
+   struct drm_modeset_acquire_ctx ctx;
+   int i, r, ret;
 
if (dev == NULL || dev->dev_private == NULL) {
return -ENODEV;
@@ -1573,12 +1575,15 @@ int radeon_suspend_kms(struct drm_device *dev, bool 
suspend,
 
drm_kms_helper_poll_disable(dev);
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
/* turn off display hw */
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
}
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+   if (ret)
+   return ret;
 
/* unpin the front buffers and cursors */
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -1663,7 +1668,8 @@ int radeon_resume_kms(struct drm_device *dev, bool 
resume, bool fbcon)
struct radeon_device *rdev = dev->dev_private;
struct pci_dev *pdev = to_pci_dev(dev->dev);
struct drm_crtc *crtc;
-   int r;
+   struct drm_modeset_acquire_ctx ctx;
+   int r, ret;
 
if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
@@ -1741,11 +1747,14 @@ int radeon_resume_kms(struct drm_device *dev, bool 
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(connector, 
&dev->mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+   if (ret)
+   return ret;
}
 
drm_kms_helper_poll_enable(dev);
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c 
b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index ec867fa880a4..3f83ee75b100 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "atom.h"
 #include "ni_reg.h"
@@ -737,11 +738,12 @@ static int radeon_debugfs_mst_info_show(struct seq_file 
*m, void *unused)
struct radeon_device *rdev = (struct radeon_device *)m->private;
struct drm_device *dev = rdev->ddev;
struct drm_connector *connector;
+   struct drm_modeset_acquire_ctx ctx;
struct radeon_connector *radeon_connector;
struct radeon_connector_atom_dig *dig_connector;
-   int i;
+   int i, ret;
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
continue;
@@ -759,8 +761,8 @@ static int radeon_debugfs_mst_info_show(struct seq_file *m, 
void *unused)
   radeon_connector->cur_stream_attribs[i].fe,
   
radeon_connector->cur_stream_attribs[i].slots);
}
-   drm_modeset_unlock_all(dev);
-   return 0;
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+   return ret;
 }
 
 DEFINE_SHOW_ATTRIBUTE(radeon_debugfs_mst_info);
-- 
2.33.0



[Freedreno] [PATCH v4 05/20] drm/vmwgfx: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 11 +++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   | 12 
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index 28af34ab6ed6..7df35c6f1458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -28,6 +28,7 @@
 #include "vmwgfx_drv.h"
 #include "vmwgfx_devcaps.h"
 #include 
+#include 
 #include "vmwgfx_kms.h"
 
 int vmw_getparam_ioctl(struct drm_device *dev, void *data,
@@ -172,6 +173,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
struct drm_framebuffer *fb;
+   struct drm_modeset_acquire_ctx ctx;
struct vmw_framebuffer *vfb;
struct vmw_resource *res;
uint32_t num_clips;
@@ -203,7 +205,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
goto out_no_copy;
}
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
if (!fb) {
@@ -231,7 +233,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 out_no_surface:
drm_framebuffer_put(fb);
 out_no_fb:
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 out_no_copy:
kfree(clips);
 out_clips:
@@ -250,6 +252,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void 
*data,
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
struct drm_framebuffer *fb;
+   struct drm_modeset_acquire_ctx ctx;
struct vmw_framebuffer *vfb;
uint32_t num_clips;
int ret;
@@ -280,7 +283,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void 
*data,
goto out_no_copy;
}
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
if (!fb) {
@@ -303,7 +306,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void 
*data,
 out_no_ttm_lock:
drm_framebuffer_put(fb);
 out_no_fb:
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 out_no_copy:
kfree(clips);
 out_clips:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 74fa41909213..268095cb8c84 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vmwgfx_kms.h"
 
@@ -243,15 +244,17 @@ void vmw_kms_legacy_hotspot_clear(struct vmw_private 
*dev_priv)
struct drm_device *dev = &dev_priv->drm;
struct vmw_display_unit *du;
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_crtc(crtc, dev) {
du = vmw_crtc_to_du(crtc);
 
du->hotspot_x = 0;
du->hotspot_y = 0;
}
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
 void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
@@ -1012,9 +1015,10 @@ static int vmw_framebuffer_bo_dirty(struct 
drm_framebuffer *framebuffer,
struct vmw_framebuffer_bo *vfbd =
vmw_framebuffer_to_vfbd(framebuffer);
struct drm_clip_rect norect;
+   struct drm_modeset_acquire_ctx ctx;
int ret, increment = 1;
 
-   drm_modeset_lock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_BEGIN((&dev_priv->drm), ctx, 0, ret);
 
if (!num_clips) {
num_clips = 1;
@@ -1040,7 +1044,7 @@ static int vmw_framebuffer_bo_dirty(struct 
drm_framebuffer *framebuffer,
 
vmw_cmd_flush(dev_priv, false);
 
-   drm_modeset_unlock_all(&dev_priv->drm);
+   DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret);
 
return ret;
 }
-- 
2.33.0



[Freedreno] [PATCH v4 04/20] drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace driver calls to
drm_modeset_lock_all() with DRM_MODESET_LOCK_ALL_BEGIN() and
DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/drm_client_modeset.c |  5 +++--
 drivers/gpu/drm/drm_crtc_helper.c| 18 --
 drivers/gpu/drm/drm_fb_helper.c  | 10 ++
 drivers/gpu/drm/drm_framebuffer.c|  6 --
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c 
b/drivers/gpu/drm/drm_client_modeset.c
index 5f5184f071ed..43f772543d2a 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1062,9 +1062,10 @@ static int drm_client_modeset_commit_legacy(struct 
drm_client_dev *client)
struct drm_device *dev = client->dev;
struct drm_mode_set *mode_set;
struct drm_plane *plane;
+   struct drm_modeset_acquire_ctx ctx;
int ret = 0;
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_plane(plane, dev) {
if (plane->type != DRM_PLANE_TYPE_PRIMARY)
drm_plane_force_disable(plane);
@@ -1093,7 +1094,7 @@ static int drm_client_modeset_commit_legacy(struct 
drm_client_dev *client)
goto out;
}
 out:
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
return ret;
 }
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index bff917531f33..f3ce073dff79 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -218,11 +218,14 @@ static void __drm_helper_disable_unused_functions(struct 
drm_device *dev)
  */
 void drm_helper_disable_unused_functions(struct drm_device *dev)
 {
+   struct drm_modeset_acquire_ctx ctx;
+   int ret;
+
WARN_ON(drm_drv_uses_atomic_modeset(dev));
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
__drm_helper_disable_unused_functions(dev);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 EXPORT_SYMBOL(drm_helper_disable_unused_functions);
 
@@ -942,12 +945,14 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
struct drm_crtc *crtc;
struct drm_encoder *encoder;
const struct drm_crtc_helper_funcs *crtc_funcs;
+   struct drm_modeset_acquire_ctx ctx;
int encoder_dpms;
bool ret;
+   int err;
 
WARN_ON(drm_drv_uses_atomic_modeset(dev));
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
drm_for_each_crtc(crtc, dev) {
 
if (!crtc->enabled)
@@ -982,7 +987,7 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 
/* disable the unused connectors while restoring the modesetting */
__drm_helper_disable_unused_functions(dev);
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
 
@@ -1002,9 +1007,10 @@ EXPORT_SYMBOL(drm_helper_resume_force_mode);
 int drm_helper_force_disable_all(struct drm_device *dev)
 {
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
int ret = 0;
 
-   drm_modeset_lock_all(dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_for_each_crtc(crtc, dev)
if (crtc->enabled) {
struct drm_mode_set set = {
@@ -1016,7 +1022,7 @@ int drm_helper_force_disable_all(struct drm_device *dev)
goto out;
}
 out:
-   drm_modeset_unlock_all(dev);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
return ret;
 }
 EXPORT_SYMBOL(drm_helper_force_disable_all);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8e7a124d6c5a..3b5661cf6c2b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -940,10 +940,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct 
fb_info *info)
struct drm_fb_helper *fb_helper = info->par;
struct drm_mode_set *modeset;
struct drm_crtc *crtc;
+   struct drm_modeset_acquire_ctx ctx;
u16 *r, *g, *b;
int ret = 0;
 
-   drm_modeset_lock_all(fb_helper->dev);
+   DRM_MODESET_LOCK_ALL_BEGIN(fb_helper->dev, ctx, 0, ret);
drm_client_for_each_modeset(modeset, &fb_helper->client) {
crtc = modeset->crtc;
if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
@@ -970,7 +971,7 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct 
fb_info *info)
goto out;
}
 out:
-   drm_modeset_unlock_all(fb_helper->dev);
+   DRM_MODESET_LOCK_ALL_END(fb_helper->dev, ctx, ret);
 
return ret;
 }
@@ -1441,10 +1442,11 @@ static int pan_display_legacy(struc

[Freedreno] [PATCH v4 03/20] drm/msm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
Reported-by: kernel test robot 
---
 drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c 
b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
index cabe15190ec1..abda52f09b09 100644
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
@@ -5,6 +5,8 @@
 
 #define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
 
+#include 
+
 #include "msm_disp_snapshot.h"
 
 static void msm_disp_state_dump_regs(u32 **reg, u32 aligned_len, void __iomem 
*base_addr)
@@ -99,20 +101,18 @@ static void msm_disp_capture_atomic_state(struct 
msm_disp_state *disp_state)
 {
struct drm_device *ddev;
struct drm_modeset_acquire_ctx ctx;
+   int ret;
 
disp_state->timestamp = ktime_get();
 
ddev = disp_state->drm_dev;
 
-   drm_modeset_acquire_init(&ctx, 0);
-
-   while (drm_modeset_lock_all_ctx(ddev, &ctx) != 0)
-   drm_modeset_backoff(&ctx);
+   DRM_MODESET_LOCK_ALL_BEGIN(ddev, ctx, 0, ret);
 
disp_state->atomic_state = drm_atomic_helper_duplicate_state(ddev,
&ctx);
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
+
+   DRM_MODESET_LOCK_ALL_END(ddev, ctx, ret);
 }
 
 void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
-- 
2.33.0



[Freedreno] [PATCH v4 02/20] drm/i915: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/i915/display/intel_display.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 9cf987ee143d..c7d9a58f9f98 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "display/intel_audio.h"
 #include "display/intel_crt.h"
@@ -11923,22 +11924,13 @@ void intel_display_resume(struct drm_device *dev)
if (state)
state->acquire_ctx = &ctx;
 
-   drm_modeset_acquire_init(&ctx, 0);
-
-   while (1) {
-   ret = drm_modeset_lock_all_ctx(dev, &ctx);
-   if (ret != -EDEADLK)
-   break;
-
-   drm_modeset_backoff(&ctx);
-   }
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
 
-   if (!ret)
-   ret = __intel_display_resume(dev, state, &ctx);
+   ret = __intel_display_resume(dev, state, &ctx);
 
intel_enable_ipc(dev_priv);
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
+
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 
if (ret)
drm_err(&dev_priv->drm,
-- 
2.33.0



[Freedreno] [PATCH v4 01/20] drm: cleanup: drm_modeset_lock_all_ctx() --> DRM_MODESET_LOCK_ALL_BEGIN()

2021-10-13 Thread Fernando Ramos
As requested in Documentation/gpu/todo.rst, replace the boilerplate code
surrounding drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN()
and DRM_MODESET_LOCK_ALL_END()

Signed-off-by: Fernando Ramos 
---
 drivers/gpu/drm/drm_client_modeset.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c 
b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..5f5184f071ed 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -574,6 +574,7 @@ static bool drm_client_firmware_config(struct 
drm_client_dev *client,
int num_connectors_detected = 0;
int num_tiled_conns = 0;
struct drm_modeset_acquire_ctx ctx;
+   int err;
 
if (!drm_drv_uses_atomic_modeset(dev))
return false;
@@ -585,10 +586,7 @@ static bool drm_client_firmware_config(struct 
drm_client_dev *client,
if (!save_enabled)
return false;
 
-   drm_modeset_acquire_init(&ctx, 0);
-
-   while (drm_modeset_lock_all_ctx(dev, &ctx) != 0)
-   drm_modeset_backoff(&ctx);
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, err);
 
memcpy(save_enabled, enabled, count);
mask = GENMASK(count - 1, 0);
@@ -743,8 +741,7 @@ static bool drm_client_firmware_config(struct 
drm_client_dev *client,
ret = false;
}
 
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
+   DRM_MODESET_LOCK_ALL_END(dev, ctx, err);
 
kfree(save_enabled);
return ret;
-- 
2.33.0



[Freedreno] [PATCH v4 00/20] drm: cleanup: Use DRM_MODESET_LOCK_ALL_* helpers

2021-10-13 Thread Fernando Ramos
Hi all,

One of the things in the DRM TODO list ("Documentation/gpu/todo.rst") was to
"use DRM_MODESET_LOCAL_ALL_* helpers instead of boilerplate". That's what this
patch series is about.

You will find two types of changes here:

  - Replacing "drm_modeset_lock_all_ctx()" (and surrounding boilerplate) with
"DRM_MODESET_LOCK_ALL_BEGIN()/END()" in the remaining places (as it has
already been done in previous commits such as b7ea04d2)

  - Replacing "drm_modeset_lock_all()" with "DRM_MODESET_LOCK_ALL_BEGIN()/END()"
in the remaining places (as it has already been done in previous commits
such as 57037094)

Most of the changes are straight forward, except for a few cases in the "amd"
and "i915" drivers where some extra dancing was needed to overcome the
limitation that the DRM_MODESET_LOCK_ALL_BEGIN()/END() macros can only be used
once inside the same function (the reason being that the macro expansion
includes *labels*, and you can not have two labels named the same inside one
function)

Notice that, even after this patch series, some places remain where
"drm_modeset_lock_all()" and "drm_modeset_lock_all_ctx()" are still present,
all inside drm core (which makes sense), except for two (in "amd" and "i915")
which cannot be replaced due to the way they are being used.

Changes in v2:
  - Fix commit message typo
  - Use the value returned by DRM_MODESET_LOCK_ALL_END when possible
  - Split drm/i915 patch into two simpler ones
  - Remove drm_modeset_(un)lock_all()
  - Fix build problems in non-x86 platforms

Changes in v3:
  - Fix in drm/i915 driver to make sure global context is no longer used
  - Fix in drm/amdgpu driver to make sure global context is no longer used
  - Split amdgpu driver to make it easier to understand
  - Remove acquire_ctx from drm_mode_config 
  - Rebase on top of drm-tip
  - WARNING: There is some discussion going on regarding whether the new macros
should be used (or not) in the i915 driver, as a different set of functions
has been proposed in the past (see here:
https://lore.kernel.org/dri-devel/yvrizxceipbug...@intel.com/).
In that case I will need to create a v4 where i915 files are left unchanged.
Let me know your thoughts regarding this.

Changes in v4:
  - Fix missing "Signed-off-by" in one commit
  - No extra comments received in one week
  - Rebase on top of drm-tip

Fernando Ramos (20):
  drm: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/i915: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/msm: cleanup: drm_modeset_lock_all_ctx() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm: cleanup: drm_modeset_lock_all() --> DRM_MODESET_LOCK_ALL_BEGIN()
  drm/vmwgfx: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/tegra: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/shmobile: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/radeon: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/omapdrm: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/nouveau: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/msm: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
  drm/i915: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
  drm/gma500: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN()
  drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 2]
  drm/amd: cleanup: drm_modeset_lock_all() -->
DRM_MODESET_LOCK_ALL_BEGIN() [part 3]
  drm: cleanup: remove drm_modeset_(un)lock_all()
  drm: cleanup: remove acquire_ctx from drm_mode_config

 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 21 +++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 58 ++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 25 ++---
 drivers/gpu/drm/drm_client_modeset.c  | 14 ++-
 drivers/gpu/drm/drm_crtc_helper.c | 18 ++--
 drivers/gpu/drm/drm_fb_helper.c   | 10 +-
 drivers/gpu/drm/drm_framebuffer.c |  6 +-
 drivers/gpu/drm/drm_modeset_lock.c| 94 +--
 drivers/gpu/drm/gma500/psb_device.c   | 18 ++--
 drivers/gpu/drm/i915/display/intel_audio.c| 16 ++--
 drivers/gpu/drm/i915/display/intel_display.c  | 25 ++---
 .../drm/i915/display/intel_display_debugfs.c  | 46 +
 drivers/gpu/drm/i915/display/intel_overlay.c  | 46 -
 drivers/gpu/drm/i915/display/intel_pipe_crc.c |  7 +-
 drivers/gpu/drm/i915/i915_drv.c   | 13 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 10 +-

Re: [Freedreno] [PATCH] drm/msm/dpu: Add CRC support for DPU

2021-10-13 Thread Jessica Zhang

On 10/11/2021 7:01 PM, Dmitry Baryshkov wrote:

On 12/10/2021 02:41, Jessica Zhang wrote:

Add CRC support to DPU, which is currently not supported by
this driver. Only supports CRC for CRTC for now, but will extend support
to other blocks later on.

Tested on Qualcomm RB3 (debian, sdm845)

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 169 +++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  20 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c   |  46 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h   |  13 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h |   9 +-
  5 files changed, 251 insertions(+), 6 deletions(-)

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

index 768012243b44..6ebf989c4e67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0-only
  /*
- * Copyright (c) 2014-2018 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014-2021 The Linux Foundation. All rights reserved.
   * Copyright (C) 2013 Red Hat
   * Author: Rob Clark 
   */
@@ -70,6 +70,121 @@ static struct drm_encoder 
*get_encoder_from_crtc(struct drm_crtc *crtc)

  return NULL;
  }
  +static enum dpu_crtc_crc_source dpu_crtc_parse_crc_source(const 
char *src_name)

+{
+    if (!src_name || !strcmp(src_name, "none"))
+    return DPU_CRTC_CRC_SOURCE_NONE;


Newlines after || please. this would improve readability.

Noted.



+    if (!strcmp(src_name, "auto") || !strcmp(src_name, "lm"))
+    return DPU_CRTC_CRC_SOURCE_LAYER_MIXER;
+
+    return DPU_CRTC_CRC_SOURCE_INVALID;
+}
+
+static bool dpu_crtc_is_valid_crc_source(enum dpu_crtc_crc_source 
source)

+{
+    return (source > DPU_CRTC_CRC_SOURCE_NONE &&
+    source < DPU_CRTC_CRC_SOURCE_MAX);
+}
+
+int dpu_crtc_verify_crc_source(struct drm_crtc *crtc, const char 
*src_name, size_t *values_cnt)

+{
+    enum dpu_crtc_crc_source source = 
dpu_crtc_parse_crc_source(src_name);

+    struct dpu_crtc_state *crtc_state = to_dpu_crtc_state(crtc->state);
+
+    if (source < 0) {


Just use dpu_crtc_is_valid_crtc_source() here.


dpu_crtc_is_valid_crc_source() is not exactly the same as checking if 
the source *name* is valid, as "none" is a valid source name (e.g. would 
pass the `source < 0` check), but 
dpu_crtc_is_valid_crc_source(DPU_CRTC_CRC_SOURCE_NONE) would return 
false as DPU_CRTC_CRC_SOURCE_NONE represents when the CRC source is set 
to nothing. The general purpose of dpu_crc_is_valid_crtc_source() is to 
check that the source specified is able to return a CRC value, so a 
source set to "none" would return false, even though "none" is a 
technically valid source name.


Seems like the root issue is that the name 
"dpu_crtc_is_valid_crc_source" is misleading and it would be better to 
rename the helper method to something clearer. Or replace the 
dpu_crtc_is_valid_crc_source() checks with a check against 
DPU_CRTC_CRC_SOURCE_NONE instead.




+    DRM_DEBUG_DRIVER("Invalid source %s for CRTC%d\n", src_name, 
crtc->index);

+    return -EINVAL;
+    }
+
+    if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER)
+    *values_cnt = crtc_state->num_mixers;
+
+    return 0;
+}
+
+int dpu_crtc_set_crc_source(struct drm_crtc *crtc, const char 
*src_name)

+{
+    enum dpu_crtc_crc_source source = 
dpu_crtc_parse_crc_source(src_name);

+    enum dpu_crtc_crc_source current_source;
+    struct drm_crtc_commit *commit;
+    struct dpu_crtc_state *crtc_state;
+    struct drm_device *drm_dev = crtc->dev;
+    struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+    struct dpu_crtc_mixer *m;
+
+    bool was_enabled;
+    bool enable = false;
+    int i, ret = 0;
+
+    if (source < 0) {
+    DRM_DEBUG_DRIVER("Invalid CRC source %s for CRTC%d\n", 
src_name, crtc->index);

+    return -EINVAL;
+    }
+
+    ret = drm_modeset_lock(&crtc->mutex, NULL);
+
+    if (ret)
+    return ret;
+
+    /* Wait for any pending commits to finish */
+    spin_lock(&crtc->commit_lock);
+    commit = list_first_entry_or_null(&crtc->commit_list, struct 
drm_crtc_commit, commit_entry);

+
+    if (commit)
+    drm_crtc_commit_get(commit);
+    spin_unlock(&crtc->commit_lock);
+
+    if (commit) {
+    ret = 
wait_for_completion_interruptible_timeout(&commit->hw_done, 10 * HZ);

+
+    if (ret)
+    goto cleanup;
+    }


AMD drivers waits for the commit to finish, because it's commit tail 
can modify CRC-related registers. It unique, no other drivers seem to 
do this kind of wait. Why do we need to do it? And if we really need, 
I'd prefer to have this code in some kind of DRM helper function.


Makes sense. I wanted to include it to be safe, but as far as I know 
nothing that happens during a commit will affect reading the CRC for 
this driver. I've also tested without the wait for commit and it doesn't 
seem to affect the CRC read, so I'll remove it.

Re: [Freedreno] [PATCH v2 01/34] component: Introduce struct aggregate_device

2021-10-13 Thread Stephen Boyd
Quoting Daniel Vetter (2021-10-13 05:22:23)
> On Wed, Oct 06, 2021 at 12:37:46PM -0700, Stephen Boyd wrote:
> > Replace 'struct master' with 'struct aggregate_device' and then rename
> > 'master' to 'adev' everywhere in the code. While we're here, put a
> > struct device inside the aggregate device so that we can register it
> > with a bus_type in the next patch.
> >
> > The diff is large but that's because this is mostly a rename, where
> > sometimes 'master' is replaced with 'adev' and other times it is
> > replaced with 'parent' to indicate that the struct device that was being
> > used is actually the parent of the aggregate device and driver.
> >
> > Cc: Daniel Vetter 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Rob Clark 
> > Cc: Russell King 
> > Cc: Saravana Kannan 
> > Signed-off-by: Stephen Boyd 
>
> This adds device model stuff, please cc Greg KH and ask him to review
> this. Maybe also an ack from Rafael would be good whether this makes
> sense.

I will explicitly Cc Greg on these component.c patches.

>
> Once we have that I think we can then go&collect acks/review for all the
> driver changes and get this sorted. Thanks a lot for pushing this forward.

I'll resend the series today. This one has some compilation problems
that I've fixed now.


Re: [Freedreno] [PATCH v2 28/34] power: supply: ab8500: Migrate to aggregate driver

2021-10-13 Thread Sebastian Reichel
Hi,

[+cc Linus Walleij (I guess we should add a MAINTAINERS entry for
the ab8500* power-supply drivers)]

On Wed, Oct 06, 2021 at 12:38:13PM -0700, Stephen Boyd wrote:
> Use an aggregate driver instead of component ops so that we can get
> proper driver probe ordering of the aggregate device with respect to all
> the component devices that make up the aggregate device.
> 
> Cc: Sebastian Reichel 
> Cc: 
> Cc: Daniel Vetter 
> Cc: "Rafael J. Wysocki" 
> Cc: Rob Clark 
> Cc: Russell King 
> Cc: Saravana Kannan 
> Signed-off-by: Stephen Boyd 
> ---

Acked-by: Sebastian Reichel 

-- Sebastian

>  drivers/power/supply/ab8500_charger.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/supply/ab8500_charger.c 
> b/drivers/power/supply/ab8500_charger.c
> index 15eadaf46f14..52d4105e28f2 100644
> --- a/drivers/power/supply/ab8500_charger.c
> +++ b/drivers/power/supply/ab8500_charger.c
> @@ -3312,8 +3312,9 @@ static const struct power_supply_desc 
> ab8500_usb_chg_desc = {
>   .get_property   = ab8500_charger_usb_get_property,
>  };
>  
> -static int ab8500_charger_bind(struct device *dev)
> +static int ab8500_charger_bind(struct aggregate_device *adev)
>  {
> + struct device *dev = adev->parent;
>   struct ab8500_charger *di = dev_get_drvdata(dev);
>   int ch_stat;
>   int ret;
> @@ -3354,8 +3355,9 @@ static int ab8500_charger_bind(struct device *dev)
>   return 0;
>  }
>  
> -static void ab8500_charger_unbind(struct device *dev)
> +static void ab8500_charger_unbind(struct aggregate_device *adev)
>  {
> + struct device *dev = adev->parent;
>   struct ab8500_charger *di = dev_get_drvdata(dev);
>   int ret;
>  
> @@ -3380,9 +3382,13 @@ static void ab8500_charger_unbind(struct device *dev)
>   component_unbind_all(dev, di);
>  }
>  
> -static const struct component_master_ops ab8500_charger_comp_ops = {
> - .bind = ab8500_charger_bind,
> - .unbind = ab8500_charger_unbind,
> +static struct aggregate_driver ab8500_charger_aggregate_driver = {
> + .probe = ab8500_charger_bind,
> + .remove = ab8500_charger_unbind,
> + .driver = {
> + .name = "ab8500_charger_agg",
> + .owner = THIS_MODULE,
> + },
>  };
>  
>  static struct platform_driver *const ab8500_charger_component_drivers[] = {
> @@ -3663,9 +3669,7 @@ static int ab8500_charger_probe(struct platform_device 
> *pdev)
>   }
>  
>  
> - ret = component_master_add_with_match(&pdev->dev,
> -   &ab8500_charger_comp_ops,
> -   match);
> + ret = component_aggregate_register(&pdev->dev, 
> &ab8500_charger_aggregate_driver, match);
>   if (ret) {
>   dev_err(dev, "failed to add component master\n");
>   goto free_notifier;
> @@ -3688,7 +3692,7 @@ static int ab8500_charger_remove(struct platform_device 
> *pdev)
>  {
>   struct ab8500_charger *di = platform_get_drvdata(pdev);
>  
> - component_master_del(&pdev->dev, &ab8500_charger_comp_ops);
> + component_aggregate_unregister(&pdev->dev, 
> &ab8500_charger_aggregate_driver);
>  
>   usb_unregister_notifier(di->usb_phy, &di->nb);
>   usb_put_phy(di->usb_phy);
> -- 
> https://chromeos.dev
> 


signature.asc
Description: PGP signature


[Freedreno] [PATCH] drm: msm: fix building without CONFIG_COMMON_CLK

2021-10-13 Thread Arnd Bergmann
From: Arnd Bergmann 

When CONFIG_COMMON_CLOCK is disabled, the 8996 specific
phy code is left out, which results in a link failure:

ld: drivers/gpu/drm/msm/hdmi/hdmi_phy.o:(.rodata+0x3f0): undefined reference to 
`msm_hdmi_phy_8996_cfg'

This was only exposed after it became possible to build
test the driver without the clock interfaces.

Make COMMON_CLK a hard dependency for compile testing,
and simplify it a little based on that.

Fixes: b3ed524f84f5 ("drm/msm: allow compile_test on !ARM")
Reported-by: Randy Dunlap 
Suggested-by: Geert Uytterhoeven 
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/msm/Kconfig  | 2 +-
 drivers/gpu/drm/msm/Makefile | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index f5107b6ded7b..cb204912e0f4 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -4,8 +4,8 @@ config DRM_MSM
tristate "MSM DRM"
depends on DRM
depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
+   depends on COMMON_CLK
depends on IOMMU_SUPPORT
-   depends on (OF && COMMON_CLK) || COMPILE_TEST
depends on QCOM_OCMEM || QCOM_OCMEM=n
depends on QCOM_LLCC || QCOM_LLCC=n
depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 904535eda0c4..bbee22b54b0c 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -23,8 +23,10 @@ msm-y := \
hdmi/hdmi_i2c.o \
hdmi/hdmi_phy.o \
hdmi/hdmi_phy_8960.o \
+   hdmi/hdmi_phy_8996.o \
hdmi/hdmi_phy_8x60.o \
hdmi/hdmi_phy_8x74.o \
+   hdmi/hdmi_pll_8960.o \
edp/edp.o \
edp/edp_aux.o \
edp/edp_bridge.o \
@@ -37,6 +39,7 @@ msm-y := \
disp/mdp4/mdp4_dtv_encoder.o \
disp/mdp4/mdp4_lcdc_encoder.o \
disp/mdp4/mdp4_lvds_connector.o \
+   disp/mdp4/mdp4_lvds_pll.o \
disp/mdp4/mdp4_irq.o \
disp/mdp4/mdp4_kms.o \
disp/mdp4/mdp4_plane.o \
@@ -117,9 +120,6 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
dp/dp_audio.o
 
 msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
-msm-$(CONFIG_COMMON_CLK) += disp/mdp4/mdp4_lvds_pll.o
-msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_pll_8960.o
-msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_phy_8996.o
 
 msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
 
-- 
2.29.2



Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-10-13 Thread Maxime Ripard
Hi Caleb,

On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> Hi,
> 
> On 30/09/2021 20:49, Amit Pundir wrote:
> > On Thu, 30 Sept 2021 at 04:50, Rob Clark  wrote:
> > > 
> > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz  
> > > wrote:
> > > > 
> > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  
> > > > wrote:
> > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > > > wrote:
> > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  
> > > > > > wrote:
> > > > > > > The best practice to avoid those issues is to register its 
> > > > > > > functions only after
> > > > > > > all its dependencies are live. We also shouldn't wait any longer 
> > > > > > > than we should
> > > > > > > to play nice with the other components that are waiting for us, 
> > > > > > > so in our case
> > > > > > > that would mean moving the DSI device registration to the bridge 
> > > > > > > probe.
> > > > > > > 
> > > > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > > > kirin and msm
> > > > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > > > changes.
> > > > > > > Exynos and kirin seems to be simple enough for a mechanical 
> > > > > > > change (that still
> > > > > > > requires to be tested), but the changes in msm seemed to be far 
> > > > > > > more important
> > > > > > > and I wasn't confortable doing them.
> > > > > > 
> > > > > > 
> > > > > > Hey Maxime,
> > > > > >Sorry for taking so long to get to this, but now that plumbers is
> > > > > > over I've had a chance to check it out on kirin
> > > > > > 
> > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > > 
> > > > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > 
> > > > > > But trying to boot hikey with that, I see the following loop 
> > > > > > indefinitely:
> > > > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > > > regulator
> > > > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > > > regulator
> > > > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > > > regulator
> > > > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > > > regulator
> > > > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > > > regulator
> > > > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > > > regulator
> > > > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > > > > 
> > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > I'll retest and let you know.
> > > > 
> > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > still seeing the same thing.  :(
> > > > 
> > > > Will dig a bit and let you know when I find more.
> > > 
> > > Did you have a chance to test it on anything using drm/msm with DSI
> > > panels?  That would at least confirm that I didn't miss anything in
> > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > 
> > Hi, smoke tested
> > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > regressions in my limited testing so far including video (youtube)
> > playback.
> Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> FBDEV_EMULATION (so we can get a working framebuffer console) which was
> otherwise broken on 5.15.
> 
> However it spits out some warnings during boot: 
> https://p.calebs.dev/gucysowyna.yaml

Thanks for testing. It looks like the runtime_pm ordering between the
msm devices changed a bit with the conversion Rob did.

Rob, do you know what could be going on?

Thanks!
Maxime


signature.asc
Description: PGP signature


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-10-13 Thread Maxime Ripard
Hi John,

On Wed, Sep 29, 2021 at 04:29:42PM -0700, John Stultz wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
> >
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > > The best practice to avoid those issues is to register its functions 
> > > > > only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > > we should
> > > > > to play nice with the other components that are waiting for us, so in 
> > > > > our case
> > > > > that would mean moving the DSI device registration to the bridge 
> > > > > probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > > (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > > important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop 
> > > > indefinitely:
> > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > regulator
> > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > regulator
> > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > regulator
> > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > regulator
> > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > regulator
> > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > regulator
> > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
> 
> Hey Maxime!
>   I chased down the issue. The dsi probe code was still calling
> drm_of_find_panel_or_bridge() in order to succeed.
> 
> I've moved the logic that looks for the bridge into the bridge_init
> and with that it seems to work.
> 
> Feel free (assuming it looks ok) to fold this change into your kirin patch:
>   
> https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=4a35ccc4d7a53f68d6d93da3b47e232a7c75b91d

Thanks for testing, I've picked and squashed your fixup

Maxime


signature.asc
Description: PGP signature


Re: [Freedreno] [PATCH v2 01/34] component: Introduce struct aggregate_device

2021-10-13 Thread Daniel Vetter
On Wed, Oct 06, 2021 at 12:37:46PM -0700, Stephen Boyd wrote:
> Replace 'struct master' with 'struct aggregate_device' and then rename
> 'master' to 'adev' everywhere in the code. While we're here, put a
> struct device inside the aggregate device so that we can register it
> with a bus_type in the next patch.
> 
> The diff is large but that's because this is mostly a rename, where
> sometimes 'master' is replaced with 'adev' and other times it is
> replaced with 'parent' to indicate that the struct device that was being
> used is actually the parent of the aggregate device and driver.
> 
> Cc: Daniel Vetter 
> Cc: "Rafael J. Wysocki" 
> Cc: Rob Clark 
> Cc: Russell King 
> Cc: Saravana Kannan 
> Signed-off-by: Stephen Boyd 

This adds device model stuff, please cc Greg KH and ask him to review
this. Maybe also an ack from Rafael would be good whether this makes
sense.

Once we have that I think we can then go&collect acks/review for all the
driver changes and get this sorted. Thanks a lot for pushing this forward.
-Daniel

> ---
>  drivers/base/component.c  | 250 --
>  include/linux/component.h |   2 +-
>  2 files changed, 134 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 5e79299f6c3f..0a41bbe14981 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -9,6 +9,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -58,18 +59,21 @@ struct component_match {
>   struct component_match_array *compare;
>  };
>  
> -struct master {
> +struct aggregate_device {
>   struct list_head node;
>   bool bound;
>  
>   const struct component_master_ops *ops;
>   struct device *parent;
> + struct device dev;
>   struct component_match *match;
> +
> + int id;
>  };
>  
>  struct component {
>   struct list_head node;
> - struct master *master;
> + struct aggregate_device *adev;
>   bool bound;
>  
>   const struct component_ops *ops;
> @@ -79,7 +83,9 @@ struct component {
>  
>  static DEFINE_MUTEX(component_mutex);
>  static LIST_HEAD(component_list);
> -static LIST_HEAD(masters);
> +static LIST_HEAD(aggregate_devices);
> +
> +static DEFINE_IDA(aggregate_ida);
>  
>  #ifdef CONFIG_DEBUG_FS
>  
> @@ -87,12 +93,12 @@ static struct dentry *component_debugfs_dir;
>  
>  static int component_devices_show(struct seq_file *s, void *data)
>  {
> - struct master *m = s->private;
> + struct aggregate_device *m = s->private;
>   struct component_match *match = m->match;
>   size_t i;
>  
>   mutex_lock(&component_mutex);
> - seq_printf(s, "%-40s %20s\n", "master name", "status");
> + seq_printf(s, "%-40s %20s\n", "aggregate_device name", "status");
>   seq_puts(s, 
> "-\n");
>   seq_printf(s, "%-40s %20s\n\n",
>  dev_name(m->parent), m->bound ? "bound" : "not bound");
> @@ -122,46 +128,46 @@ static int __init component_debug_init(void)
>  
>  core_initcall(component_debug_init);
>  
> -static void component_master_debugfs_add(struct master *m)
> +static void component_master_debugfs_add(struct aggregate_device *m)
>  {
>   debugfs_create_file(dev_name(m->parent), 0444, component_debugfs_dir, m,
>   &component_devices_fops);
>  }
>  
> -static void component_master_debugfs_del(struct master *m)
> +static void component_master_debugfs_del(struct aggregate_device *m)
>  {
>   debugfs_remove(debugfs_lookup(dev_name(m->parent), 
> component_debugfs_dir));
>  }
>  
>  #else
>  
> -static void component_master_debugfs_add(struct master *m)
> +static void component_master_debugfs_add(struct aggregate_device *m)
>  { }
>  
> -static void component_master_debugfs_del(struct master *m)
> +static void component_master_debugfs_del(struct aggregate_device *m)
>  { }
>  
>  #endif
>  
> -static struct master *__master_find(struct device *parent,
> +static struct aggregate_device *__aggregate_find(struct device *parent,
>   const struct component_master_ops *ops)
>  {
> - struct master *m;
> + struct aggregate_device *m;
>  
> - list_for_each_entry(m, &masters, node)
> + list_for_each_entry(m, &aggregate_devices, node)
>   if (m->parent == parent && (!ops || m->ops == ops))
>   return m;
>  
>   return NULL;
>  }
>  
> -static struct component *find_component(struct master *master,
> +static struct component *find_component(struct aggregate_device *adev,
>   struct component_match_array *mc)
>  {
>   struct component *c;
>  
>   list_for_each_entry(c, &component_list, node) {
> - if (c->master && c->master != master)
> + if (c->adev && c->adev != adev)
>   continue;
>  
>   if (mc->compare && mc->compare(c->dev, mc->data))
> @@ -175,101 +181,102 @@ static struct component *

Re: [Freedreno] [PATCH v2 00/34] component: Make into an aggregate bus

2021-10-13 Thread Daniel Vetter
On Thu, Oct 07, 2021 at 04:46:22PM -0400, Stephen Boyd wrote:
> Quoting Andrzej Hajda (2021-10-07 03:16:27)
> > Hi Stephen,
> >
> > On 06.10.2021 21:37, Stephen Boyd wrote:
> > > This series is from discussion we had on reordering the device lists for
> > > drm shutdown paths[1]. I've introduced an 'aggregate' bus that we put
> > > the aggregate device onto and then we probe the aggregate device once
> > > all the components are probed and call component_add(). The probe/remove
> > > hooks are where the bind/unbind calls go, and then a shutdown hook is
> > > added that can be used to shutdown the drm display pipeline at the right
> > > time.
> > >
> > > This works for me on my sc7180 board. I no longer get a warning from i2c
> > > at shutdown that we're trying to make an i2c transaction after the i2c
> > > bus has been shutdown. There's more work to do on the msm drm driver to
> > > extract component device resources like clks, regulators, etc. out of
> > > the component bind function into the driver probe but I wanted to move
> > > everything over now in other component drivers before tackling that
> > > problem.
> >
> >
> > As I understand you have DSI host with i2c-controlled DSI bridge. And
> > there is an issue that bridge is shutdown before msmdrm. Your solution
> > is to 'adjust' device order on pm list.
> > I had similar issue and solved it locally by adding notification from
> > DSI bridge to DSI host that is has to be removed: mipi_dsi_detach, this
> > notification escalates in DSI host to component_del and this allow to
> > react properly.
> >
> > Advantages:
> > - it is local (only involves DSI host and DSI device),
> > - it does not depend on PM internals,
> > - it can be used in other scenarios as well - unbinding DSI device driver
> >
> > Disadvantage:
> > - It is DSI specific (but this is your case), I have advertised some
> > time ago more general approach [1][2].
> >
> > [1]: https://static.sched.com/hosted_files/osseu18/0f/deferred_problem.pdf
> > [2]: https://lwn.net/Articles/625454/
> >
> 
> I think these are all points for or against using the component code in
> general? Maybe you can send patches that you think can solve the problem
> I'm experiencing and we can review them on the list.

Yeah I think this is entirely orthogonal. If you use component, then
component should provide a way to handle this.

If you use something else, like drm_bridge or dsi or whatever, then that
part should provide a solution to stage stuff correctly and handle all the
ordering.

Now there's a bunch of drivers which mix up component with bridge use and
hilarity ensues, but since there's no real effort to fix that I think it's
toally fine to just improve component.c meanwhile.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [Freedreno] mmotm 2021-10-05-19-53 uploaded (drivers/gpu/drm/msm/hdmi/hdmi_phy.o)

2021-10-13 Thread Arnd Bergmann
On Wed, Oct 13, 2021 at 12:54 PM Arnd Bergmann  wrote:
> On Thu, Oct 7, 2021 at 11:51 AM Geert Uytterhoeven  
> wrote:
>
> -msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
> -msm-$(CONFIG_COMMON_CLK) += disp/mdp4/mdp4_lvds_pll.o
> -msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_pll_8960.o
> -msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_phy_8996.o
> +msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o \
> + disp/mdp4/mdp4_lvds_pll.o \
> + hdmi/hdmi_pll_8960.o \
> + hdmi/hdmi_phy_8996.o
>
>  msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o

I fixed my local copy now after noticing that these should not go
after CONFIG_DRM_FBDEV_EMULATION but the top-level option:

@@ -23,8 +23,10 @@ msm-y := \
hdmi/hdmi_i2c.o \
hdmi/hdmi_phy.o \
hdmi/hdmi_phy_8960.o \
+   hdmi/hdmi_phy_8996.o
hdmi/hdmi_phy_8x60.o \
hdmi/hdmi_phy_8x74.o \
+   hdmi/hdmi_pll_8960.o \
edp/edp.o \
edp/edp_aux.o \
edp/edp_bridge.o \
@@ -37,6 +39,7 @@ msm-y := \
disp/mdp4/mdp4_dtv_encoder.o \
disp/mdp4/mdp4_lcdc_encoder.o \
disp/mdp4/mdp4_lvds_connector.o \
+   disp/mdp4/mdp4_lvds_pll.o \
disp/mdp4/mdp4_irq.o \
disp/mdp4/mdp4_kms.o \
disp/mdp4/mdp4_plane.o \

   Arnd


Re: [Freedreno] mmotm 2021-10-05-19-53 uploaded (drivers/gpu/drm/msm/hdmi/hdmi_phy.o)

2021-10-13 Thread Arnd Bergmann
On Thu, Oct 7, 2021 at 11:51 AM Geert Uytterhoeven  wrote:
> On Wed, Oct 6, 2021 at 9:28 AM Christian König  
> wrote:
> > Am 06.10.21 um 09:20 schrieb Stephen Rothwell:
> > > On Tue, 5 Oct 2021 22:48:03 -0700 Randy Dunlap  
> > > wrote:
> > >> on i386:
> > >>
> > >> ld: drivers/gpu/drm/msm/hdmi/hdmi_phy.o:(.rodata+0x3f0): undefined 
> > >> reference to `msm_hdmi_phy_8996_cfg'

I ran into the same thing now as well.
E_TEST) && COMMON_CLK
>
> I'd make that:
>
> -depends on DRM
> +   depends on COMMON_CLK && DRM && IOMMU_SUPPORT
> depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
> -depends on IOMMU_SUPPORT
> -   depends on (OF && COMMON_CLK) || COMPILE_TEST
> +   depends on OF || COMPILE_TEST
>
> to keep a better separation between hard and soft dependencies.
>
> Note that the "depends on OF || COMPILE_TEST" can even be
> deleted, as the dependency on ARCH_QCOM || SOC_IMX5 implies OF.

Looks good to me, I would also drop that last line in this case, and maybe
add this change as building without COMMON_CLK is no longer possible:

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 904535eda0c4..a5d87e03812f 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -116,10 +116,10 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
  dp/dp_power.o \
  dp/dp_audio.o

-msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
-msm-$(CONFIG_COMMON_CLK) += disp/mdp4/mdp4_lvds_pll.o
-msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_pll_8960.o
-msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_phy_8996.o
+msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o \
+ disp/mdp4/mdp4_lvds_pll.o \
+ hdmi/hdmi_pll_8960.o \
+ hdmi/hdmi_phy_8996.o

 msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o

Has anyone submitted a patch already, or should I send the version
that I am using locally now?

Arnd


[Freedreno] [PATCH 2/2] drm/msm: uninitialized variable in msm_gem_import()

2021-10-13 Thread Dan Carpenter
The msm_gem_new_impl() function cleans up after itself so there is no
need to call drm_gem_object_put().  Conceptually, it does not make sense
to call a kref_put() function until after the reference counting has
been initialized which happens immediately after this call in the
drm_gem_(private_)object_init() functions.

In the msm_gem_import() function the "obj" pointer is uninitialized, so
it will lead to a crash.

Fixes: 05b849111c07 ("drm/msm: prime support")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/msm/msm_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 49185d524be3..0e491cd21c53 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1167,7 +1167,7 @@ struct drm_gem_object *msm_gem_new(struct drm_device 
*dev, uint32_t size, uint32
 
ret = msm_gem_new_impl(dev, size, flags, &obj);
if (ret)
-   goto fail;
+   return ERR_PTR(ret);
 
msm_obj = to_msm_bo(obj);
 
@@ -1251,7 +1251,7 @@ struct drm_gem_object *msm_gem_import(struct drm_device 
*dev,
 
ret = msm_gem_new_impl(dev, size, MSM_BO_WC, &obj);
if (ret)
-   goto fail;
+   return ERR_PTR(ret);
 
drm_gem_private_object_init(dev, obj, size);
 
-- 
2.20.1



[Freedreno] [PATCH 1/2] drm/msm: fix potential NULL dereference in cleanup

2021-10-13 Thread Dan Carpenter
The "msm_obj->node" list needs to be initialized earlier so that the
list_del() in msm_gem_free_object() doesn't experience a NULL pointer
dereference.

Fixes: 6ed0897cd800 ("drm/msm: Fix debugfs deadlock")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/msm/msm_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 40a9863f5951..49185d524be3 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1132,6 +1132,7 @@ static int msm_gem_new_impl(struct drm_device *dev,
msm_obj->flags = flags;
msm_obj->madv = MSM_MADV_WILLNEED;
 
+   INIT_LIST_HEAD(&msm_obj->node);
INIT_LIST_HEAD(&msm_obj->vmas);
 
*obj = &msm_obj->base;
-- 
2.20.1