All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: kernel test robot <lkp@intel.com>, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, llvm@lists.linux.dev,
	kbuild-all@lists.01.org
Subject: Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
Date: Mon, 11 Apr 2022 16:54:45 +0300	[thread overview]
Message-ID: <878rsbiv2y.fsf@intel.com> (raw)
In-Reply-To: <202204112019.U9iIZWqP-lkp@intel.com>

On Mon, 11 Apr 2022, kernel test robot <lkp@intel.com> wrote:
> Hi Jani,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm-tip/drm-tip]
> [also build test WARNING on next-20220411]
> [cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
> [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/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
> config: i386-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220411/202204112019.U9iIZWqP-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c6e83f560f06cdfe8aa47b248d8bdc58f947274b)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
>         git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
>         # 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=i386 SHELL=/bin/bash drivers/gpu/drm/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/gpu/drm/drm_edid.c:2170:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
>            if (!edid_extension_block_count(edid) == 0)
>                ^                                 ~~
>    drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses after the '!' to evaluate the comparison first
>            if (!edid_extension_block_count(edid) == 0)
>                ^
>                 (                                    )
>    drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses around left hand side expression to silence this warning
>            if (!edid_extension_block_count(edid) == 0)
>                ^
>                (                                )
>    1 warning generated.

Whoops, thanks for the report!

BR,
Jani.

>
>
> vim +2170 drivers/gpu/drm/drm_edid.c
>
>   2112	
>   2113	/**
>   2114	 * drm_do_get_edid - get EDID data using a custom EDID block read function
>   2115	 * @connector: connector we're probing
>   2116	 * @get_edid_block: EDID block read function
>   2117	 * @data: private data passed to the block read function
>   2118	 *
>   2119	 * When the I2C adapter connected to the DDC bus is hidden behind a device that
>   2120	 * exposes a different interface to read EDID blocks this function can be used
>   2121	 * to get EDID data using a custom block read function.
>   2122	 *
>   2123	 * As in the general case the DDC bus is accessible by the kernel at the I2C
>   2124	 * level, drivers must make all reasonable efforts to expose it as an I2C
>   2125	 * adapter and use drm_get_edid() instead of abusing this function.
>   2126	 *
>   2127	 * The EDID may be overridden using debugfs override_edid or firmware EDID
>   2128	 * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
>   2129	 * order. Having either of them bypasses actual EDID reads.
>   2130	 *
>   2131	 * Return: Pointer to valid EDID or NULL if we couldn't find any.
>   2132	 */
>   2133	struct edid *drm_do_get_edid(struct drm_connector *connector,
>   2134				     read_block_fn read_block,
>   2135				     void *context)
>   2136	{
>   2137		enum edid_block_status status;
>   2138		int i, invalid_blocks = 0;
>   2139		struct edid *edid, *new;
>   2140	
>   2141		edid = drm_get_override_edid(connector);
>   2142		if (edid)
>   2143			goto ok;
>   2144	
>   2145		edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
>   2146		if (!edid)
>   2147			return NULL;
>   2148	
>   2149		status = edid_block_read(edid, 0, read_block, context);
>   2150	
>   2151		edid_block_status_print(status, edid, 0);
>   2152	
>   2153		if (status == EDID_BLOCK_READ_FAIL)
>   2154			goto fail;
>   2155	
>   2156		/* FIXME: Clarify what a corrupt EDID actually means. */
>   2157		if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
>   2158			connector->edid_corrupt = false;
>   2159		else
>   2160			connector->edid_corrupt = true;
>   2161	
>   2162		if (!edid_block_status_valid(status, edid_block_tag(edid))) {
>   2163			if (status == EDID_BLOCK_ZERO)
>   2164				connector->null_edid_counter++;
>   2165	
>   2166			connector_bad_edid(connector, edid, 1);
>   2167			goto fail;
>   2168		}
>   2169	
>> 2170		if (!edid_extension_block_count(edid) == 0)
>   2171			goto ok;
>   2172	
>   2173		new = krealloc(edid, edid_size(edid), GFP_KERNEL);
>   2174		if (!new)
>   2175			goto fail;
>   2176		edid = new;
>   2177	
>   2178		for (i = 1; i < edid_block_count(edid); i++) {
>   2179			void *block = (void *)edid_block_data(edid, i);
>   2180	
>   2181			status = edid_block_read(block, i, read_block, context);
>   2182	
>   2183			edid_block_status_print(status, block, i);
>   2184	
>   2185			if (!edid_block_status_valid(status, edid_block_tag(block))) {
>   2186				if (status == EDID_BLOCK_READ_FAIL)
>   2187					goto fail;
>   2188				invalid_blocks++;
>   2189			}
>   2190		}
>   2191	
>   2192		if (invalid_blocks) {
>   2193			connector_bad_edid(connector, edid, edid_block_count(edid));
>   2194	
>   2195			edid = edid_filter_invalid_blocks(edid, invalid_blocks);
>   2196		}
>   2197	
>   2198	ok:
>   2199		return edid;
>   2200	
>   2201	fail:
>   2202		kfree(edid);
>   2203		return NULL;
>   2204	}
>   2205	EXPORT_SYMBOL_GPL(drm_do_get_edid);
>   2206	

-- 
Jani Nikula, Intel Open Source Graphics Center

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@intel.com>
To: kernel test robot <lkp@intel.com>, dri-devel@lists.freedesktop.org
Cc: llvm@lists.linux.dev, kbuild-all@lists.01.org,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
Date: Mon, 11 Apr 2022 16:54:45 +0300	[thread overview]
Message-ID: <878rsbiv2y.fsf@intel.com> (raw)
In-Reply-To: <202204112019.U9iIZWqP-lkp@intel.com>

On Mon, 11 Apr 2022, kernel test robot <lkp@intel.com> wrote:
> Hi Jani,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm-tip/drm-tip]
> [also build test WARNING on next-20220411]
> [cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
> [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/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
> config: i386-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220411/202204112019.U9iIZWqP-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c6e83f560f06cdfe8aa47b248d8bdc58f947274b)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
>         git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
>         # 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=i386 SHELL=/bin/bash drivers/gpu/drm/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/gpu/drm/drm_edid.c:2170:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
>            if (!edid_extension_block_count(edid) == 0)
>                ^                                 ~~
>    drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses after the '!' to evaluate the comparison first
>            if (!edid_extension_block_count(edid) == 0)
>                ^
>                 (                                    )
>    drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses around left hand side expression to silence this warning
>            if (!edid_extension_block_count(edid) == 0)
>                ^
>                (                                )
>    1 warning generated.

Whoops, thanks for the report!

BR,
Jani.

>
>
> vim +2170 drivers/gpu/drm/drm_edid.c
>
>   2112	
>   2113	/**
>   2114	 * drm_do_get_edid - get EDID data using a custom EDID block read function
>   2115	 * @connector: connector we're probing
>   2116	 * @get_edid_block: EDID block read function
>   2117	 * @data: private data passed to the block read function
>   2118	 *
>   2119	 * When the I2C adapter connected to the DDC bus is hidden behind a device that
>   2120	 * exposes a different interface to read EDID blocks this function can be used
>   2121	 * to get EDID data using a custom block read function.
>   2122	 *
>   2123	 * As in the general case the DDC bus is accessible by the kernel at the I2C
>   2124	 * level, drivers must make all reasonable efforts to expose it as an I2C
>   2125	 * adapter and use drm_get_edid() instead of abusing this function.
>   2126	 *
>   2127	 * The EDID may be overridden using debugfs override_edid or firmware EDID
>   2128	 * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
>   2129	 * order. Having either of them bypasses actual EDID reads.
>   2130	 *
>   2131	 * Return: Pointer to valid EDID or NULL if we couldn't find any.
>   2132	 */
>   2133	struct edid *drm_do_get_edid(struct drm_connector *connector,
>   2134				     read_block_fn read_block,
>   2135				     void *context)
>   2136	{
>   2137		enum edid_block_status status;
>   2138		int i, invalid_blocks = 0;
>   2139		struct edid *edid, *new;
>   2140	
>   2141		edid = drm_get_override_edid(connector);
>   2142		if (edid)
>   2143			goto ok;
>   2144	
>   2145		edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
>   2146		if (!edid)
>   2147			return NULL;
>   2148	
>   2149		status = edid_block_read(edid, 0, read_block, context);
>   2150	
>   2151		edid_block_status_print(status, edid, 0);
>   2152	
>   2153		if (status == EDID_BLOCK_READ_FAIL)
>   2154			goto fail;
>   2155	
>   2156		/* FIXME: Clarify what a corrupt EDID actually means. */
>   2157		if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
>   2158			connector->edid_corrupt = false;
>   2159		else
>   2160			connector->edid_corrupt = true;
>   2161	
>   2162		if (!edid_block_status_valid(status, edid_block_tag(edid))) {
>   2163			if (status == EDID_BLOCK_ZERO)
>   2164				connector->null_edid_counter++;
>   2165	
>   2166			connector_bad_edid(connector, edid, 1);
>   2167			goto fail;
>   2168		}
>   2169	
>> 2170		if (!edid_extension_block_count(edid) == 0)
>   2171			goto ok;
>   2172	
>   2173		new = krealloc(edid, edid_size(edid), GFP_KERNEL);
>   2174		if (!new)
>   2175			goto fail;
>   2176		edid = new;
>   2177	
>   2178		for (i = 1; i < edid_block_count(edid); i++) {
>   2179			void *block = (void *)edid_block_data(edid, i);
>   2180	
>   2181			status = edid_block_read(block, i, read_block, context);
>   2182	
>   2183			edid_block_status_print(status, block, i);
>   2184	
>   2185			if (!edid_block_status_valid(status, edid_block_tag(block))) {
>   2186				if (status == EDID_BLOCK_READ_FAIL)
>   2187					goto fail;
>   2188				invalid_blocks++;
>   2189			}
>   2190		}
>   2191	
>   2192		if (invalid_blocks) {
>   2193			connector_bad_edid(connector, edid, edid_block_count(edid));
>   2194	
>   2195			edid = edid_filter_invalid_blocks(edid, invalid_blocks);
>   2196		}
>   2197	
>   2198	ok:
>   2199		return edid;
>   2200	
>   2201	fail:
>   2202		kfree(edid);
>   2203		return NULL;
>   2204	}
>   2205	EXPORT_SYMBOL_GPL(drm_do_get_edid);
>   2206	

-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2022-04-11 13:54 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-11  9:47 [Intel-gfx] [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
2022-04-11  9:47 ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 02/12] drm/edid: have edid_block_check() detect blocks that are all zero Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 03/12] drm/edid: refactor EDID block status printing Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 04/12] drm/edid: add a helper to log dump an EDID block Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 05/12] drm/edid: pass struct edid to connector_bad_edid() Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 06/12] drm/edid: add typedef for block read function Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 07/12] drm/edid: abstract an EDID block read helper Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 08/12] drm/edid: use EDID block read helper in drm_do_get_edid() Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 09/12] drm/edid: convert extension block read to EDID block read helper Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 10/12] drm/edid: drop extra local var Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 11/12] drm/edid: add single point of return to drm_do_get_edid() Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11  9:47 ` [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
2022-04-11  9:47   ` Jani Nikula
2022-04-11 12:14   ` [Intel-gfx] " kernel test robot
2022-04-11 13:57     ` Jani Nikula
2022-04-11 12:24   ` kernel test robot
2022-04-11 12:24     ` kernel test robot
2022-04-11 13:54     ` Jani Nikula [this message]
2022-04-11 13:54       ` Jani Nikula
2022-04-11 12:11 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/edid: low level EDID block read refactoring etc. (rev4) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878rsbiv2y.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=kbuild-all@lists.01.org \
    --cc=lkp@intel.com \
    --cc=llvm@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.