diff mbox series

[04/12] drm/radeon: remove radeon_connector_edid() and stop using edid_blob_ptr

Message ID e4cb7b0c7217511429e69c1c78729f0e864c5b24.1680190534.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm: reduce drm_detect_monitor_audio/drm_detect_hdmi_monitor/edid_blob_ptr usage | expand

Commit Message

Jani Nikula March 30, 2023, 3:39 p.m. UTC 
radeon_connector_edid() copies the EDID from edid_blob_ptr as a side
effect if radeon_connector->edid isn't initialized. However, everywhere
that the returned EDID is used, the EDID should have been set
beforehands.

Only the EDID code and sysfs should look at the EDID property, anyway,
so stop using it.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/radeon/radeon_audio.c      |  5 ++---
 drivers/gpu/drm/radeon/radeon_connectors.c | 15 ---------------
 drivers/gpu/drm/radeon/radeon_mode.h       |  2 --
 3 files changed, 2 insertions(+), 20 deletions(-)

Comments

kernel test robot March 30, 2023, 8:13 p.m. UTC | #1 
Hi Jani,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-exynos/exynos-drm-next linus/master v6.3-rc4 next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-parse-display-info-has_audio-similar-to-is_hdmi/20230330-234201
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/e4cb7b0c7217511429e69c1c78729f0e864c5b24.1680190534.git.jani.nikula%40intel.com
patch subject: [PATCH 04/12] drm/radeon: remove radeon_connector_edid() and stop using edid_blob_ptr
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20230331/202303310350.HnqZZxIV-lkp@intel.com/config)
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/ad73d8b0ebf2124b058e95ef5831caa8f2d34229
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jani-Nikula/drm-edid-parse-display-info-has_audio-similar-to-is_hdmi/20230330-234201
        git checkout ad73d8b0ebf2124b058e95ef5831caa8f2d34229
        # 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 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303310350.HnqZZxIV-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/radeon/radeon_audio.c: In function 'radeon_audio_write_sad_regs':
>> drivers/gpu/drm/radeon/radeon_audio.c:312:37: error: 'radeon_connector' undeclared (first use in this function)
     312 |         sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
         |                                     ^~~~~~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_audio.c:312:37: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/radeon/radeon_audio.c: In function 'radeon_audio_write_speaker_allocation':
   drivers/gpu/drm/radeon/radeon_audio.c:335:52: error: 'radeon_connector' undeclared (first use in this function)
     335 |         sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid, &sadb);
         |                                                    ^~~~~~~~~~~~~~~~


vim +/radeon_connector +312 drivers/gpu/drm/radeon/radeon_audio.c

   301	
   302	static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
   303	{
   304		struct drm_connector *connector = radeon_get_connector_for_encoder(encoder);
   305		struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
   306		struct cea_sad *sads;
   307		int sad_count;
   308	
   309		if (!connector)
   310			return;
   311	
 > 312		sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
   313		if (sad_count < 0)
   314			DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
   315		if (sad_count <= 0)
   316			return;
   317		BUG_ON(!sads);
   318	
   319		if (radeon_encoder->audio && radeon_encoder->audio->write_sad_regs)
   320			radeon_encoder->audio->write_sad_regs(encoder, sads, sad_count);
   321	
   322		kfree(sads);
   323	}
   324
kernel test robot March 30, 2023, 8:13 p.m. UTC | #2 
Hi Jani,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-exynos/exynos-drm-next linus/master v6.3-rc4 next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-parse-display-info-has_audio-similar-to-is_hdmi/20230330-234201
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/e4cb7b0c7217511429e69c1c78729f0e864c5b24.1680190534.git.jani.nikula%40intel.com
patch subject: [PATCH 04/12] drm/radeon: remove radeon_connector_edid() and stop using edid_blob_ptr
config: riscv-randconfig-r042-20230329 (https://download.01.org/0day-ci/archive/20230331/202303310412.bgDHaLy4-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
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
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/ad73d8b0ebf2124b058e95ef5831caa8f2d34229
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jani-Nikula/drm-edid-parse-display-info-has_audio-similar-to-is_hdmi/20230330-234201
        git checkout ad73d8b0ebf2124b058e95ef5831caa8f2d34229
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303310412.bgDHaLy4-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/radeon/radeon_audio.c:312:30: error: use of undeclared identifier 'radeon_connector'
           sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
                                       ^
   drivers/gpu/drm/radeon/radeon_audio.c:335:45: error: use of undeclared identifier 'radeon_connector'
           sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid, &sadb);
                                                      ^
   2 errors generated.


vim +/radeon_connector +312 drivers/gpu/drm/radeon/radeon_audio.c

   301	
   302	static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
   303	{
   304		struct drm_connector *connector = radeon_get_connector_for_encoder(encoder);
   305		struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
   306		struct cea_sad *sads;
   307		int sad_count;
   308	
   309		if (!connector)
   310			return;
   311	
 > 312		sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
   313		if (sad_count < 0)
   314			DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
   315		if (sad_count <= 0)
   316			return;
   317		BUG_ON(!sads);
   318	
   319		if (radeon_encoder->audio && radeon_encoder->audio->write_sad_regs)
   320			radeon_encoder->audio->write_sad_regs(encoder, sads, sad_count);
   321	
   322		kfree(sads);
   323	}
   324
Jani Nikula March 31, 2023, 8:50 a.m. UTC | #3 
On Thu, 30 Mar 2023, Jani Nikula <jani.nikula@intel.com> wrote:
> radeon_connector_edid() copies the EDID from edid_blob_ptr as a side
> effect if radeon_connector->edid isn't initialized. However, everywhere
> that the returned EDID is used, the EDID should have been set
> beforehands.
>
> Only the EDID code and sysfs should look at the EDID property, anyway,
> so stop using it.

Never mind this, I need to fix my config to actually build
this. *facepalm*

>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/radeon/radeon_audio.c      |  5 ++---
>  drivers/gpu/drm/radeon/radeon_connectors.c | 15 ---------------
>  drivers/gpu/drm/radeon/radeon_mode.h       |  2 --
>  3 files changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
> index 947de91e13f6..759b5dfaca15 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -309,7 +309,7 @@ static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
>  	if (!connector)
>  		return;
>  
> -	sad_count = drm_edid_to_sad(radeon_connector_edid(connector), &sads);
> +	sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
>  	if (sad_count < 0)
>  		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
>  	if (sad_count <= 0)
> @@ -332,8 +332,7 @@ static void radeon_audio_write_speaker_allocation(struct drm_encoder *encoder)
>  	if (!connector)
>  		return;
>  
> -	sad_count = drm_edid_to_speaker_allocation(radeon_connector_edid(connector),
> -						   &sadb);
> +	sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid, &sadb);
>  	if (sad_count < 0) {
>  		DRM_DEBUG("Couldn't read Speaker Allocation Data Block: %d\n",
>  			  sad_count);
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index adebf8e9d2bd..99e8f387fe21 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -256,21 +256,6 @@ static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector,
>  	return NULL;
>  }
>  
> -struct edid *radeon_connector_edid(struct drm_connector *connector)
> -{
> -	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> -	struct drm_property_blob *edid_blob = connector->edid_blob_ptr;
> -
> -	if (radeon_connector->edid) {
> -		return radeon_connector->edid;
> -	} else if (edid_blob) {
> -		struct edid *edid = kmemdup(edid_blob->data, edid_blob->length, GFP_KERNEL);
> -		if (edid)
> -			radeon_connector->edid = edid;
> -	}
> -	return radeon_connector->edid;
> -}
> -
>  static void radeon_connector_get_edid(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 3a59d016e8cd..ab71a744d2b2 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -708,8 +708,6 @@ extern u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connecto
>  extern bool radeon_connector_is_dp12_capable(struct drm_connector *connector);
>  extern int radeon_get_monitor_bpc(struct drm_connector *connector);
>  
> -extern struct edid *radeon_connector_edid(struct drm_connector *connector);
> -
>  extern void radeon_connector_hotplug(struct drm_connector *connector);
>  extern int radeon_dp_mode_valid_helper(struct drm_connector *connector,
>  				       struct drm_display_mode *mode);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 947de91e13f6..759b5dfaca15 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -309,7 +309,7 @@  static void radeon_audio_write_sad_regs(struct drm_encoder *encoder)
 	if (!connector)
 		return;
 
-	sad_count = drm_edid_to_sad(radeon_connector_edid(connector), &sads);
+	sad_count = drm_edid_to_sad(radeon_connector->edid, &sads);
 	if (sad_count < 0)
 		DRM_ERROR("Couldn't read SADs: %d\n", sad_count);
 	if (sad_count <= 0)
@@ -332,8 +332,7 @@  static void radeon_audio_write_speaker_allocation(struct drm_encoder *encoder)
 	if (!connector)
 		return;
 
-	sad_count = drm_edid_to_speaker_allocation(radeon_connector_edid(connector),
-						   &sadb);
+	sad_count = drm_edid_to_speaker_allocation(radeon_connector->edid, &sadb);
 	if (sad_count < 0) {
 		DRM_DEBUG("Couldn't read Speaker Allocation Data Block: %d\n",
 			  sad_count);
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index adebf8e9d2bd..99e8f387fe21 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -256,21 +256,6 @@  static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector,
 	return NULL;
 }
 
-struct edid *radeon_connector_edid(struct drm_connector *connector)
-{
-	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
-	struct drm_property_blob *edid_blob = connector->edid_blob_ptr;
-
-	if (radeon_connector->edid) {
-		return radeon_connector->edid;
-	} else if (edid_blob) {
-		struct edid *edid = kmemdup(edid_blob->data, edid_blob->length, GFP_KERNEL);
-		if (edid)
-			radeon_connector->edid = edid;
-	}
-	return radeon_connector->edid;
-}
-
 static void radeon_connector_get_edid(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 3a59d016e8cd..ab71a744d2b2 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -708,8 +708,6 @@  extern u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connecto
 extern bool radeon_connector_is_dp12_capable(struct drm_connector *connector);
 extern int radeon_get_monitor_bpc(struct drm_connector *connector);
 
-extern struct edid *radeon_connector_edid(struct drm_connector *connector);
-
 extern void radeon_connector_hotplug(struct drm_connector *connector);
 extern int radeon_dp_mode_valid_helper(struct drm_connector *connector,
 				       struct drm_display_mode *mode);