Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
Quoting dom.const...@free.fr (2018-02-06 18:57:05) > > > > Quoting dom.const...@free.fr (2018-02-02 18:37:12) > > > Hello, > > > > > > For my HTPC setup, I'm using the option "CustomEDID". > > > With this option, output attaching and destroying events leads to > > > crashes. > > > > > > The following sequence leads to a crash: > > > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" > > > - Starting Xorg > > > - Connect HDMI2 > > > - Disconnect HDMI2 > > > - Reconnect HDMI2 > > > -> Crash > > > > > > > > > The crash happens in xf86OutputSetEDID > > > (xorg/xserver/hw/xfree86/modes/xf86Crtc.c) > > > at "free(output->MonInfo)". MonInfo is assigned with > > > sna_output->fake_edid_mon > > > which is allocated by intel driver in sna_output_load_fake_edid > > > (src/sna/sna_display.c). > > > > > > > > > Sequence details: > > > - Starting Xorg > > >-> fake_edid_mon is initialized > > > > > > - Connect HDMI2 > > >-> xf86OutputSetEDID is called: > > >- MonInfo is NULL > > >- MonInfo is assigned with fake_edid_mon pointer > > >- MonInfo is read by Xorg > > > > > > - Disconnect HDMI2 > > > > > > - Reconnect HDMI2 > > >-> xf86OutputSetEDID is called: > > >- MonInfo is freed thus also fake_edid_mon > > >- MonInfo is assigned with fake_edid_mon > > >- MonInfo is read but it was freed -> CRASH > > > > > > > > > The fix consists of a new instance of xf86MonPtr for each calls of > > > xf86OutputSetEDID. > > > This instance is initialized with fake_edid_raw which render > > > fake_edid_mon useless. > > > With this proposal, the behaviour of an EDID override is similar to > > > a "real" EDID. > > > > > > Regards, > > > > > > > > > Signed-off-by: Dominique Constant> > > To: Chris Wilson > > > > Apologies if this is a resend, but could you send me this patch using > > "git send-email" (or attach the output of "git format-patch"). git am > > is not happy with it. > > -Chris > > > > > Hello, > Thank you for taking into consideration this patch. Finally, got around to picking up patches for -intel. Thank you very much for the fix, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
> Quoting dom.const...@free.fr (2018-02-02 18:37:12) > > Hello, > > > > For my HTPC setup, I'm using the option "CustomEDID". > > With this option, output attaching and destroying events leads to > > crashes. > > > > The following sequence leads to a crash: > > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" > > - Starting Xorg > > - Connect HDMI2 > > - Disconnect HDMI2 > > - Reconnect HDMI2 > > -> Crash > > > > > > The crash happens in xf86OutputSetEDID > > (xorg/xserver/hw/xfree86/modes/xf86Crtc.c) > > at "free(output->MonInfo)". MonInfo is assigned with > > sna_output->fake_edid_mon > > which is allocated by intel driver in sna_output_load_fake_edid > > (src/sna/sna_display.c). > > > > > > Sequence details: > > - Starting Xorg > >-> fake_edid_mon is initialized > > > > - Connect HDMI2 > >-> xf86OutputSetEDID is called: > >- MonInfo is NULL > >- MonInfo is assigned with fake_edid_mon pointer > >- MonInfo is read by Xorg > > > > - Disconnect HDMI2 > > > > - Reconnect HDMI2 > >-> xf86OutputSetEDID is called: > >- MonInfo is freed thus also fake_edid_mon > >- MonInfo is assigned with fake_edid_mon > >- MonInfo is read but it was freed -> CRASH > > > > > > The fix consists of a new instance of xf86MonPtr for each calls of > > xf86OutputSetEDID. > > This instance is initialized with fake_edid_raw which render > > fake_edid_mon useless. > > With this proposal, the behaviour of an EDID override is similar to > > a "real" EDID. > > > > Regards, > > > > > > Signed-off-by: Dominique Constant> > To: Chris Wilson > > Apologies if this is a resend, but could you send me this patch using > "git send-email" (or attach the output of "git format-patch"). git am > is not happy with it. > -Chris > Hello, Thank you for taking into consideration this patch. Regards, Dominique CONSTANT Signed-off-by: Dominique Constant --- src/sna/sna_display.c | 28 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index ea2f148d..ad805c2f 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -263,7 +263,6 @@ struct sna_output { uint32_t edid_blob_id; uint32_t edid_len; void *edid_raw; - xf86MonPtr fake_edid_mon; void *fake_edid_raw; bool has_panel_limits; @@ -4102,13 +4101,21 @@ static DisplayModePtr sna_output_override_edid(xf86OutputPtr output) { struct sna_output *sna_output = output->driver_private; + xf86MonPtr mon = NULL; + + if (sna_output->fake_edid_raw == NULL) + return NULL; - if (sna_output->fake_edid_mon == NULL) + mon = xf86InterpretEDID(output->scrn->scrnIndex, sna_output->fake_edid_raw); + if (mon == NULL) { return NULL; + } + + mon->flags |= MONITOR_EDID_COMPLETE_RAWDATA; + + xf86OutputSetEDID(output, mon); - xf86OutputSetEDID(output, sna_output->fake_edid_mon); - return xf86DDCGetModes(output->scrn->scrnIndex, - sna_output->fake_edid_mon); + return xf86DDCGetModes(output->scrn->scrnIndex, mon); } static DisplayModePtr @@ -4896,7 +4903,6 @@ sna_output_load_fake_edid(xf86OutputPtr output) FILE *file; void *raw; int size; - xf86MonPtr mon; filename = fake_edid_name(output); if (filename == NULL) @@ -4928,16 +4934,6 @@ sna_output_load_fake_edid(xf86OutputPtr output) } fclose(file); - mon = xf86InterpretEDID(output->scrn->scrnIndex, raw); - if (mon == NULL) { - free(raw); - goto err; - } - - if (mon && size > 128) - mon->flags |= MONITOR_EDID_COMPLETE_RAWDATA; - - sna_output->fake_edid_mon = mon; sna_output->fake_edid_raw = raw; xf86DrvMsg(output->scrn->scrnIndex, X_CONFIG, -- 2.15.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
Quoting dom.const...@free.fr (2018-02-02 18:37:12) > Hello, > > For my HTPC setup, I'm using the option "CustomEDID". > With this option, output attaching and destroying events leads to crashes. > > The following sequence leads to a crash: > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" > - Starting Xorg > - Connect HDMI2 > - Disconnect HDMI2 > - Reconnect HDMI2 > -> Crash > > > The crash happens in xf86OutputSetEDID > (xorg/xserver/hw/xfree86/modes/xf86Crtc.c) > at "free(output->MonInfo)". MonInfo is assigned with > sna_output->fake_edid_mon > which is allocated by intel driver in sna_output_load_fake_edid > (src/sna/sna_display.c). > > > Sequence details: > - Starting Xorg >-> fake_edid_mon is initialized > > - Connect HDMI2 >-> xf86OutputSetEDID is called: >- MonInfo is NULL >- MonInfo is assigned with fake_edid_mon pointer >- MonInfo is read by Xorg > > - Disconnect HDMI2 > > - Reconnect HDMI2 >-> xf86OutputSetEDID is called: >- MonInfo is freed thus also fake_edid_mon >- MonInfo is assigned with fake_edid_mon >- MonInfo is read but it was freed -> CRASH > > > The fix consists of a new instance of xf86MonPtr for each calls of > xf86OutputSetEDID. > This instance is initialized with fake_edid_raw which render fake_edid_mon > useless. > With this proposal, the behaviour of an EDID override is similar to a "real" > EDID. > > Regards, > > > Signed-off-by: Dominique Constant> To: Chris Wilson Apologies if this is a resend, but could you send me this patch using "git send-email" (or attach the output of "git format-patch"). git am is not happy with it. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
Quoting Jani Nikula (2018-02-02 19:13:53) > On Fri, 02 Feb 2018, dom.const...@free.fr wrote: > > For my HTPC setup, I'm using the option "CustomEDID". > > With this option, output attaching and destroying events leads to crashes. > > > > The following sequence leads to a crash: > > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" > > I know nothing about xorg customedid stuff. > > But there's drm.edid_firmware=HDMI-2:/etc/my_edid.bin (or > drm_kms_helper.edid_firmware on older kernels) module parameter for the > kernel that does this transparently across the stack. There's also the difference in that Xorg only uses the custom EDID to conveniently load a set of modelines, but the kernel EDID override describes the sink completely (e.g. audio modes, link protocols, pixel formats etc). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
Quoting dom.const...@free.fr (2018-02-02 18:37:12) > Hello, > > For my HTPC setup, I'm using the option "CustomEDID". > With this option, output attaching and destroying events leads to crashes. > > The following sequence leads to a crash: > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" > - Starting Xorg > - Connect HDMI2 > - Disconnect HDMI2 > - Reconnect HDMI2 > -> Crash > > > The crash happens in xf86OutputSetEDID > (xorg/xserver/hw/xfree86/modes/xf86Crtc.c) > at "free(output->MonInfo)". MonInfo is assigned with > sna_output->fake_edid_mon > which is allocated by intel driver in sna_output_load_fake_edid > (src/sna/sna_display.c). Ho hum, looks like I never tested it more than a quick start of Xorg. Sorry about that :( > Sequence details: > - Starting Xorg >-> fake_edid_mon is initialized > > - Connect HDMI2 >-> xf86OutputSetEDID is called: >- MonInfo is NULL >- MonInfo is assigned with fake_edid_mon pointer >- MonInfo is read by Xorg > > - Disconnect HDMI2 > > - Reconnect HDMI2 >-> xf86OutputSetEDID is called: >- MonInfo is freed thus also fake_edid_mon >- MonInfo is assigned with fake_edid_mon >- MonInfo is read but it was freed -> CRASH > > > The fix consists of a new instance of xf86MonPtr for each calls of > xf86OutputSetEDID. > This instance is initialized with fake_edid_raw which render fake_edid_mon > useless. > With this proposal, the behaviour of an EDID override is similar to a "real" > EDID. > > Regards, > > > Signed-off-by: Dominique Constant> To: Chris Wilson Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna: CustomEDID fix
On Fri, 02 Feb 2018, dom.const...@free.fr wrote: > For my HTPC setup, I'm using the option "CustomEDID". > With this option, output attaching and destroying events leads to crashes. > > The following sequence leads to a crash: > - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" I know nothing about xorg customedid stuff. But there's drm.edid_firmware=HDMI-2:/etc/my_edid.bin (or drm_kms_helper.edid_firmware on older kernels) module parameter for the kernel that does this transparently across the stack. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] sna: CustomEDID fix
Hello, For my HTPC setup, I'm using the option "CustomEDID". With this option, output attaching and destroying events leads to crashes. The following sequence leads to a crash: - In xorg.conf: Option "CustomEDID" "HDMI2:/etc/my_edid.bin" - Starting Xorg - Connect HDMI2 - Disconnect HDMI2 - Reconnect HDMI2 -> Crash The crash happens in xf86OutputSetEDID (xorg/xserver/hw/xfree86/modes/xf86Crtc.c) at "free(output->MonInfo)". MonInfo is assigned with sna_output->fake_edid_mon which is allocated by intel driver in sna_output_load_fake_edid (src/sna/sna_display.c). Sequence details: - Starting Xorg -> fake_edid_mon is initialized - Connect HDMI2 -> xf86OutputSetEDID is called: - MonInfo is NULL - MonInfo is assigned with fake_edid_mon pointer - MonInfo is read by Xorg - Disconnect HDMI2 - Reconnect HDMI2 -> xf86OutputSetEDID is called: - MonInfo is freed thus also fake_edid_mon - MonInfo is assigned with fake_edid_mon - MonInfo is read but it was freed -> CRASH The fix consists of a new instance of xf86MonPtr for each calls of xf86OutputSetEDID. This instance is initialized with fake_edid_raw which render fake_edid_mon useless. With this proposal, the behaviour of an EDID override is similar to a "real" EDID. Regards, Signed-off-by: Dominique ConstantTo: Chris Wilson diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index ea2f148d..ad805c2f 100644 --- a/src/sna/sna_display.c +++ b/src/sna/sna_display.c @@ -263,7 +263,6 @@ struct sna_output { uint32_t edid_blob_id; uint32_t edid_len; void *edid_raw; -xf86MonPtr fake_edid_mon; void *fake_edid_raw; bool has_panel_limits; @@ -4102,13 +4101,21 @@ static DisplayModePtr sna_output_override_edid(xf86OutputPtr output) { struct sna_output *sna_output = output->driver_private; +xf86MonPtr mon = NULL; + +if (sna_output->fake_edid_raw == NULL) +return NULL; -if (sna_output->fake_edid_mon == NULL) +mon = xf86InterpretEDID(output->scrn->scrnIndex, sna_output->fake_edid_raw); +if (mon == NULL) { return NULL; +} + +mon->flags |= MONITOR_EDID_COMPLETE_RAWDATA; + +xf86OutputSetEDID(output, mon); -xf86OutputSetEDID(output, sna_output->fake_edid_mon); -return xf86DDCGetModes(output->scrn->scrnIndex, - sna_output->fake_edid_mon); +return xf86DDCGetModes(output->scrn->scrnIndex, mon); } static DisplayModePtr @@ -4896,7 +4903,6 @@ sna_output_load_fake_edid(xf86OutputPtr output) FILE *file; void *raw; int size; -xf86MonPtr mon; filename = fake_edid_name(output); if (filename == NULL) @@ -4928,16 +4934,6 @@ sna_output_load_fake_edid(xf86OutputPtr output) } fclose(file); -mon = xf86InterpretEDID(output->scrn->scrnIndex, raw); -if (mon == NULL) { -free(raw); -goto err; -} - -if (mon && size > 128) -mon->flags |= MONITOR_EDID_COMPLETE_RAWDATA; - -sna_output->fake_edid_mon = mon; sna_output->fake_edid_raw = raw; xf86DrvMsg(output->scrn->scrnIndex, X_CONFIG, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx