diff mbox series

[drm-misc-next,v2,7/9] drm/arm/malidp: crtc: protect device resources after removal

Message ID 20221001163946.534067-8-dakr@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/arm/malidp: use drm managed resources | expand

Commit Message

Danilo Krummrich Oct. 1, 2022, 4:39 p.m. UTC 
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.

However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.

Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().

Signed-off-by: Danilo Krummrich <dakr@redhat.com>
---
 drivers/gpu/drm/arm/malidp_crtc.c | 61 +++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 11 deletions(-)

Comments

kernel test robot Oct. 1, 2022, 7:47 p.m. UTC | #1 
Hi Danilo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 08fb97de03aa2205c6791301bd83a095abc1949c]

url:    https://github.com/intel-lab-lkp/linux/commits/Danilo-Krummrich/drm-arm-malidp-use-drm-managed-resources/20221002-004256
base:   08fb97de03aa2205c6791301bd83a095abc1949c
config: s390-allyesconfig
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/379208cbeb040ca0643b28ff14ca3f6ab99128a4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Danilo-Krummrich/drm-arm-malidp-use-drm-managed-resources/20221002-004256
        git checkout 379208cbeb040ca0643b28ff14ca3f6ab99128a4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/drm/arm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/arm/malidp_crtc.c: In function 'malidp_crtc_mode_valid':
>> drivers/gpu/drm/arm/malidp_crtc.c:31:30: warning: variable 'status' set but not used [-Wunused-but-set-variable]
      31 |         enum drm_mode_status status = MODE_OK;
         |                              ^~~~~~


vim +/status +31 drivers/gpu/drm/arm/malidp_crtc.c

    25	
    26	static enum drm_mode_status malidp_crtc_mode_valid(struct drm_crtc *crtc,
    27							   const struct drm_display_mode *mode)
    28	{
    29		struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
    30		struct malidp_hw_device *hwdev = malidp->dev;
  > 31		enum drm_mode_status status = MODE_OK;
    32		int idx;
    33	
    34		/*
    35		 * check that the hardware can drive the required clock rate,
    36		 * but skip the check if the clock is meant to be disabled (req_rate = 0)
    37		 */
    38		long rate, req_rate = mode->crtc_clock * 1000;
    39	
    40		if (!drm_dev_enter(&malidp->base, &idx))
    41			return MODE_NOCLOCK;
    42	
    43		if (req_rate) {
    44			rate = clk_round_rate(hwdev->pxlclk, req_rate);
    45			if (rate != req_rate) {
    46				DRM_DEBUG_DRIVER("pxlclk doesn't support %ld Hz\n",
    47						 req_rate);
    48				status = MODE_NOCLOCK;
    49				goto out;
    50			}
    51		}
    52	
    53	out:
    54		drm_dev_exit(idx);
    55		return MODE_OK;
    56	}
    57
diff mbox series

Patch

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index dc01c43f6193..e11cda5fdeb7 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -14,6 +14,7 @@ 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
@@ -27,6 +28,8 @@  static enum drm_mode_status malidp_crtc_mode_valid(struct drm_crtc *crtc,
 {
 	struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
 	struct malidp_hw_device *hwdev = malidp->dev;
+	enum drm_mode_status status = MODE_OK;
+	int idx;
 
 	/*
 	 * check that the hardware can drive the required clock rate,
@@ -34,15 +37,21 @@  static enum drm_mode_status malidp_crtc_mode_valid(struct drm_crtc *crtc,
 	 */
 	long rate, req_rate = mode->crtc_clock * 1000;
 
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return MODE_NOCLOCK;
+
 	if (req_rate) {
 		rate = clk_round_rate(hwdev->pxlclk, req_rate);
 		if (rate != req_rate) {
 			DRM_DEBUG_DRIVER("pxlclk doesn't support %ld Hz\n",
 					 req_rate);
-			return MODE_NOCLOCK;
+			status = MODE_NOCLOCK;
+			goto out;
 		}
 	}
 
+out:
+	drm_dev_exit(idx);
 	return MODE_OK;
 }
 
@@ -52,11 +61,15 @@  static void malidp_crtc_atomic_enable(struct drm_crtc *crtc,
 	struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
 	struct malidp_hw_device *hwdev = malidp->dev;
 	struct videomode vm;
-	int err = pm_runtime_get_sync(crtc->dev->dev);
+	int err, idx;
+
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return;
 
+	err = pm_runtime_get_sync(crtc->dev->dev);
 	if (err < 0) {
 		DRM_DEBUG_DRIVER("Failed to enable runtime power management: %d\n", err);
-		return;
+		goto out;
 	}
 
 	drm_display_mode_to_videomode(&crtc->state->adjusted_mode, &vm);
@@ -68,6 +81,9 @@  static void malidp_crtc_atomic_enable(struct drm_crtc *crtc,
 	hwdev->hw->modeset(hwdev, &vm);
 	hwdev->hw->leave_config_mode(hwdev);
 	drm_crtc_vblank_on(crtc);
+
+out:
+	drm_dev_exit(idx);
 }
 
 static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -77,7 +93,10 @@  static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
 									 crtc);
 	struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
 	struct malidp_hw_device *hwdev = malidp->dev;
-	int err;
+	int err, idx;
+
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return;
 
 	/* always disable planes on the CRTC that is being turned off */
 	drm_atomic_helper_disable_planes_on_crtc(old_state, false);
@@ -91,6 +110,8 @@  static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
 	if (err < 0) {
 		DRM_DEBUG_DRIVER("Failed to disable runtime power management: %d\n", err);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static const struct gamma_curve_segment {
@@ -260,7 +281,10 @@  static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 	u32 h_upscale_factor = 0; /* U16.16 */
 	u32 v_upscale_factor = 0; /* U16.16 */
 	u8 scaling = cs->scaled_planes_mask;
-	int ret;
+	int idx, ret;
+
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return -ENODEV;
 
 	if (!scaling) {
 		s->scale_enable = false;
@@ -268,8 +292,10 @@  static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 	}
 
 	/* The scaling engine can only handle one plane at a time. */
-	if (scaling & (scaling - 1))
-		return -EINVAL;
+	if (scaling & (scaling - 1)) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
 		struct malidp_plane *mp = to_malidp_plane(plane);
@@ -331,10 +357,10 @@  static int malidp_crtc_atomic_check_scaling(struct drm_crtc *crtc,
 
 mclk_calc:
 	drm_display_mode_to_videomode(&state->adjusted_mode, &vm);
-	ret = hwdev->hw->se_calc_mclk(hwdev, s, &vm);
-	if (ret < 0)
-		return -EINVAL;
-	return 0;
+	ret = hwdev->hw->se_calc_mclk(hwdev, s, &vm) < 0 ? -EINVAL : 0;
+out:
+	drm_dev_exit(idx);
+	return ret;
 }
 
 static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
@@ -498,9 +524,16 @@  static int malidp_crtc_enable_vblank(struct drm_crtc *crtc)
 {
 	struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
 	struct malidp_hw_device *hwdev = malidp->dev;
+	int idx;
+
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return -ENODEV;
 
 	malidp_hw_enable_irq(hwdev, MALIDP_DE_BLOCK,
 			     hwdev->hw->map.de_irq_map.vsync_irq);
+
+	drm_dev_exit(idx);
+
 	return 0;
 }
 
@@ -508,9 +541,15 @@  static void malidp_crtc_disable_vblank(struct drm_crtc *crtc)
 {
 	struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
 	struct malidp_hw_device *hwdev = malidp->dev;
+	int idx;
+
+	if (!drm_dev_enter(&malidp->base, &idx))
+		return;
 
 	malidp_hw_disable_irq(hwdev, MALIDP_DE_BLOCK,
 			      hwdev->hw->map.de_irq_map.vsync_irq);
+
+	drm_dev_exit(idx);
 }
 
 static const struct drm_crtc_funcs malidp_crtc_funcs = {