Re: [PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_{b,c}*.c
Hi, Please, don't apply this patch. I have to review it,. BR On Thu, Dec 30, 2021 at 10:31:45PM +0800, kernel test robot wrote: > Hi Claudio, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on drm/drm-next] > [also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip > v5.16-rc7 next-20211224] > [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] > > url: > https://github.com/0day-ci/linux/commits/Claudio-Suarez/drm-get-rid-of-DRM_DEBUG_-log-calls-in-drm-core-files-drm_-b-c-c/20211230-185446 > base: git://anongit.freedesktop.org/drm/drm drm-next > config: hexagon-randconfig-r041-20211230 > (https://download.01.org/0day-ci/archive/20211230/202112302236.ikd2et5w-...@intel.com/config) > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project > cd284b7ac0615afc6e0f1a30da2777e361de27a3) > 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/0day-ci/linux/commit/9cfa12f89e858cd6d2eb5eb17c6db7ab689343e3 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review > Claudio-Suarez/drm-get-rid-of-DRM_DEBUG_-log-calls-in-drm-core-files-drm_-b-c-c/20211230-185446 > git checkout 9cfa12f89e858cd6d2eb5eb17c6db7ab689343e3 > # save the config file to linux build tree > mkdir build_dir > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 > O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > >drivers/gpu/drm/drm_bufs.c:174:14: error: incompatible pointer types > passing 'struct drm_device *' to parameter of type 'const struct device *' > [-Werror,-Wincompatible-pointer-types] >drm_dev_dbg(dev, "offset = 0x%08llx, size = 0x%08lx, type = %d\n", >^~~ >include/drm/drm_print.h:337:39: note: passing argument to parameter 'dev' > here >void drm_dev_dbg(const struct device *dev, enum drm_debug_category > category, > ^ > >> drivers/gpu/drm/drm_bufs.c:174:19: warning: incompatible pointer to > >> integer conversion passing 'char[46]' to parameter of type 'enum > >> drm_debug_category' [-Wint-conversion] >drm_dev_dbg(dev, "offset = 0x%08llx, size = 0x%08lx, type = %d\n", > ^~~~ >include/drm/drm_print.h:337:68: note: passing argument to parameter > 'category' here >void drm_dev_dbg(const struct device *dev, enum drm_debug_category > category, > ^ > >> drivers/gpu/drm/drm_bufs.c:175:7: warning: incompatible integer to pointer > >> conversion passing 'unsigned long long' to parameter of type 'const char > >> *' [-Wint-conversion] >(unsigned long long)map->offset, map->size, map->type); >^~~ >include/drm/drm_print.h:338:16: note: passing argument to parameter > 'format' here > const char *format, ...); > ^ >drivers/gpu/drm/drm_bufs.c:208:17: error: incompatible pointer types > passing 'struct drm_device *' to parameter of type 'const struct device *' > [-Werror,-Wincompatible-pointer-types] >drm_dev_dbg(dev, >^~~ >include/drm/drm_print.h:337:39: note: passing argument to parameter 'dev' > here >void drm_dev_dbg(const struct device *dev, enum drm_debug_category > category, > ^ >drivers/gpu/drm/drm_bufs.c:209:10: warning: incompatible pointer to > integer conversion passing 'char[62]' to parameter of type 'enum > drm_debug_category' [-Wint-conversion] >"Matching maps of type %d with > mismatched sizes, (%ld vs %ld)\n", > > ^~~~ >include/
[PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_{b,c}*.c
DRM_DEBUG_* and DRM_* log calls are deprecated.
Change them to drm_dbg_* / drm_{err,info,...} calls in drm core
files.
To avoid making a very big patch, this change is split in
smaller patches. This one includes drm_{b,c}*.c
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_blend.c | 6 +-
drivers/gpu/drm/drm_bridge.c | 6 +-
drivers/gpu/drm/drm_bufs.c | 116 +--
drivers/gpu/drm/drm_client_modeset.c | 109 +
drivers/gpu/drm/drm_color_mgmt.c | 6 +-
drivers/gpu/drm/drm_connector.c | 37 +
drivers/gpu/drm/drm_context.c| 18 ++---
drivers/gpu/drm/drm_crtc.c | 40 -
drivers/gpu/drm/drm_crtc_helper.c| 61 +++---
9 files changed, 211 insertions(+), 188 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index ec37cbfabb50..4a988815f998 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -450,7 +450,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct
drm_crtc *crtc,
int i, n = 0;
int ret = 0;
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+ drm_dbg_atomic(dev, "[CRTC:%d:%s] calculating normalized zpos values\n",
crtc->base.id, crtc->name);
states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
@@ -469,7 +469,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct
drm_crtc *crtc,
goto done;
}
states[n++] = plane_state;
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
+ drm_dbg_atomic(dev, "[PLANE:%d:%s] processing zpos value %d\n",
plane->base.id, plane->name,
plane_state->zpos);
}
@@ -480,7 +480,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct
drm_crtc *crtc,
plane = states[i]->plane;
states[i]->normalized_zpos = i;
- DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+ drm_dbg_atomic(dev, "[PLANE:%d:%s] normalized zpos value %d\n",
plane->base.id, plane->name, i);
}
crtc_state->zpos_changed = true;
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c96847fc0ebc..b108377b4b40 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -288,10 +288,12 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct
drm_bridge *bridge,
list_del(&bridge->chain_node);
#ifdef CONFIG_OF
- DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
+ drm_err(encoder->dev,
+ "failed to attach bridge %pOF to encoder %s: %d\n",
bridge->of_node, encoder->name, ret);
#else
- DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
+ drm_err(encoder->dev,
+ "failed to attach bridge to encoder %s: %d\n",
encoder->name, ret);
#endif
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index fcca21e8efac..dd8e100e120c 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -171,8 +171,8 @@ static int drm_addmap_core(struct drm_device *dev,
resource_size_t offset,
kfree(map);
return -EINVAL;
}
- DRM_DEBUG("offset = 0x%08llx, size = 0x%08lx, type = %d\n",
- (unsigned long long)map->offset, map->size, map->type);
+ drm_dev_dbg(dev, "offset = 0x%08llx, size = 0x%08lx, type = %d\n",
+ (unsigned long long)map->offset, map->size, map->type);
/* page-align _DRM_SHM maps. They are allocated here so there is no
security
* hole created by that and it works around various broken drivers that
use
@@ -205,10 +205,9 @@ static int drm_addmap_core(struct drm_device *dev,
resource_size_t offset,
list = drm_find_matching_map(dev, map);
if (list != NULL) {
if (list->map->size != map->size) {
- DRM_DEBUG("Matching maps of type %d with "
- "mismatched sizes, (%ld vs %ld)\n",
- map->type, map->size,
- list->map->size);
+ drm_dev_dbg(dev,
+ "Matching maps of type %d with
mismatched sizes, (%ld vs %ld)\n",
+ map->type, map->size,
list->map->size);
Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997
On Mon, Dec 20, 2021 at 06:11:31PM +0100, Daniel Vetter wrote:
> On Mon, Dec 20, 2021 at 10:18:38AM +0100, Daniel Vetter wrote:
> > On Thu, Dec 02, 2021 at 10:51:12AM +0100, Claudio Suarez wrote:
> > > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > > calls in drm core, files drm_a*.c") fails when the drm_device
> > > cannot be found in the parameter plane_state->crtc.
> > > Fix it using plane_state->plane.
> > >
> > > Reported-by: kernel test robot
> > > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core,
> > > files drm_a*.c")
>
> My scrip complained about the fixes line, so I fixed it. I guess you've
> used the sha1 from your tree, not from upstream?
Yes, my bad, sorry. Thanks for the advice.
> Please always use
> upstream sha1 when referencing commits.
>
> Anyway patches are now pushed.
Thank you!
Best regards.
Claudio Suarez.
Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997
The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
calls in drm core, files drm_a*.c") fails when the drm_device
cannot be found in the parameter plane_state->crtc.
Fix it using plane_state->plane.
Reported-by: kernel test robot
Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files
drm_a*.c")
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_atomic_helper.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index aef2fbd676e5..a7a05e1e26bb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -828,8 +828,8 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
}
if (!crtc_state->enable && !can_update_disabled) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Cannot update plane of a disabled CRTC.\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}
@@ -839,8 +839,8 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Invalid scaling of plane\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Invalid scaling of plane\n");
drm_rect_debug_print("src: ", &plane_state->src, true);
drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
@@ -864,8 +864,8 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
return 0;
if (!can_position && !drm_rect_equals(dst, &clip)) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Plane must cover entire CRTC\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Plane must cover entire CRTC\n");
drm_rect_debug_print("dst: ", dst, false);
drm_rect_debug_print("clip: ", &clip, false);
return -EINVAL;
--
2.33.0
[PATCH] mock a drm_plane in igt_check_plane_state to make the test more robust
igt_check_plane_state test crashes in drm_atomic_helper_check_plane_state
when trying to de-reference drm_plane_state->plane->dev
due to the lack of a struct drm_plane in the mock struct drm_plane_state.
Since drm_plane_state always should contain a plane, the mock also
needs a plane to be the test more robust and realistic. Add it.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/selftests/test-drm_plane_helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c
b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
index 0a9553f51796..ceebeede55ea 100644
--- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
@@ -87,11 +87,15 @@ int igt_check_plane_state(void *ignored)
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
},
};
+ struct drm_plane plane = {
+ .dev = NULL
+ };
struct drm_framebuffer fb = {
.width = 2048,
.height = 2048
};
struct drm_plane_state plane_state = {
+ .plane = &plane,
.crtc = ZERO_SIZE_PTR,
.fb = &fb,
.rotation = DRM_MODE_ROTATE_0
--
2.33.0
Re: [PATCH] drm: fix error found in some cases after the patch d1af5cd86997
On Tue, Nov 30, 2021 at 09:38:11AM +0100, Daniel Vetter wrote:
> On Mon, Nov 29, 2021 at 08:27:45PM +0100, Claudio Suarez wrote:
> > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > calls in drm core, files drm_a*.c") fails when the drm_device
> > cannot be found in the parameter plane_state. Fix it.
> >
> > Reported-by: kernel test robot
> > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core,
> > files drm_a*.c")
> > Signed-off-by: Claudio Suarez
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 12 +---
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index aef2fbd676e5..8bd4472d7949 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -312,7 +312,7 @@ update_connector_routing(struct drm_atomic_state *state,
> >
> > if (!new_connector_state->crtc) {
> > drm_dbg_atomic(connector->dev, "Disabling [CONNECTOR:%d:%s]\n",
> > - connector->base.id, connector->name);
> > + connector->base.id, connector->name);
>
> Can you pls split this checkpatch fix out?
Of course.
>
> >
> > set_best_encoder(state, new_connector_state, NULL);
> >
> > @@ -805,6 +805,7 @@ int drm_atomic_helper_check_plane_state(struct
> > drm_plane_state *plane_state,
> > bool can_update_disabled)
> > {
> > struct drm_framebuffer *fb = plane_state->fb;
> > + struct drm_device *dev = plane_state->plane ? plane_state->plane->dev :
> > NULL;
>
> For real drivers plane_state->plane really should never be NULL, and
> looking at the test report we blow up in an selftest. I think the right
> fix here is to make the selftest more robust and also mock a drm_plane
> (with a NULL plane->dev pointer, that should be fine).
>
> Can you pls spin that and test it with the selftests enabled in the
> config?
You are right. I made this change in the test and the error was gone.
Output before and after follow.
The code in the kernel should be fixed though. Currently plane_state->crtc
is used. It should be plane_state->plane
I going to send two patches: one for the test and a v2 for the drm code.
Output before changing the test:
[ 38.161315][T1] drm_mm: bottom-up fragmented insert of 1 and 2
insertions took 7047892 and 14424064 nsecs
[ 38.191061][T1] drm_mm: top-down fragmented insert of 1 and 2
insertions took 7114502 and 14582794 nsecs
[ 40.260837][T1] BUG: kernel NULL pointer dereference, address:
[ 40.261381][T1] #PF: supervisor read access in kernel mode
[ 40.261845][T1] #PF: error_code(0x) - not-present page
[ 40.262346][T1] *pde =
[ 40.262692][T1] Oops: [#1] PREEMPT SMP
[ 40.263061][T1] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G S W
5.16.0-rc2-2-g58f3ee4dc1e9-dirty #1
[ 40.264036][T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.14.0-2 04/01/2014
[ 40.264036][T1] EIP: drm_atomic_helper_check_plane_state+0x15e/0x280
[ 40.264036][T1] Code: 8b 45 d4 50 89 f0 e8 c1 0c 03 00 5a 80 7b 6c 00 74
6a 80 7d cc 00 75 64 8b 45 e4 39 43 5c 75 08 8b 45 ec 39 43 64 74 44 8b 03 <8b>
00 85 c0 74 03 8b 40 08 68 33 c2 70 c2 6a 04 50 e8 4c 14 04 00
[ 40.264036][T1] EAX: EBX: c036dce8 ECX: 0800 EDX: 0001
Output after:
[ 38.230609][T1] drm_mm: bottom-up fragmented insert of 1 and 2
insertions took 7235202 and 14675224 nsecs
[ 38.259900][T1] drm_mm: top-down fragmented insert of 1 and 2
insertions took 7078062 and 14642524 nsecs
[ 40.347207][T1] random: get_random_bytes called from
igt_dp_mst_sideband_msg_req_decode+0x25f/0x300 with crng_init=0
[ 40.347248][T1] ACPI: bus type drm_connector registered
[ 40.350980][T1] [drm] Initialized vgem 1.0.0 20120112 for vgem on minor
0
[ 40.351670][T2] random: get_random_u32 called from
copy_process+0x21c/0x1a00 with crng_init=0
[ 40.353310][T1] [drm] Initialized vkms 1.0.0 20180514 for vkms on minor
1
BR
Claudio Suarez
>
> Thanks, Daniel
>
> > struct drm_rect *src = &plane_state->src;
> > struct drm_rect *dst = &plane_state->dst;
> > unsigned int rotation = plane_state->rotation;
> > @@ -828,8 +829,7 @@ int drm_atomic_helper_che
[PATCH] drm: fix error found in some cases after the patch d1af5cd86997
The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
calls in drm core, files drm_a*.c") fails when the drm_device
cannot be found in the parameter plane_state. Fix it.
Reported-by: kernel test robot
Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files
drm_a*.c")
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_atomic_helper.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index aef2fbd676e5..8bd4472d7949 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -312,7 +312,7 @@ update_connector_routing(struct drm_atomic_state *state,
if (!new_connector_state->crtc) {
drm_dbg_atomic(connector->dev, "Disabling [CONNECTOR:%d:%s]\n",
- connector->base.id, connector->name);
+ connector->base.id, connector->name);
set_best_encoder(state, new_connector_state, NULL);
@@ -805,6 +805,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
bool can_update_disabled)
{
struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_device *dev = plane_state->plane ? plane_state->plane->dev :
NULL;
struct drm_rect *src = &plane_state->src;
struct drm_rect *dst = &plane_state->dst;
unsigned int rotation = plane_state->rotation;
@@ -828,8 +829,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
}
if (!crtc_state->enable && !can_update_disabled) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Cannot update plane of a disabled CRTC.\n");
+ drm_dbg_kms(dev, "Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}
@@ -839,8 +839,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Invalid scaling of plane\n");
+ drm_dbg_kms(dev, "Invalid scaling of plane\n");
drm_rect_debug_print("src: ", &plane_state->src, true);
drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
@@ -864,8 +863,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
return 0;
if (!can_position && !drm_rect_equals(dst, &clip)) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Plane must cover entire CRTC\n");
+ drm_dbg_kms(dev, "Plane must cover entire CRTC\n");
drm_rect_debug_print("dst: ", dst, false);
drm_rect_debug_print("clip: ", &clip, false);
return -EINVAL;
--
2.33.0
Re: [PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c
On Fri, Nov 26, 2021 at 04:45:46PM +0100, Daniel Vetter wrote:
> On Fri, Nov 26, 2021 at 11:49:49AM +0100, Claudio Suarez wrote:
> > DRM_DEBUG_* and DRM_* log calls are deprecated.
> > Change them to drm_dbg_* / drm_{err,info,...} calls in drm core
> > files.
> >
> > To avoid making a very big patch, this change is split in
> > smaller patches. This one includes drm_a*.c
> >
> > Signed-off-by: Claudio Suarez
>
> lgtm, merged into drm-misc-next, thanks for the patch.
> -Daniel
Thanks to you, Daniel.
Best regards
Claudio Suarez
[PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c
DRM_DEBUG_* and DRM_* log calls are deprecated.
Change them to drm_dbg_* / drm_{err,info,...} calls in drm core
files.
To avoid making a very big patch, this change is split in
smaller patches. This one includes drm_a*.c
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_atomic.c| 180 +++--
drivers/gpu/drm/drm_atomic_helper.c | 243 +++-
drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
drivers/gpu/drm/drm_auth.c | 12 +-
4 files changed, 242 insertions(+), 195 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ff1416cd609a..21174efd91be 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -74,7 +74,7 @@ int drm_crtc_commit_wait(struct drm_crtc_commit *commit)
ret = wait_for_completion_timeout(&commit->hw_done, timeout);
if (!ret) {
- DRM_ERROR("hw_done timed out\n");
+ drm_err(commit->crtc->dev, "hw_done timed out\n");
return -ETIMEDOUT;
}
@@ -84,7 +84,7 @@ int drm_crtc_commit_wait(struct drm_crtc_commit *commit)
*/
ret = wait_for_completion_timeout(&commit->flip_done, timeout);
if (!ret) {
- DRM_ERROR("flip_done timed out\n");
+ drm_err(commit->crtc->dev, "flip_done timed out\n");
return -ETIMEDOUT;
}
@@ -140,7 +140,7 @@ drm_atomic_state_init(struct drm_device *dev, struct
drm_atomic_state *state)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
+ drm_dbg_atomic(dev, "Allocated atomic state %p\n", state);
return 0;
fail:
@@ -191,7 +191,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state
*state)
struct drm_mode_config *config = &dev->mode_config;
int i;
- DRM_DEBUG_ATOMIC("Clearing atomic state %p\n", state);
+ drm_dbg_atomic(dev, "Clearing atomic state %p\n", state);
for (i = 0; i < state->num_connector; i++) {
struct drm_connector *connector = state->connectors[i].ptr;
@@ -301,7 +301,7 @@ void __drm_atomic_state_free(struct kref *ref)
drm_atomic_state_clear(state);
- DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
+ drm_dbg_atomic(state->dev, "Freeing atomic state %p\n", state);
if (config->funcs->atomic_state_free) {
config->funcs->atomic_state_free(state);
@@ -358,8 +358,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
state->crtcs[index].ptr = crtc;
crtc_state->state = state;
- DRM_DEBUG_ATOMIC("Added [CRTC:%d:%s] %p state to %p\n",
-crtc->base.id, crtc->name, crtc_state, state);
+ drm_dbg_atomic(state->dev, "Added [CRTC:%d:%s] %p state to %p\n",
+ crtc->base.id, crtc->name, crtc_state, state);
return crtc_state;
}
@@ -379,8 +379,9 @@ static int drm_atomic_crtc_check(const struct
drm_crtc_state *old_crtc_state,
*/
if (new_crtc_state->active && !new_crtc_state->enable) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] active without enabled\n",
-crtc->base.id, crtc->name);
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] active without enabled\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -390,15 +391,17 @@ static int drm_atomic_crtc_check(const struct
drm_crtc_state *old_crtc_state,
*/
if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
WARN_ON(new_crtc_state->enable && !new_crtc_state->mode_blob)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled without mode blob\n",
-crtc->base.id, crtc->name);
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] enabled without mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) &&
WARN_ON(!new_crtc_state->enable && new_crtc_state->mode_blob)) {
- DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled with mode blob\n",
-crtc->base.id, crtc->name);
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] disabled with mode blob\n",
+ crtc->base.id, crtc->name);
return -EINVAL;
}
@@ -414,8 +417,9 @@ s
[PATCH v2] drm: change logs to print connectors in the form [CONNECTOR:id:name]
The preferred way to log connectors is [CONNECTOR:id:name]. Change it in
drm core programs. Also replace obsolete log calls (like DRM_DEBUG_*)
to the new ones (like drm_dbg_*)
Suggested-by: Ville Syrjälä
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_client_modeset.c | 66 +---
drivers/gpu/drm/drm_connector.c | 24 +-
drivers/gpu/drm/drm_edid.c | 45 +++
drivers/gpu/drm/drm_edid_load.c | 21 +
drivers/gpu/drm/drm_mode_config.c| 3 +-
5 files changed, 95 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..8df53b6e7687 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -240,8 +240,9 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
for (i = 0; i < connector_count; i++) {
connector = connectors[i];
enabled[i] = drm_connector_enabled(connector, true);
- DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
- connector->display_info.non_desktop ? "non
desktop" : enabled[i] ? "yes" : "no");
+ drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] enabled? %s\n",
+ connector->base.id, connector->name,
+ connector->display_info.non_desktop ? "non desktop"
: enabled[i] ? "yes" : "no");
any_enabled |= enabled[i];
}
@@ -350,8 +351,9 @@ static int drm_client_get_tile_offsets(struct drm_connector
**connectors,
continue;
if (!modes[i] && (h_idx || v_idx)) {
- DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,
- connector->base.id);
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s]: no modes tiled %d\n",
+ connector->base.id, connector->name, i);
continue;
}
if (connector->tile_h_loc < h_idx)
@@ -419,14 +421,17 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
drm_client_get_tile_offsets(connectors,
connector_count, modes, offsets, i,
connector->tile_h_loc,
connector->tile_v_loc);
}
- DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
- connector->base.id);
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s]: looking for cmdline mode\n",
+ connector->base.id, connector->name);
/* got for command line mode first */
modes[i] = drm_connector_pick_cmdline_mode(connector);
if (!modes[i]) {
- DRM_DEBUG_KMS("looking for preferred mode on connector
%d %d\n",
- connector->base.id, connector->tile_group
? connector->tile_group->id : 0);
+ drm_dbg_kms(connector->dev,
+ "[CONNECTOR:%d:%s]: looking for preferred
mode %d\n",
+ connector->base.id, connector->name,
+ connector->tile_group ?
connector->tile_group->id : 0);
modes[i] = drm_connector_has_preferred_mode(connector,
width, height);
}
/* No preferred modes, pick one off the list */
@@ -448,8 +453,8 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
(connector->tile_h_loc == 0 &&
connector->tile_v_loc == 0 &&
!drm_connector_get_tiled_mode(connector))) {
- DRM_DEBUG_KMS("Falling back to non tiled mode
on Connector %d\n",
- connector->base.id);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]: falling back
to non tiled mode\n",
+ connector->base.id,
connector->name);
modes[i] =
drm_connector_fallback_non_tiled_mode(connector);
} else {
modes[i] =
drm_connector_get_tiled_mode(connector);
@@ -617,15 +622,17 @@ static bool drm_client_firmware_config(struct
drm_client_dev *client,
num_connectors_detected++;
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 10:17:58PM +0200, Jani Nikula wrote:
> On Mon, 15 Nov 2021, Claudio Suarez wrote:
> > On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
> >> On Sun, 14 Nov 2021, Claudio Suarez wrote:
> >> > On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
> >> >> Hi Claudio,
> >> >>
> >> >> On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
> >> >> > The prefered way to log connectors is [CONNECTOR:id:name]. Change it
> >> >> > in
> >> >> > drm core programs.
> >> >> >
> >> >> > Suggested-by: Ville Syrjälä
> >> >> > Signed-off-by: Claudio Suarez
> >> >>
> >> >> While touching all these logging calls, could you convernt them to the
> >> >> newer drm_dbg_kms variants?
> >> >> DRM_DEBUG_* are all deprecated.
> >> >>
> >> >
> >> > Yes, I can, but it is recommended to do it in a different patch:
> >> >
> >> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes
> >> >
> >> > C&P:
> >> > "Separate your changes
> >> > Separate each logical change into a separate patch.
> >> > For example, if your changes include both bug fixes and performance
> >> > enhancements..."
> >> >
> >> >
> >> > I will study and send a new separate patch with your suggestion.
> >>
> >> I feel these logging changes are the types of changes where I'd err on
> >> the side of fewer patches than strictly following the above guidelines.
> >
> > To size the problem:
> > - there are about 3434 references to DRM_DEBUG_* in all the drm code, all
> > drivers.
> > - there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of
> >them are related to connectors.
> > - there are 62 references to DRM_ERR* and 7 references to DRM_INFO in
> >the drm core programs.
> >
> > I meant I can make two patches:
> > 1 - this one with the change to CONNECTOR:id:name (29 changes)
> > 2 - a new and bigger patch to change 413 + 62 + 7 references to
> > DRM_{DEBUG,ERR,INFO}
> > in the drm core programs.
>
> The second one is an on-going change that will have to happen gradually,
> file by file. Changing connector references while at it seems like a
> reasonable drive-by-change to me. (Others may disagree.)
There are about 50 files = 50 patches. It seems excessive to me, but
I get the point: smaller changes, so they are easier to control/review/...
I am going so send the patch 1 and one of the patches 2 and we can see.
Thanks,
Claudio Suarez
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote:
> On Sun, 14 Nov 2021, Claudio Suarez wrote:
> > On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote:
> >> Hi Claudio,
> >>
> >> On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
> >> > The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> >> > drm core programs.
> >> >
> >> > Suggested-by: Ville Syrjälä
> >> > Signed-off-by: Claudio Suarez
> >>
> >> While touching all these logging calls, could you convernt them to the
> >> newer drm_dbg_kms variants?
> >> DRM_DEBUG_* are all deprecated.
> >>
> >
> > Yes, I can, but it is recommended to do it in a different patch:
> >
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes
> >
> > C&P:
> > "Separate your changes
> > Separate each logical change into a separate patch.
> > For example, if your changes include both bug fixes and performance
> > enhancements..."
> >
> >
> > I will study and send a new separate patch with your suggestion.
>
> I feel these logging changes are the types of changes where I'd err on
> the side of fewer patches than strictly following the above guidelines.
To size the problem:
- there are about 3434 references to DRM_DEBUG_* in all the drm code, all
drivers.
- there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of
them are related to connectors.
- there are 62 references to DRM_ERR* and 7 references to DRM_INFO in
the drm core programs.
I meant I can make two patches:
1 - this one with the change to CONNECTOR:id:name (29 changes)
2 - a new and bigger patch to change 413 + 62 + 7 references to
DRM_{DEBUG,ERR,INFO}
in the drm core programs.
The second patch will be ready in a few days.
Is it a good plan ? or can it be improved ?
Best regards,
Claudio Suarez.
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 12:22:08PM +0200, Jani Nikula wrote:
> On Sat, 13 Nov 2021, Claudio Suarez wrote:
> > The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> > drm core programs.
> >
> > Suggested-by: Ville Syrjälä
> > Signed-off-by: Claudio Suarez
> > ---
> > drivers/gpu/drm/drm_client_modeset.c | 51 ++--
> > drivers/gpu/drm/drm_connector.c | 12 ---
> > drivers/gpu/drm/drm_edid.c | 36 ++--
> > drivers/gpu/drm/drm_edid_load.c | 11 +++---
> > drivers/gpu/drm/drm_mode_config.c| 3 +-
> > 5 files changed, 59 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c
> > b/drivers/gpu/drm/drm_client_modeset.c
> > index ced09c7c06f9..4f35dc375bdd 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct
> > drm_connector **connectors,
> > for (i = 0; i < connector_count; i++) {
> > connector = connectors[i];
> > enabled[i] = drm_connector_enabled(connector, true);
> > - DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
> > + DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n",
> > connector->base.id, connector->name,
> > connector->display_info.non_desktop ? "non
> > desktop" : enabled[i] ? "yes" : "no");
>
> You have a semicolon instead of a colon there.
Sorry my typo. I am going to do a new version.
>
> Not to block this patch, but I've wondered about bigger changes such as:
>
> - Adding "debug_name" member or similar in drm_connector, which would be
> an allocated string with "[CONNECTOR:id:name]" that you could use with
> just %s while debug logging.
>
> - Adding drm_dbg_connector() which would take drm_connector as context,
> and do drm_dbg_kms() with the above prefix.
>
> >
> > any_enabled |= enabled[i];
> > @@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct
> > drm_connector **connectors,
> > continue;
> >
> > if (!modes[i] && (h_idx || v_idx)) {
> > - DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,
> > - connector->base.id);
> > + DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled
> > %d\n",
> > + connector->base.id, connector->name, i);
>
> Personally I'd prefer adding [CONNECTOR:id:name] as a prefix in the
> beginning, throughout, not in the middle.
I'd prefer too. I am going to change it in the new version.
B.R.
Claudio Suarez.
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote: > Hi Claudio, > > On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote: > > The prefered way to log connectors is [CONNECTOR:id:name]. Change it in > > drm core programs. > > > > Suggested-by: Ville Syrjälä > > Signed-off-by: Claudio Suarez > > While touching all these logging calls, could you convernt them to the > newer drm_dbg_kms variants? > DRM_DEBUG_* are all deprecated. > Yes, I can, but it is recommended to do it in a different patch: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes C&P: "Separate your changes Separate each logical change into a separate patch. For example, if your changes include both bug fixes and performance enhancements..." I will study and send a new separate patch with your suggestion. Best regards, Claudio Suarez
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
> The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> drm core programs.
>
> Suggested-by: Ville Syrjälä
> Signed-off-by: Claudio Suarez
> ---
> drivers/gpu/drm/drm_client_modeset.c | 51 ++--
> drivers/gpu/drm/drm_connector.c | 12 ---
> drivers/gpu/drm/drm_edid.c | 36 ++--
> drivers/gpu/drm/drm_edid_load.c | 11 +++---
> drivers/gpu/drm/drm_mode_config.c| 3 +-
> 5 files changed, 59 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index ced09c7c06f9..4f35dc375bdd 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct
> drm_connector **connectors,
> for (i = 0; i < connector_count; i++) {
> connector = connectors[i];
> enabled[i] = drm_connector_enabled(connector, true);
> - DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
> + DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n",
> connector->base.id, connector->name,
> connector->display_info.non_desktop ? "non
> desktop" : enabled[i] ? "yes" : "no");
>
> any_enabled |= enabled[i];
> @@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct
> drm_connector **connectors,
> continue;
>
> if (!modes[i] && (h_idx || v_idx)) {
> - DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,
> - connector->base.id);
> + DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled
> %d\n",
> + connector->base.id, connector->name, i);
> continue;
> }
> if (connector->tile_h_loc < h_idx)
> @@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct
> drm_connector **connectors,
> drm_client_get_tile_offsets(connectors,
> connector_count, modes, offsets, i,
> connector->tile_h_loc,
> connector->tile_v_loc);
> }
> - DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
> - connector->base.id);
> + DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",
> + connector->base.id, connector->name);
>
> /* got for command line mode first */
> modes[i] = drm_connector_pick_cmdline_mode(connector);
> if (!modes[i]) {
> - DRM_DEBUG_KMS("looking for preferred mode on connector
> %d %d\n",
> - connector->base.id, connector->tile_group
> ? connector->tile_group->id : 0);
> + DRM_DEBUG_KMS("looking for preferred mode on
> [CONNECTOR:%d:%s] %d\n",
> + connector->base.id, connector->name,
> + connector->tile_group ?
> connector->tile_group->id : 0);
> modes[i] = drm_connector_has_preferred_mode(connector,
> width, height);
> }
> /* No preferred modes, pick one off the list */
> @@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct
> drm_connector **connectors,
> (connector->tile_h_loc == 0 &&
>connector->tile_v_loc == 0 &&
>!drm_connector_get_tiled_mode(connector))) {
> - DRM_DEBUG_KMS("Falling back to non tiled mode
> on Connector %d\n",
> - connector->base.id);
> + DRM_DEBUG_KMS("Falling back to non tiled mode
> on [CONNECTOR:%d:%s]\n",
> + connector->base.id,
> connector->name);
> modes[i] =
> drm_connector_fallback_non_tiled_mode(connector);
> } else {
> modes[i] =
> drm_connector_get_tiled_mode(connector);
> @@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct
> drm_client_dev *client,
> num_connectors_detected++;
>
>
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
+CC: Ville Syrjälä
+CC: Daniel Vetter
On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote:
> The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
> drm core programs.
>
> Suggested-by: Ville Syrjälä
> Signed-off-by: Claudio Suarez
> ---
> drivers/gpu/drm/drm_client_modeset.c | 51 ++--
> drivers/gpu/drm/drm_connector.c | 12 ---
> drivers/gpu/drm/drm_edid.c | 36 ++--
> drivers/gpu/drm/drm_edid_load.c | 11 +++---
> drivers/gpu/drm/drm_mode_config.c| 3 +-
> 5 files changed, 59 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index ced09c7c06f9..4f35dc375bdd 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct
> drm_connector **connectors,
> for (i = 0; i < connector_count; i++) {
> connector = connectors[i];
> enabled[i] = drm_connector_enabled(connector, true);
> - DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
> + DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n",
> connector->base.id, connector->name,
> connector->display_info.non_desktop ? "non
> desktop" : enabled[i] ? "yes" : "no");
>
> any_enabled |= enabled[i];
> @@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct
> drm_connector **connectors,
> continue;
>
> if (!modes[i] && (h_idx || v_idx)) {
> - DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,
> - connector->base.id);
> + DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled
> %d\n",
> + connector->base.id, connector->name, i);
> continue;
> }
> if (connector->tile_h_loc < h_idx)
> @@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct
> drm_connector **connectors,
> drm_client_get_tile_offsets(connectors,
> connector_count, modes, offsets, i,
> connector->tile_h_loc,
> connector->tile_v_loc);
> }
> - DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
> - connector->base.id);
> + DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",
> + connector->base.id, connector->name);
>
> /* got for command line mode first */
> modes[i] = drm_connector_pick_cmdline_mode(connector);
> if (!modes[i]) {
> - DRM_DEBUG_KMS("looking for preferred mode on connector
> %d %d\n",
> - connector->base.id, connector->tile_group
> ? connector->tile_group->id : 0);
> + DRM_DEBUG_KMS("looking for preferred mode on
> [CONNECTOR:%d:%s] %d\n",
> + connector->base.id, connector->name,
> + connector->tile_group ?
> connector->tile_group->id : 0);
> modes[i] = drm_connector_has_preferred_mode(connector,
> width, height);
> }
> /* No preferred modes, pick one off the list */
> @@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct
> drm_connector **connectors,
> (connector->tile_h_loc == 0 &&
>connector->tile_v_loc == 0 &&
>!drm_connector_get_tiled_mode(connector))) {
> - DRM_DEBUG_KMS("Falling back to non tiled mode
> on Connector %d\n",
> - connector->base.id);
> + DRM_DEBUG_KMS("Falling back to non tiled mode
> on [CONNECTOR:%d:%s]\n",
> + connector->base.id,
> connector->name);
> modes[i] =
> drm_connector_fallback_non_tiled_mode(connector);
> } else {
> modes[i] =
> drm_connector_get_tiled_mode(connector);
> @@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct
> drm_client_dev *client,
> num_conn
[PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
The prefered way to log connectors is [CONNECTOR:id:name]. Change it in
drm core programs.
Suggested-by: Ville Syrjälä
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_client_modeset.c | 51 ++--
drivers/gpu/drm/drm_connector.c | 12 ---
drivers/gpu/drm/drm_edid.c | 36 ++--
drivers/gpu/drm/drm_edid_load.c | 11 +++---
drivers/gpu/drm/drm_mode_config.c| 3 +-
5 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..4f35dc375bdd 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -240,7 +240,7 @@ static void drm_client_connectors_enabled(struct
drm_connector **connectors,
for (i = 0; i < connector_count; i++) {
connector = connectors[i];
enabled[i] = drm_connector_enabled(connector, true);
- DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
+ DRM_DEBUG_KMS("[CONNECTOR:%d;%s] enabled? %s\n",
connector->base.id, connector->name,
connector->display_info.non_desktop ? "non
desktop" : enabled[i] ? "yes" : "no");
any_enabled |= enabled[i];
@@ -350,8 +350,8 @@ static int drm_client_get_tile_offsets(struct drm_connector
**connectors,
continue;
if (!modes[i] && (h_idx || v_idx)) {
- DRM_DEBUG_KMS("no modes for connector tiled %d %d\n", i,
- connector->base.id);
+ DRM_DEBUG_KMS("no modes for [CONNECTOR:%d:%s] tiled
%d\n",
+ connector->base.id, connector->name, i);
continue;
}
if (connector->tile_h_loc < h_idx)
@@ -419,14 +419,15 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
drm_client_get_tile_offsets(connectors,
connector_count, modes, offsets, i,
connector->tile_h_loc,
connector->tile_v_loc);
}
- DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
- connector->base.id);
+ DRM_DEBUG_KMS("looking for cmdline mode on [CONNECTOR:%d:%s]\n",
+ connector->base.id, connector->name);
/* got for command line mode first */
modes[i] = drm_connector_pick_cmdline_mode(connector);
if (!modes[i]) {
- DRM_DEBUG_KMS("looking for preferred mode on connector
%d %d\n",
- connector->base.id, connector->tile_group
? connector->tile_group->id : 0);
+ DRM_DEBUG_KMS("looking for preferred mode on
[CONNECTOR:%d:%s] %d\n",
+ connector->base.id, connector->name,
+ connector->tile_group ?
connector->tile_group->id : 0);
modes[i] = drm_connector_has_preferred_mode(connector,
width, height);
}
/* No preferred modes, pick one off the list */
@@ -448,8 +449,8 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
(connector->tile_h_loc == 0 &&
connector->tile_v_loc == 0 &&
!drm_connector_get_tiled_mode(connector))) {
- DRM_DEBUG_KMS("Falling back to non tiled mode
on Connector %d\n",
- connector->base.id);
+ DRM_DEBUG_KMS("Falling back to non tiled mode
on [CONNECTOR:%d:%s]\n",
+ connector->base.id,
connector->name);
modes[i] =
drm_connector_fallback_non_tiled_mode(connector);
} else {
modes[i] =
drm_connector_get_tiled_mode(connector);
@@ -617,15 +618,15 @@ static bool drm_client_firmware_config(struct
drm_client_dev *client,
num_connectors_detected++;
if (!enabled[i]) {
- DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
- connector->name);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] not enabled,
skipping\n",
+ connector->base.id, connector->name);
conn_configured |= BIT(i);
continue;
Re: [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Wed, Oct 27, 2021 at 07:28:45AM +0900, Inki Dae wrote: > Hi, > > 21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글: > > Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. Retriving the same information with > > drm_detect_hdmi_monitor() is less efficient. Change to > > drm_display_info.is_hdmi > > > > Signed-off-by: Claudio Suarez > > --- > > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions( > > > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > > b/drivers/gpu/drm/exynos/exynos_hdmi.c > > index 7655142a4651..a563d6386abe 100644 > > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > > @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector > > *connector) > > if (!edid) > > return -ENODEV; > > > > - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); > > + /* This updates connector->display_info */ > > + drm_connector_update_edid_property(connector, edid); > > + > > + hdata->dvi_mode = !connector->display_info.is_hdmi; > > Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi > parsed from EDID. > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/drm_edid.c?h=v5.14.14#n4725 > > Signed-off-by: Inki Dae Thank you, Inki. Best regards Claudio Suarez
Re: [PATCH v4 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) will be treated
as an HDMI sink.
>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.
The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
--
2.33.0
Re: [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Mon, Oct 25, 2021 at 12:17:37AM +0200, Claudio Suarez wrote: [...] No new comments about this, I suppose everything is fine. I'm going to send the patch with this changes. Thanks to all and special thanks to you, Ville. Hope this helps the kernel. Don't hesitate to ask new changes if necessary. Best regards Claudio Suarez
Re: [Intel-gfx] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 22, 2021 at 03:22:57PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
> > > On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > > > > drm_get_edid() internally calls to
> > > > > drm_connector_update_edid_property()
> > > > > and then drm_add_display_info(), which parses the EDID.
> > > > > This happens in the function intel_hdmi_set_edid() and
> > > > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > > > >
> > > > > Once EDID is parsed, the monitor HDMI support information is available
> > > > > through drm_display_info.is_hdmi. Retriving the same information with
> > > > > drm_detect_hdmi_monitor() is less efficient. Change to
> > > > > drm_display_info.is_hdmi
> > > >
> > > > I meant we need to examine all call chains that can lead to
> > > > .detect() to make sure all of them do in fact update the
> > > > display_info beforehand.
> > >
> > > Well, I studied it carefully and, yes, all call chains that can lead to
> > > drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
> > > beforehand. In the case that this doesn't happen, the code is unchanged.
> > >
> > > Do you want I explain the changes in the code here again ? Or do you want
> > > to me change the commit message to be more clear ? In the first case, I
> > > can
> > > write here a detailed explanation. In the second case I can make a longer
> > > commit
> > > message.
> > >
> > > Or both?
> >
> > I want all those call chains explained in the commit message,
> > otherwise I have no easy way to confirm whether the change
> > is correct or not.
>
> Hmm. OK, so I had a bit of a dig around and seems that what we do now
> .detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()
Yes. I said before that I felt something was wrong when I read the
documentation and then the code. To be more explicit now, I expected that
drm_connector_update_edid_property() will be done in the
fill_modes/get_modes phase instead of when reading the edid.
The documentation suggests that but the code reads the edid in the
detect phase.
Now, since drm_connector_update_edid_property() is called in the detect
phase, it is not necessary to keep the edid data in the private connector
struct. It is in struct drm_connector from the beginning.
But this is topic for another patch.
> Now the question is when did that start happening? Looks like it was
> commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields
> at edid property set [v2]") that started to call drm_add_display_info()
> from drm_connector_update_edid_property(), and then commit 5186421cbfe2
> ("drm: Introduce epoch counter to drm_connector") started to call
> drm_connector_update_edid_property() from drm_get_edid(). Before both
> of those commits were in place display_info would still contain
> some stale garbage during .detect().
>
> That is the story I think we want in these commit messages since it
> a) explains why the old code was directly parsing the edid instead
> b) why it's now safe to change this
--commit-message?
drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) shall be treated
as an HDMI sink.
>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.
The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.
---
>
> PS. connector->force handling in drm_get_edid() looks a bit busted
> since it doesn't
Re: [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote: > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote: > > drm_get_edid() internally calls to drm_connector_update_edid_property() > > and then drm_add_display_info(), which parses the EDID. > > This happens in the function intel_hdmi_set_edid() and > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()). > > > > Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. Retriving the same information with > > drm_detect_hdmi_monitor() is less efficient. Change to > > drm_display_info.is_hdmi > > I meant we need to examine all call chains that can lead to > .detect() to make sure all of them do in fact update the > display_info beforehand. Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged. Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message. Or both? Best Regards, Claudio Suarez.
Re: [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
drm_get_edid() internally calls to drm_connector_update_edid_property()
and then drm_add_display_info(), which parses the EDID.
This happens in the function intel_hdmi_set_edid() and
intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
--
2.33.0
Re: [PATCH v3 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
According to the documentation, drm_add_edid_modes
"... Also fills out the &drm_display_info structure and ELD in @connector
with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.
Since drm_edid_is_valid() considers NULL as an invalid EDID, simplify the
code to avoid duplicating code in the case of NULL/error.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_edid.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..a019a26ede7a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,14 @@ int drm_add_edid_modes(struct drm_connector *connector,
struct edid *edid)
int num_modes = 0;
u32 quirks;
- if (edid == NULL) {
- clear_eld(connector);
- return 0;
- }
if (!drm_edid_is_valid(edid)) {
+ /* edid == NULL or invalid here */
clear_eld(connector);
- drm_warn(connector->dev, "%s: EDID invalid.\n",
-connector->name);
+ drm_reset_display_info(connector);
+ if (edid)
+ drm_warn(connector->dev,
+"[CONNECTOR:%d:%s] EDID invalid.\n",
+connector->base.id, connector->name);
return 0;
}
--
2.33.0
Re: [PATCH v2 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Tue, Oct 19, 2021 at 09:35:08PM +0300, Ville Syrjälä wrote:
> On Sat, Oct 16, 2021 at 08:42:14PM +0200, Claudio Suarez wrote:
> > According to the documentation, drm_add_edid_modes
> > "... Also fills out the &drm_display_info structure and ELD in @connector
> > with any information which can be derived from the edid."
> >
> > drm_add_edid_modes accepts a struct edid *edid parameter which may have a
> > value or may be null. When it is not null, connector->display_info and
> > connector->eld are updated according to the edid. When edid=NULL, only
> > connector->eld is reset. Reset connector->display_info to be consistent
> > and accurate.
> >
> > Signed-off-by: Claudio Suarez
> > ---
> > drivers/gpu/drm/drm_edid.c | 11 +--
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 6325877c5fd6..c643db17782c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector
> > *connector, struct edid *edid)
> > int num_modes = 0;
> > u32 quirks;
> >
> > - if (edid == NULL) {
> > - clear_eld(connector);
> > - return 0;
> > - }
> > if (!drm_edid_is_valid(edid)) {
>
> OK, so drm_edid_is_valid() will happily accept NULL and considers
> it invalid. You may want to mention that explicitly in the commit
> message.
Thank you for your comments, I appreciate :)
I'm sending new mails with the new commit messages.
> > + /* edid == NULL or invalid here */
> > clear_eld(connector);
> > - drm_warn(connector->dev, "%s: EDID invalid.\n",
> > -connector->name);
> > + drm_reset_display_info(connector);
> > + if (edid)
> > + drm_warn(connector->dev, "%s: EDID invalid.\n",
> > +connector->name);
>
> Could you respin this to use the standard [CONNECTOR:%d:%s] form
> while at it? Or I guess a patch to mass convert the whole drm_edid.c
> might be another option.
Good point.
I like the idea of a new patch. I'll start working on it. I can change
this drm_warn here to avoid merge conflicts.
> Patch looks good.
> Reviewed-by: Ville Syrjälä
Thanks!
BR
Claudio Suarez.
[PATCH v2 08/13] drm/sun4i: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2f2c9f0a1071..f57bedbbeeb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector
*connector)
if (!edid)
return 0;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
- drm_connector_update_edid_property(connector, edid);
cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
--
2.33.0
[PATCH v2 12/13] drm/nouveau: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++--
drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..b3c7199aa63d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct
nouveau_crtc *nv_crtc,
int ret;
int size;
- if (!drm_detect_hdmi_monitor(nv_connector->edid))
+ if (!nv_connector->base.display_info.is_hdmi)
return;
hdmi = &nv_connector->base.display_info.hdmi;
@@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
struct drm_atomic_state *sta
*/
if (mode->clock >= 165000 &&
nv_encoder->dcb->duallink_possible &&
- !drm_detect_hdmi_monitor(nv_connector->edid))
+ !nv_connector->base.display_info.is_hdmi)
proto =
NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
} else {
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index d66f97280282..29d6c010ac13 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
struct drm_display_mode *omode = &asyh->state.adjusted_mode;
struct drm_display_mode *umode = &asyh->state.mode;
int mode = asyc->scaler.mode;
- struct edid *edid;
int umode_vdisplay, omode_hdisplay, omode_vdisplay;
- if (connector->edid_blob_ptr)
- edid = (struct edid *)connector->edid_blob_ptr->data;
- else
- edid = NULL;
-
if (!asyc->scaler.full) {
if (mode == DRM_MODE_SCALE_NONE)
omode = umode;
@@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
*/
if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
(asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
-drm_detect_hdmi_monitor(edid {
+connector->display_info.is_hdmi))) {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
--
2.33.0
[PATCH v2 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH v2 05/13] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..d9db5d52d52e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect(
struct edid *edid = NULL;
enum drm_connector_status status = connector_status_disconnected;
+ /* This updates connector->display_info */
edid = drm_get_edid(connector, &gma_encoder->i2c_bus->adapter);
hdmi_priv->has_hdmi_sink = false;
@@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect(
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
hdmi_priv->has_hdmi_sink =
- drm_detect_hdmi_monitor(edid);
+ connector->display_info.is_hdmi;
hdmi_priv->has_hdmi_audio =
drm_detect_monitor_audio(edid);
}
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 355da2856389..5ef49d17de98 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (psb_intel_sdvo->is_hdmi) {
- psb_intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
- psb_intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ psb_intel_sdvo->has_hdmi_monitor =
+ connector->display_info.is_hdmi;
+ psb_intel_sdvo->has_hdmi_audio =
+ drm_detect_monitor_audio(edid);
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH v2 09/13] drm/sti: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sti/sti_hdmi.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f3ace11209dd..3f8b04a1407e 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
goto fail;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
- DRM_DEBUG_KMS("%s : %dx%d cm\n",
- (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
- edid->width_cm, edid->height_cm);
cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
count = drm_add_edid_modes(connector, edid);
+
+ /* This updates connector->display_info */
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
+ DRM_DEBUG_KMS("%s : %dx%d cm\n",
+ (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
+ edid->width_cm, edid->height_cm);
kfree(edid);
return count;
--
2.33.0
[PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi Signed-off-by: Claudio Suarez --- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV; - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); + /* This updates connector->display_info */ + drm_connector_update_edid_property(connector, edid); + + hdata->dvi_mode = !connector->display_info.is_hdmi; DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm); - drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid); ret = drm_add_edid_modes(connector, edid); -- 2.33.0
[PATCH v2 07/13] drm/msm: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 58707a1f3878..07585092f919 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct
drm_connector *connector)
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
- hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_mode = connector->display_info.is_hdmi;
if (edid) {
ret = drm_add_edid_modes(connector, edid);
--
2.33.0
[PATCH v2 10/13] drm/rockchip: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++--
drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7afdc54eb3ec..d479f230833e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
- hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
+ hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 1c546c3a8998..03aaae39cf61 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
--
2.33.0
[PATCH v2 11/13] drm/bridge: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
drivers/gpu/drm/bridge/sii902x.c | 2 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..f6891280a58d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511
*adv7511,
__adv7511_power_off(adv7511);
adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
- drm_detect_hdmi_monitor(edid));
+ connector->display_info.is_hdmi);
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..5719be0a03c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector
*connector)
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
drm_connector_update_edid_property(connector, edid);
if (edid) {
- if (drm_detect_hdmi_monitor(edid))
+ if (connector->display_info.is_hdmi)
output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
num = drm_add_edid_modes(connector, edid);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..33f0afb6b646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);
- hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+ hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
return edid;
--
2.33.0
[PATCH v2 04/13] drm/tegra: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/tegra/hdmi.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..21571221b49b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
static bool tegra_output_is_hdmi(struct tegra_output *output)
{
- struct edid *edid;
-
if (!output->connector.edid_blob_ptr)
return false;
- edid = (struct edid *)output->connector.edid_blob_ptr->data;
-
- return drm_detect_hdmi_monitor(edid);
+ return output->connector.display_info.is_hdmi;
}
static enum drm_connector_status
--
2.33.0
[PATCH v2 03/13] drm/radeon: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information is less
efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Also, correct an inacurracy or bug in
radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables
have EDID data:
- struct radeon_connector.edid
- struct drm_connector.edid_blob_ptr
The second is updated by calling drm_connector_update_edid_property() or
drm_get_edid() - which internally calls drm_connector_update_edid_property().
drm_display_info.is_hdmi is updated when this function is called.
radeon_connector_get_edid() calls drm_get_edid() to update
drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it
to be always up to date, so drm_display_info is always correct.
As counterpart, reset these values in radeon_connector_free_edid().
This second change is necessary for the previous one to work properly.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
drivers/gpu/drm/radeon/radeon_connectors.c | 15 +--
drivers/gpu/drm/radeon/radeon_display.c| 2 +-
drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++--
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..844f61a36f20 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_connector->use_digital &&
(radeon_connector->audio == RADEON_AUDIO_ENABLE))
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else if (radeon_connector->use_digital)
@@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
@@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
} else if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 607ad5620bd9..deaae181ac59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (radeon_connector->use_digital) {
- if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
- if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
dig_connector = radeon_connector->con_priv;
if ((dig_connector->dp_sink_type ==
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
(dig_connector->
[PATCH v2 02/13] drm/vc4: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Use this value instead of calling
drm_detect_hdmi_monitor() to avoid a second parse.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b4b4653fe301..d531e4c501eb 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
+ vc4_hdmi->encoder.hdmi_monitor =
+ connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
+ vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
kfree(edid);
if (vc4_hdmi->disable_4kp60) {
--
2.33.0
[PATCH v2 00/13] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Changelog: v2: - no helper function - A separate patch is made for amdgpu - zte patch is removed because that driver no longer exists [Why] Copy&paste from Documentation/gpu/todo.rst === Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi --- Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to retrieve the same information, which is less efficient. Audit each individual driver calling drm_detect_hdmi_monitor() and switch to drm_display_info.is_hdmi if applicable. = [How] I did it in two steps: - check that drm_display_info has a correct value. - in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some small inconsistencies have been solved. Stats: drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +- drivers/gpu/drm/drm_edid.c | 11 +-- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 -- drivers/gpu/drm/i915/display/intel_hdmi.c| 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c| 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c| 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++-- drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +--- drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 +-- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c | 10 ++ drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++-- drivers/gpu/drm/tegra/hdmi.c | 6 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++--- 22 files changed, 55 insertions(+), 55 deletions(-) Best regards. Claudio Suarez
[PATCH v2 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
According to the documentation, drm_add_edid_modes
"... Also fills out the &drm_display_info structure and ELD in @connector
with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_edid.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..c643db17782c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector,
struct edid *edid)
int num_modes = 0;
u32 quirks;
- if (edid == NULL) {
- clear_eld(connector);
- return 0;
- }
if (!drm_edid_is_valid(edid)) {
+ /* edid == NULL or invalid here */
clear_eld(connector);
- drm_warn(connector->dev, "%s: EDID invalid.\n",
-connector->name);
+ drm_reset_display_info(connector);
+ if (edid)
+ drm_warn(connector->dev, "%s: EDID invalid.\n",
+connector->name);
return 0;
}
--
2.33.0
Re: [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver
On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote: > > > On 2021-10-15 07:37, Claudio Suarez wrote: > > a) Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. The amdgpu driver still calls > > drm_detect_hdmi_monitor() to retrieve the same information, which > > is less efficient. Change to drm_display_info.is_hdmi > > > > This is a TODO task in Documentation/gpu/todo.rst > > > > b) drm_display_info is updated by drm_get_edid() or > > drm_connector_update_edid_property(). In the amdgpu driver it is almost > > always updated when the edid is read in amdgpu_connector_get_edid(), > > but not always. Change amdgpu_connector_get_edid() and > > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows > > a) > > to work properly. > > > > c) Use drm_edid_get_monitor_name() instead of duplicating the code that > > parses the EDID in dm_helpers_parse_edid_caps() > > > > Also, remove the unused "struct dc_context *ctx" parameter in > > dm_helpers_parse_edid_caps() > > > > Thanks for this work. > > The fact that you listed three separate changes in this commit > is a clear indication that this patch should be three separate > patches instead. Separating the functional bits from the straight > refactor will help with bisection if this leads to a regression. > > All changes look reasonable to me, though. With this patch split > into three patches in the sequence (b), (c), then (a) this is > Reviewed-by: Harry Wentland Ok, thanks. I'll send three patches. BR Claudio Suarez
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Sat, Oct 16, 2021 at 10:25:03AM +0200, Claudio Suarez wrote:
> On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > > According to the documentation, drm_add_edid_modes
> > > > > "... Also fills out the &drm_display_info structure and ELD in
> > > > > @connector
> > > > > with any information which can be derived from the edid."
> > > > >
> > > > > drm_add_edid_modes accepts a struct edid *edid parameter which may
> > > > > have a
> > > > > value or may be null. When it is not null, connector->display_info and
> > > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > > connector->eld is reset. Reset connector->display_info to be
> > > > > consistent
> > > > > and accurate.
> > > > >
> > > > > Signed-off-by: Claudio Suarez
> > > > > ---
> > > > > drivers/gpu/drm/drm_edid.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector
> > > > > *connector, struct edid *edid)
> > > > >
> > > > > if (edid == NULL) {
> > > > > clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > > > return 0;
> > > > > }
> > > > > if (!drm_edid_is_valid(edid)) {
> > > > > clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > >
> > > > Looks easier if you pull both of those out from these branches and
> > > > just call them unconditionally at the start.
> > >
> > > After looking at the full code, I am not sure. This is the code:
> > > ==
> > > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > > {
> > > int num_modes = 0;
> > > u32 quirks;
> > >
> > > if (edid == NULL) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > return 0;
> > > }
> > > if (!drm_edid_is_valid(edid)) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > drm_warn(connector->dev, "%s: EDID invalid.\n",
> > > connector->name);
> > > return 0;
> > > }
> > >
> > > drm_edid_to_eld(connector, edid);
> > >
> > > quirks = drm_add_display_info(connector, edid);
> > > etc...
> > > =
> > >
> > > If we move those out of these branches and edid != NULL, we are executing
> > > an
> > > unnecessary clear_eld(connector) and an unnecessary
> > > drm_reset_display_info(connector)
> > > because the fields will be set in the next drm_edid_to_eld(connector,
> > > edid) and
> > > drm_add_display_info(connector, edid)
> > >
> > > Do we want this ?
> >
> > Seems fine by me. And maybe we could nuke the second
> > drm_reset_display_info() from deeper inside drm_add_display_info()?
> > Not sure if drm_add_display_info() still has to be able to operate
> > standalone or not.
> >
> > Hmm. Another option is to just move all these NULL/invalid edid
> > checks into drm_edid_to_eld() and drm_add_display_info().
>
> I was thinking about this. We can use a boolean variable:
> ===
> bool edid_is_invalid;
>
> edid_is_invalid = !drm_edid_is_valid(edid);
>
> if (edid == NULL || edid_is_invalid) {
> clear_eld(connector);
> drm_reset_display_info(connector);
> if (edid_is_invalid)
> drm_warn(connector->dev, "%s: EDID invalid.\n",
> connector->name);
> return 0;
> }
>
> drm_edid_to_eld(connector, edid);
> ...
> ===
> Internally, drm_edid_is_valid() handles NULL pointers properly.
> It is a quite elegant solution with a small change in the original
> design, and it improves this part in the way you pointed out.
I'll send a patch with this idea and we can talk about the new code.
Thanks!
Best regards,
Claudio Suarez.
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > According to the documentation, drm_add_edid_modes
> > > > "... Also fills out the &drm_display_info structure and ELD in
> > > > @connector
> > > > with any information which can be derived from the edid."
> > > >
> > > > drm_add_edid_modes accepts a struct edid *edid parameter which may have
> > > > a
> > > > value or may be null. When it is not null, connector->display_info and
> > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > connector->eld is reset. Reset connector->display_info to be consistent
> > > > and accurate.
> > > >
> > > > Signed-off-by: Claudio Suarez
> > > > ---
> > > > drivers/gpu/drm/drm_edid.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector
> > > > *connector, struct edid *edid)
> > > >
> > > > if (edid == NULL) {
> > > > clear_eld(connector);
> > > > + drm_reset_display_info(connector);
> > > > return 0;
> > > > }
> > > > if (!drm_edid_is_valid(edid)) {
> > > > clear_eld(connector);
> > > > + drm_reset_display_info(connector);
> > >
> > > Looks easier if you pull both of those out from these branches and
> > > just call them unconditionally at the start.
> >
> > After looking at the full code, I am not sure. This is the code:
> > ==
> > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > {
> > int num_modes = 0;
> > u32 quirks;
> >
> > if (edid == NULL) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > return 0;
> > }
> > if (!drm_edid_is_valid(edid)) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > drm_warn(connector->dev, "%s: EDID invalid.\n",
> > connector->name);
> > return 0;
> > }
> >
> > drm_edid_to_eld(connector, edid);
> >
> > quirks = drm_add_display_info(connector, edid);
> > etc...
> > =
> >
> > If we move those out of these branches and edid != NULL, we are executing an
> > unnecessary clear_eld(connector) and an unnecessary
> > drm_reset_display_info(connector)
> > because the fields will be set in the next drm_edid_to_eld(connector, edid)
> > and
> > drm_add_display_info(connector, edid)
> >
> > Do we want this ?
>
> Seems fine by me. And maybe we could nuke the second
> drm_reset_display_info() from deeper inside drm_add_display_info()?
> Not sure if drm_add_display_info() still has to be able to operate
> standalone or not.
>
> Hmm. Another option is to just move all these NULL/invalid edid
> checks into drm_edid_to_eld() and drm_add_display_info().
I was thinking about this. We can use a boolean variable:
=======
bool edid_is_invalid;
edid_is_invalid = !drm_edid_is_valid(edid);
if (edid == NULL || edid_is_invalid) {
clear_eld(connector);
drm_reset_display_info(connector);
if (edid_is_invalid)
drm_warn(connector->dev, "%s: EDID invalid.\n",
connector->name);
return 0;
}
drm_edid_to_eld(connector, edid);
...
===
Internally, drm_edid_is_valid() handles NULL pointers properly.
It is a quite elegant solution with a small change in the original
design, and it improves this part in the way you pointed out.
Best regards,
Claudio Suarez
Re: [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > According to the documentation, drm_add_edid_modes
> > "... Also fills out the &drm_display_info structure and ELD in @connector
> > with any information which can be derived from the edid."
> >
> > drm_add_edid_modes accepts a struct edid *edid parameter which may have a
> > value or may be null. When it is not null, connector->display_info and
> > connector->eld are updated according to the edid. When edid=NULL, only
> > connector->eld is reset. Reset connector->display_info to be consistent
> > and accurate.
> >
> > Signed-off-by: Claudio Suarez
> > ---
> > drivers/gpu/drm/drm_edid.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 6325877c5fd6..6cbe09b2357c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector
> > *connector, struct edid *edid)
> >
> > if (edid == NULL) {
> > clear_eld(connector);
> > + drm_reset_display_info(connector);
> > return 0;
> > }
> > if (!drm_edid_is_valid(edid)) {
> > clear_eld(connector);
> > + drm_reset_display_info(connector);
>
> Looks easier if you pull both of those out from these branches and
> just call them unconditionally at the start.
After looking at the full code, I am not sure. This is the code:
==
int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
{
int num_modes = 0;
u32 quirks;
if (edid == NULL) {
clear_eld(connector);
drm_reset_display_info(connector); <--- added by me
return 0;
}
if (!drm_edid_is_valid(edid)) {
clear_eld(connector);
drm_reset_display_info(connector); <--- added by me
drm_warn(connector->dev, "%s: EDID invalid.\n",
connector->name);
return 0;
}
drm_edid_to_eld(connector, edid);
quirks = drm_add_display_info(connector, edid);
etc...
=
If we move those out of these branches and edid != NULL, we are executing an
unnecessary clear_eld(connector) and an unnecessary
drm_reset_display_info(connector)
because the fields will be set in the next drm_edid_to_eld(connector, edid) and
drm_add_display_info(connector, edid)
Do we want this ?
BR
Claudio Suarez
Re: [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 06:18:34PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Ville Syrjälä wrote:
> > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> >> On Fri, 15 Oct 2021, Claudio Suarez wrote:
> >> > Once EDID is parsed, the monitor HDMI support information is available
> >> > through drm_display_info.is_hdmi. Retriving the same information with
> >> > drm_detect_hdmi_monitor() is less efficient. Change to
> >> > drm_display_info.is_hdmi where possible.
> >> >
> >> > This is a TODO task in Documentation/gpu/todo.rst
> >> >
> >> > Signed-off-by: Claudio Suarez
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_connector.c | 5 +
> >> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> >> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> >> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> >> > b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > index 9bed1ccecea0..3346b55df6e1 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
> >> > *connector,
> >> > return ret;
> >> > }
> >> >
> >> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> >> > +{
> >> > +return connector->display_info.is_hdmi;
> >> > +}
> >> > +
> >>
> >> A helper like this belongs in drm, not i915. Seems useful in other
> >> drivers too.
> >
> > Not sure it's actually helpful for i915. We end up having to root around
> > in the display_info in a lot of places anyway. So a helper for single
> > boolean seems a bit out of place perhaps.
>
> *shrug*
>
> Maybe it's just my frustration at the lack of interfaces and poking
> around in the depths of nested structs and pointer chasing that's coming
> through. You just need to change so many things if you want to later
> refactor where "is hdmi" comes from and is stored.
>
> Anyway, if a helper is being added like in this series, I think it
> should be one helper in drm, not redundant copies in multiple
> drivers. Or we should not have the helper(s) at all. One or the other,
> not the worst of both worlds.
Thank you all for your comments :)
The big work here was to figure out which drm_detect_hdmi_monitor() can be
replaced. Changing a helper isn't a problem.
I'll send a new patch in a few hours.
BR
Claudio Suarez.
Re: [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 03:30:49PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 01:37:13PM +0200, Claudio Suarez wrote:
> > Once EDID is parsed, the monitor HDMI support information is available
> > through drm_display_info.is_hdmi. Retriving the same information with
> > drm_detect_hdmi_monitor() is less efficient. Change to
> > drm_display_info.is_hdmi where possible.
> >
> > This is a TODO task in Documentation/gpu/todo.rst
> >
> > Signed-off-by: Claudio Suarez
> > ---
> > drivers/gpu/drm/i915/display/intel_connector.c | 5 +
> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> > b/drivers/gpu/drm/i915/display/intel_connector.c
> > index 9bed1ccecea0..3346b55df6e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
> > *connector,
> > return ret;
> > }
> >
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> > +{
> > + return connector->display_info.is_hdmi;
> > +}
> > +
> > static const struct drm_prop_enum_list force_audio_names[] = {
> > { HDMI_AUDIO_OFF_DVI, "force-dvi" },
> > { HDMI_AUDIO_OFF, "off" },
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
> > b/drivers/gpu/drm/i915/display/intel_connector.h
> > index 661a37a3c6d8..ceda6e72ece6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.h
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> > @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
> > *connector);
> > int intel_connector_update_modes(struct drm_connector *connector,
> > struct edid *edid);
> > int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter
> > *adapter);
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
> > void intel_attach_force_audio_property(struct drm_connector *connector);
> > void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> > void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index b04685bb6439..2b1d7c5bebdd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> > to_intel_connector(connector)->detect_edid = edid;
> > if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> > intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> > - intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> > + intel_hdmi->has_hdmi_sink =
> > intel_connector_is_hdmi_monitor(connector);
>
> Hmm. Have we parse the EDID by this point actually? I don't think that
> was the case in the past but maybe it changed at some point.
Yes, I think so. The complete code is:
edid = drm_get_edid(connector, i2c);
if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
drm_dbg_kms(&dev_priv->drm,
"HDMI GMBUS EDID read failed, retry using GPIO
bit-banging\n");
intel_gmbus_force_bit(i2c, true);
edid = drm_get_edid(connector, i2c);
intel_gmbus_force_bit(i2c, false);
}
intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
intel_hdmi->has_hdmi_sink =
intel_connector_is_hdmi_monitor(connector);
The edid value comes from drm_get_edid(), first or second.
drm_get_edid() internally calls drm_connector_update_edid_property()
drm_connector_update_edid_property() calls drm_add_display_info() and parses
the edid.
So, the edid is parsed.
I checked this and I read the docs many times because at the first time I felt
something
was wrong. But that is the sequence of calls.
>
> >
> > conn
[PATCH 05/15] drm/tegra: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/tegra/hdmi.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..21571221b49b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
static bool tegra_output_is_hdmi(struct tegra_output *output)
{
- struct edid *edid;
-
if (!output->connector.edid_blob_ptr)
return false;
- edid = (struct edid *)output->connector.edid_blob_ptr->data;
-
- return drm_detect_hdmi_monitor(edid);
+ return output->connector.display_info.is_hdmi;
}
static enum drm_connector_status
--
2.33.0
[PATCH 04/15] drm/radeon: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information is less
efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Also, correct an inacurracy or bug in
radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables
have EDID data:
- struct radeon_connector.edid
- struct drm_connector.edid_blob_ptr
The second is updated by calling drm_connector_update_edid_property() or
drm_get_edid() - which internally calls drm_connector_update_edid_property().
drm_display_info.is_hdmi is updated when this function is called.
radeon_connector_get_edid() calls drm_get_edid() to update
drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it
to be always up to date, so drm_display_info is always correct.
As counterpart, reset these values in radeon_connector_free_edid().
This second change is necessary for the previous one to work properly.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
drivers/gpu/drm/radeon/radeon_connectors.c | 20 ++--
drivers/gpu/drm/radeon/radeon_display.c| 2 +-
drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++--
drivers/gpu/drm/radeon/radeon_mode.h | 1 +
5 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..29a140732f71 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_connector->use_digital &&
(radeon_connector->audio == RADEON_AUDIO_ENABLE))
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (radeon_connector_is_hdmi_monitor(connector) &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else if (radeon_connector->use_digital)
@@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (radeon_connector_is_hdmi_monitor(connector) &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
@@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
} else if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (radeon_connector_is_hdmi_monitor(connector) &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 607ad5620bd9..0200f094467c 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (radeon_connector->use_digital) {
- if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (radeon_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
- if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (radeon_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
dig_connector = radeon_connector->con_priv;
if ((dig_
[PATCH 08/15] drm/msm: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 58707a1f3878..07585092f919 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct
drm_connector *connector)
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
- hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_mode = connector->display_info.is_hdmi;
if (edid) {
ret = drm_add_edid_modes(connector, edid);
--
2.33.0
[PATCH 09/15] drm/sun4i: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2f2c9f0a1071..f57bedbbeeb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector
*connector)
if (!edid)
return 0;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
- drm_connector_update_edid_property(connector, edid);
cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
--
2.33.0
[PATCH 00/15] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Copy&paste from the TODO document Documentation/gpu/todo.rst === Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi --- Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to retrieve the same information, which is less efficient. Audit each individual driver calling drm_detect_hdmi_monitor() and switch to drm_display_info.is_hdmi if applicable. = I did it in two steps: - check that drm_display_info has a correct value. - in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some small inconsistencies have been solved. Stats: drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.h| 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/atombios_encoders.c| 6 +++--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 --- drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/drm_edid.c| 2 ++ drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 -- drivers/gpu/drm/i915/display/intel_connector.c| 5 + drivers/gpu/drm/i915/display/intel_connector.h| 1 + drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++-- drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +--- drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.h | 6 ++ drivers/gpu/drm/radeon/atombios_encoders.c| 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c| 20 ++-- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- drivers/gpu/drm/radeon/radeon_mode.h | 1 + drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c| 2 +- drivers/gpu/drm/sti/sti_hdmi.c| 10 ++ drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c| 4 ++-- drivers/gpu/drm/tegra/hdmi.c | 6 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 6 +++--- drivers/gpu/drm/zte/zx_hdmi.c | 4 ++-- 37 files changed, 112 insertions(+), 96 deletions(-) Best regards. Claudio Suarez
[PATCH 10/15] drm/sti: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sti/sti_hdmi.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f3ace11209dd..3f8b04a1407e 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
goto fail;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
- DRM_DEBUG_KMS("%s : %dx%d cm\n",
- (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
- edid->width_cm, edid->height_cm);
cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
count = drm_add_edid_modes(connector, edid);
+
+ /* This updates connector->display_info */
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
+ DRM_DEBUG_KMS("%s : %dx%d cm\n",
+ (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
+ edid->width_cm, edid->height_cm);
kfree(edid);
return count;
--
2.33.0
[PATCH 11/15] drm/zte: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi Signed-off-by: Claudio Suarez --- drivers/gpu/drm/zte/zx_hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c index cd79ca0a92a9..7df682d90723 100644 --- a/drivers/gpu/drm/zte/zx_hdmi.c +++ b/drivers/gpu/drm/zte/zx_hdmi.c @@ -265,9 +265,9 @@ static int zx_hdmi_connector_get_modes(struct drm_connector *connector) if (!edid) return 0; - hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); - hdmi->sink_has_audio = drm_detect_monitor_audio(edid); drm_connector_update_edid_property(connector, edid); + hdmi->sink_is_hdmi = connector->display_info.is_hdmi; + hdmi->sink_has_audio = drm_detect_monitor_audio(edid); ret = drm_add_edid_modes(connector, edid); kfree(edid); -- 2.33.0
[PATCH 12/15] drm/rockchip: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++--
drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7afdc54eb3ec..d479f230833e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
- hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
+ hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 1c546c3a8998..03aaae39cf61 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
--
2.33.0
[PATCH 13/15] drm/bridge: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
drivers/gpu/drm/bridge/sii902x.c | 2 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..f6891280a58d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511
*adv7511,
__adv7511_power_off(adv7511);
adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
- drm_detect_hdmi_monitor(edid));
+ connector->display_info.is_hdmi);
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..5719be0a03c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector
*connector)
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
drm_connector_update_edid_property(connector, edid);
if (edid) {
- if (drm_detect_hdmi_monitor(edid))
+ if (connector->display_info.is_hdmi)
output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
num = drm_add_edid_modes(connector, edid);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..33f0afb6b646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);
- hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+ hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
return edid;
--
2.33.0
[PATCH 14/15] drm/nouveau: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++--
drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.h | 6 ++
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..fadd58b015d6 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct
nouveau_crtc *nv_crtc,
int ret;
int size;
- if (!drm_detect_hdmi_monitor(nv_connector->edid))
+ if (nouveau_connector_is_hdmi_monitor(&nv_connector->base))
return;
hdmi = &nv_connector->base.display_info.hdmi;
@@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
struct drm_atomic_state *sta
*/
if (mode->clock >= 165000 &&
nv_encoder->dcb->duallink_possible &&
- !drm_detect_hdmi_monitor(nv_connector->edid))
+
!nouveau_connector_is_hdmi_monitor(&nv_connector->base))
proto =
NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
} else {
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index d66f97280282..0a138bfb8f32 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
struct drm_display_mode *omode = &asyh->state.adjusted_mode;
struct drm_display_mode *umode = &asyh->state.mode;
int mode = asyc->scaler.mode;
- struct edid *edid;
int umode_vdisplay, omode_hdisplay, omode_vdisplay;
- if (connector->edid_blob_ptr)
- edid = (struct edid *)connector->edid_blob_ptr->data;
- else
- edid = NULL;
-
if (!asyc->scaler.full) {
if (mode == DRM_MODE_SCALE_NONE)
omode = umode;
@@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
*/
if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
(asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
-drm_detect_hdmi_monitor(edid {
+nouveau_connector_is_hdmi_monitor(connector {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22b83a6577eb..211543373b72 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1010,7 +1010,7 @@ get_tmds_link_bandwidth(struct drm_connector *connector)
unsigned duallink_scale =
nouveau_duallink && nv_encoder->dcb->duallink_possible ? 2 : 1;
- if (drm_detect_hdmi_monitor(nv_connector->edid)) {
+ if (nouveau_connector_is_hdmi_monitor(connector)) {
info = &nv_connector->base.display_info;
duallink_scale = 1;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index 40f90e353540..299f3a3b2331 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -167,6 +167,12 @@ nouveau_connector_is_mst(struct drm_connector *connector)
return encoder->encoder_type == DRM_MODE_ENCODER_DPMST;
}
+static inline bool
+nouveau_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+ return connector->display_info.is_hdmi;
+}
+
#define nouveau_for_each_non_mst_connector_iter(connector, iter) \
drm_for_each_connector_iter(connector, iter) \
for_each_if(!nouveau_connector_is_mst(connector))
--
2.33.0
[PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_connector.c | 5 +
drivers/gpu/drm/i915/display/intel_connector.h | 1 +
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 9bed1ccecea0..3346b55df6e1 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
return ret;
}
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+ return connector->display_info.is_hdmi;
+}
+
static const struct drm_prop_enum_list force_audio_names[] = {
{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
{ HDMI_AUDIO_OFF, "off" },
diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
b/drivers/gpu/drm/i915/display/intel_connector.h
index 661a37a3c6d8..ceda6e72ece6 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.h
+++ b/drivers/gpu/drm/i915/display/intel_connector.h
@@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
*connector);
int intel_connector_update_modes(struct drm_connector *connector,
struct edid *edid);
int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
void intel_attach_aspect_ratio_property(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..2b1d7c5bebdd 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink =
intel_connector_is_hdmi_monitor(connector);
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..a32279e4fee8 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
intel_connector_is_hdmi_monitor(connector);
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH 06/15] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..d9db5d52d52e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect(
struct edid *edid = NULL;
enum drm_connector_status status = connector_status_disconnected;
+ /* This updates connector->display_info */
edid = drm_get_edid(connector, &gma_encoder->i2c_bus->adapter);
hdmi_priv->has_hdmi_sink = false;
@@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect(
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
hdmi_priv->has_hdmi_sink =
- drm_detect_hdmi_monitor(edid);
+ connector->display_info.is_hdmi;
hdmi_priv->has_hdmi_audio =
drm_detect_monitor_audio(edid);
}
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 355da2856389..5ef49d17de98 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (psb_intel_sdvo->is_hdmi) {
- psb_intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
- psb_intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ psb_intel_sdvo->has_hdmi_monitor =
+ connector->display_info.is_hdmi;
+ psb_intel_sdvo->has_hdmi_audio =
+ drm_detect_monitor_audio(edid);
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH 07/15] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi Signed-off-by: Claudio Suarez --- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV; - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); + /* This updates connector->display_info */ + drm_connector_update_edid_property(connector, edid); + + hdata->dvi_mode = !connector->display_info.is_hdmi; DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm); - drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid); ret = drm_add_edid_modes(connector, edid); -- 2.33.0
[PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver
a) Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. The amdgpu driver still calls
drm_detect_hdmi_monitor() to retrieve the same information, which
is less efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
b) drm_display_info is updated by drm_get_edid() or
drm_connector_update_edid_property(). In the amdgpu driver it is almost
always updated when the edid is read in amdgpu_connector_get_edid(),
but not always. Change amdgpu_connector_get_edid() and
amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a)
to work properly.
c) Use drm_edid_get_monitor_name() instead of duplicating the code that
parses the EDID in dm_helpers_parse_edid_caps()
Also, remove the unused "struct dc_context *ctx" parameter in
dm_helpers_parse_edid_caps()
Signed-off-by: Claudio Suarez
---
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 +++
.../gpu/drm/amd/amdgpu/amdgpu_connectors.h| 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 4 +-
.../gpu/drm/amd/amdgpu/atombios_encoders.c| 6 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 ++-
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 +-
9 files changed, 39 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index b9c11c2b2885..7b41a1120b70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -25,6 +25,7 @@
*/
#include
+#include
#include
#include
#include
@@ -108,7 +109,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (amdgpu_connector->use_digital) {
- if
(drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -116,7 +117,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
- if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -125,7 +126,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
dig_connector = amdgpu_connector->con_priv;
if ((dig_connector->dp_sink_type ==
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
(dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) ||
- drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ (amdgpu_connector_is_hdmi_monitor(connector))) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -149,7 +150,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
break;
}
- if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
/*
* Pre DCE-8 hw can't handle > 12 bpc, and more than 12 bpc
doesn't make
* much sense without support for > 12 bpc framebuffers. RGB
4:4:4 at
@@ -315,8 +316,10 @@ static void amdgpu_connector_get_edid(struct drm_connector
*connector)
if (!amdgpu_connector->edid) {
/* some laptops provide a hardcoded edid in rom for LCDs */
if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
-(connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
+(connector->connector_type == DRM_MODE_CONNECTOR_eDP))) {
amdgpu_connector->edid =
amdgpu_connector_get_hardcoded_edid(adev);
+ drm_connector_update_edid_property(connector,
amdgpu_connector->edid);
+ }
}
}
@@ -326,6 +329,7 @@ static void amdgpu_connector_free_edid(struct drm_connector
*connector)
kfree(amdgpu_connector->edid);
amdgpu_connector->edid = NULL;
+ drm_connector_update_edid_property(connector, NULL);
}
static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
@@ -1170,7 +1174,7 @@ s
[PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
According to the documentation, drm_add_edid_modes
"... Also fills out the &drm_display_info structure and ELD in @connector
with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_edid.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..6cbe09b2357c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector *connector,
struct edid *edid)
if (edid == NULL) {
clear_eld(connector);
+ drm_reset_display_info(connector);
return 0;
}
if (!drm_edid_is_valid(edid)) {
clear_eld(connector);
+ drm_reset_display_info(connector);
drm_warn(connector->dev, "%s: EDID invalid.\n",
connector->name);
return 0;
--
2.33.0
[PATCH 03/15] drm/vc4: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Use this value instead of calling
drm_detect_hdmi_monitor() to avoid a second parse.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b4b4653fe301..d531e4c501eb 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
+ vc4_hdmi->encoder.hdmi_monitor =
+ connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
+ vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
kfree(edid);
if (vc4_hdmi->disable_4kp60) {
--
2.33.0
Re: [PATCH] fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)
On Wed, Oct 13, 2021 at 04:08:02PM +0200, Thomas Zimmermann wrote: > Hi > > Am 01.10.21 um 14:48 schrieb Claudio Suarez: > > On Fri, Oct 01, 2021 at 10:21:44AM +0200, Thomas Zimmermann wrote: > > > Hi > > > > > > Am 30.09.21 um 17:10 schrieb Claudio: > > > > Scroll acceleration is disabled in fbcon by hard-wiring > > > > p->scrollmode = SCROLL_REDRAW. Remove the obsolete code in fbcon.c > > > > and fbdev/core/ > > > > > > > > Signed-off-by: Claudio Suarez > > > > --- > > > > > > > > - This is a task in the TODO list Documentation/gpu/todo.rst > > > > - The contact in the task is Daniel Vetter. He is/you are in copy. > > > > - To ease the things and saving time, I did a patch. It is included in > > > > this > > > > message. I can redo it if there is something wrong. > > > > - I tested it in some configurations. > > > > > > > > My plan for new patches in this task: > > > > - A buch of patches to remove code from drivers: fb_copyarea and > > > > related. > > > > - Simplify the code around fbcon_ops as much as possible to remove the > > > > hooks > > > > as the TODO suggests. > > > > - Remove fb_copyarea in headers and exported symbols: cfb_copyarea, > > > > etc. This > > > > must be done when all the drivers are changed. > > > > > > > > I think that the correct list to ask questions about this > > > > is linux-fb...@vger.kernel.org . Is it correct ? > > > > My question: I can develop the new changes. I can test in two > > > > computers/two > > > > drivers. Is there a way to test the rest of the patches ? I have not > > > > hardware > > > > to test them. Is anyone helping with this? Only regression tests are > > > > needed. > > > > I can test other patches in return. > > > > > > > > Thank you. > > > > Claudio Suarez. > > > > > > > > Patch follows: > > > > > > > >Documentation/gpu/todo.rst | 13 +- > > > >drivers/video/fbdev/core/bitblit.c | 16 - > > > >drivers/video/fbdev/core/fbcon.c| 509 > > > > ++-- > > > >drivers/video/fbdev/core/fbcon.h| 59 > > > >drivers/video/fbdev/core/fbcon_ccw.c| 28 +- > > > >drivers/video/fbdev/core/fbcon_cw.c | 28 +- > > > >drivers/video/fbdev/core/fbcon_rotate.h | 9 - > > > >drivers/video/fbdev/core/fbcon_ud.c | 37 +-- > > > >drivers/video/fbdev/core/tileblit.c | 16 - > > > >drivers/video/fbdev/skeletonfb.c| 12 +- > > > >include/linux/fb.h | 2 +- > > > >11 files changed, 51 insertions(+), 678 deletions(-) > > > > > > Nice stats :) > > > > > > I looked through it and it looks good. Maybe double-check that everything > > > still builds. > > > > > > Acked-by: Thomas Zimmermann > > > > > > > Yes, it still builds :) > > I had built with some different .config options, including > > allyesconfig, allno, some randoms and debian default config. I tested > > some .config options related to fbdev. I spent time running some kernels > > with different parameters and everything was ok. > > Today, I've just applied the patch to source from two gits: Linus > > rc and drm. Both have built ok. > > I think that I did enough tests to ensure it works fine. This code is going > > to run in many computers, mine included! > > Of course, if you or anyone is worried about something specific, please, > > tell me and I can check and re-check it. I don't want to miss something > > important. > > > > Thank you! > > I added the patch to drm-misc-next. Great! Then I start to work in the second phase of removing unnecessary code from every driver, which depends on this one. Thanks a lot! Best regards, Claudio Suarez > > Best regards > Thomas > > > > > Best regards > > Claudio Suarez > > > > > Best regards > > > Thomas > > > > > > > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > > > index 12e61869939e..bb1e04bbf4fb 100644 > > > > --- a/Documentation/gpu/todo.rst > > > > +++ b/Documentation/gpu/to
Re: [PATCH] fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)
On Fri, Oct 01, 2021 at 10:21:44AM +0200, Thomas Zimmermann wrote: > Hi > > Am 30.09.21 um 17:10 schrieb Claudio: > > Scroll acceleration is disabled in fbcon by hard-wiring > > p->scrollmode = SCROLL_REDRAW. Remove the obsolete code in fbcon.c > > and fbdev/core/ > > > > Signed-off-by: Claudio Suarez > > --- > > > > - This is a task in the TODO list Documentation/gpu/todo.rst > > - The contact in the task is Daniel Vetter. He is/you are in copy. > > - To ease the things and saving time, I did a patch. It is included in this > >message. I can redo it if there is something wrong. > > - I tested it in some configurations. > > > > My plan for new patches in this task: > > - A buch of patches to remove code from drivers: fb_copyarea and related. > > - Simplify the code around fbcon_ops as much as possible to remove the hooks > >as the TODO suggests. > > - Remove fb_copyarea in headers and exported symbols: cfb_copyarea, etc. > > This > >must be done when all the drivers are changed. > > > > I think that the correct list to ask questions about this > > is linux-fb...@vger.kernel.org . Is it correct ? > > My question: I can develop the new changes. I can test in two computers/two > > drivers. Is there a way to test the rest of the patches ? I have not > > hardware > > to test them. Is anyone helping with this? Only regression tests are needed. > > I can test other patches in return. > > > > Thank you. > > Claudio Suarez. > > > > Patch follows: > > > > Documentation/gpu/todo.rst | 13 +- > > drivers/video/fbdev/core/bitblit.c | 16 - > > drivers/video/fbdev/core/fbcon.c| 509 > > ++-- > > drivers/video/fbdev/core/fbcon.h| 59 > > drivers/video/fbdev/core/fbcon_ccw.c| 28 +- > > drivers/video/fbdev/core/fbcon_cw.c | 28 +- > > drivers/video/fbdev/core/fbcon_rotate.h | 9 - > > drivers/video/fbdev/core/fbcon_ud.c | 37 +-- > > drivers/video/fbdev/core/tileblit.c | 16 - > > drivers/video/fbdev/skeletonfb.c| 12 +- > > include/linux/fb.h | 2 +- > > 11 files changed, 51 insertions(+), 678 deletions(-) > > Nice stats :) > > I looked through it and it looks good. Maybe double-check that everything > still builds. > > Acked-by: Thomas Zimmermann > Yes, it still builds :) I had built with some different .config options, including allyesconfig, allno, some randoms and debian default config. I tested some .config options related to fbdev. I spent time running some kernels with different parameters and everything was ok. Today, I've just applied the patch to source from two gits: Linus rc and drm. Both have built ok. I think that I did enough tests to ensure it works fine. This code is going to run in many computers, mine included! Of course, if you or anyone is worried about something specific, please, tell me and I can check and re-check it. I don't want to miss something important. Thank you! Best regards Claudio Suarez > Best regards > Thomas > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 12e61869939e..bb1e04bbf4fb 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -314,16 +314,19 @@ Level: Advanced > > Garbage collect fbdev scrolling acceleration > > > > -Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode = > > -SCROLL_REDRAW. There's a ton of code this will allow us to remove: > > +Scroll acceleration has been disabled in fbcon. Now it works as the old > > +SCROLL_REDRAW mode. A ton of code was removed in fbcon.c and the hook > > bmove was > > +removed from fbcon_ops. > > +Remaining tasks: > > -- lots of code in fbcon.c > > - > > -- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be > > called > > +- a bunch of the hooks in fbcon_ops could be removed or simplified by > > calling > > directly instead of the function table (with a switch on p->rotate) > > - fb_copyarea is unused after this, and can be deleted from all drivers > > +- after that, fb_copyarea can be deleted from fb_ops in include/linux/fb.h > > as > > + well as cfb_copyarea > > + > > Note that not all acceleration code can be deleted, since clearing and > > cursor > > support is still accelerated, which might be good candidates for further > > deletion projects. > >
