| Message ID | 1541407715-5417-1-git-send-email-cniedermaier@dh-electronics.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Docs/EDID: Fixed and improved EDID documentation | expand |
On Mon, 5 Nov 2018 09:48:33 +0100 Christoph Niedermaier <cniedermaier@dh-electronics.de> wrote: > A problem was found when EDID data sets for displays other > than the provided samples were generated. The patch series has > no effect on the provided samples that still match the data > used in drivers/gpu/drm/drm_edid_load.c. > The provided samples use small values for XOFFSET, XPULSE, > YOFFSET and YPULSE, where the error doesn't occur. This fix > corrects the use of that values in case of high values, because > the most significant bits were treated incorrectly. > > The previous version made it necessary to first generate an > EDID data set without correct CRC and then to fix the CRC in > a second step. This patch series adds the CRC calculation to the > makefile in such a way that a correct EDID data set is generated > in a single build step. This seems reasonable, I guess; I've applied both. It seems to me, though, that this stuff is in the wrong place. Perhaps we should go one step further and move it to tools/ ? Thanks, jon
On Tue, 06 Nov 2018, Jonathan Corbet <corbet@lwn.net> wrote: > On Mon, 5 Nov 2018 09:48:33 +0100 > Christoph Niedermaier <cniedermaier@dh-electronics.de> wrote: > >> A problem was found when EDID data sets for displays other >> than the provided samples were generated. The patch series has >> no effect on the provided samples that still match the data >> used in drivers/gpu/drm/drm_edid_load.c. >> The provided samples use small values for XOFFSET, XPULSE, >> YOFFSET and YPULSE, where the error doesn't occur. This fix >> corrects the use of that values in case of high values, because >> the most significant bits were treated incorrectly. >> >> The previous version made it necessary to first generate an >> EDID data set without correct CRC and then to fix the CRC in >> a second step. This patch series adds the CRC calculation to the >> makefile in such a way that a correct EDID data set is generated >> in a single build step. > > This seems reasonable, I guess; I've applied both. It seems to me, though, > that this stuff is in the wrong place. Perhaps we should go one step > further and move it to tools/ ? And then the next step further would be to write a tool in a high level language to generate the data rather than assemble the binary. Such a tool would, of course, catch errors like the ones fixed by this patch. BR, Jani.
Hi Jon, On Tue, Nov 6, 2018, Jonathan Corbet wrote: > This seems reasonable, I guess; I've applied both. It seems to me, though, > that this stuff is in the wrong place. Perhaps we should go one step > further and move it to tools/ ? I spoke to Carsten and his intention apparently was to merely document how the somewhat cryptic data structures in drivers/gpu/drm/drm_edid_load.c were derived. The code needed to be run only once to generate the data and never again. So it not really is a tool that someone may want to run from time to time for a particular purpose. So we would propose to leave the EDID directory where it currently is. However, should you feel that it would better be placed in the tools directory then you certainly may do that. Best regards, Christoph
On Tue, 13 Nov 2018, Jani Nikula wrote: > On Tue, 06 Nov 2018, Jonathan Corbet <corbet@lwn.net> wrote: >> On Mon, 5 Nov 2018 09:48:33 +0100 >> Christoph Niedermaier <cniedermaier@dh-electronics.de> wrote: >> >>> A problem was found when EDID data sets for displays other than the >>> provided samples were generated. The patch series has no effect on >>> the provided samples that still match the data used in >>> drivers/gpu/drm/drm_edid_load.c. >>> The provided samples use small values for XOFFSET, XPULSE, YOFFSET >>> and YPULSE, where the error doesn't occur. This fix corrects the use >>> of that values in case of high values, because the most significant >>> bits were treated incorrectly. >>> >>> The previous version made it necessary to first generate an EDID data >>> set without correct CRC and then to fix the CRC in a second step. >>> This patch series adds the CRC calculation to the makefile in such a >>> way that a correct EDID data set is generated in a single build step. >> >> This seems reasonable, I guess; I've applied both. It seems to me, >> though, that this stuff is in the wrong place. Perhaps we should go >> one step further and move it to tools/ ? > > And then the next step further would be to write a tool in a high level language to generate the data rather than assemble the binary. Such a tool would, of course, catch errors like the ones fixed by this patch. It would be nice, but for now it works. Speaking for me such a task has low priority, because in my opinion this code is not often used and it is more intended for documentation purpose. Best regards, Christoph