| Message ID | 20230713064037.315387-2-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/2] drm/ast: Add BMC virtual connector | expand |
Hi Jocelyn, Thanks for your work. On 2023/7/13 下午 02:40, Jocelyn Falempe wrote: > Aspeed always report the display port as "connected", because it > doesn't set a .detect_ctx callback. > Fix this by providing the proper detect callback for astdp and dp501. > > This also fixes the following regression: > Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no > EDID on DP") The default resolution is now 640x480 when no monitor is > connected. But Aspeed graphics is mostly used in servers, where no monitor > is attached. This also affects the remote BMC resolution to 640x480, which > is inconvenient, and breaks the anaconda installer. > > v2: Add .detect callback to the dp/dp501 connector (Jani Nikula) > v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann) > Add a BMC virtual connector > v4: Better indent detect_ctx() functions (Thomas Zimmermann) > v5: Enable polling of the dp and dp501 connector status > (Thomas Zimmermann) > > Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP") > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/ast/ast_dp.c | 11 ++++++++++ > drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++----------- > drivers/gpu/drm/ast/ast_drv.h | 2 ++ > drivers/gpu/drm/ast/ast_mode.c | 31 +++++++++++++++++++++++++-- > 4 files changed, 67 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c > index 6dc1a09504e1..bf78f3d4aa3f 100644 > --- a/drivers/gpu/drm/ast/ast_dp.c > +++ b/drivers/gpu/drm/ast/ast_dp.c > @@ -7,6 +7,17 @@ > #include <drm/drm_print.h> > #include "ast_drv.h" > > +bool ast_astdp_is_connected(struct ast_device *ast) > +{ > + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, ASTDP_MCU_FW_EXECUTING)) > + return false; > + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, ASTDP_LINK_SUCCESS)) > + return false; > + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD)) > + return false; > + return true; > +} * ASPDP_LINK_SUCCESS is true, when DP link training is finished. The DP link quality is good enough to deliver video data. * ASTDP_HPD is true, when there is DP sink pull high HPD. Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would suggest to remove the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for connected status. If you want to check all these three status, please change the order, FW_EXECUTING -> HPD -> LINK_SUCCESS. > + > int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata) > { > struct ast_device *ast = to_ast_device(dev); > diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c > index a5d285850ffb..f10d53b0c94f 100644 > --- a/drivers/gpu/drm/ast/ast_dp501.c > +++ b/drivers/gpu/drm/ast/ast_dp501.c > @@ -272,11 +272,9 @@ static bool ast_launch_m68k(struct drm_device *dev) > return true; > } > > -bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) > +bool ast_dp501_is_connected(struct ast_device *ast) > { > - struct ast_device *ast = to_ast_device(dev); > - u32 i, boot_address, offset, data; > - u32 *pEDIDidx; > + u32 boot_address, offset, data; > > if (ast->config_mode == ast_use_p2a) { > boot_address = get_fw_base(ast); > @@ -292,14 +290,6 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) > data = ast_mindwm(ast, boot_address + offset); > if (!(data & AST_DP501_PNP_CONNECTED)) > return false; > - > - /* Read EDID */ > - offset = AST_DP501_EDID_DATA; > - for (i = 0; i < 128; i += 4) { > - data = ast_mindwm(ast, boot_address + offset + i); > - pEDIDidx = (u32 *)(ediddata + i); > - *pEDIDidx = data; > - } > } else { > if (!ast->dp501_fw_buf) > return false; > @@ -319,7 +309,30 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) > data = readl(ast->dp501_fw_buf + offset); > if (!(data & AST_DP501_PNP_CONNECTED)) > return false; > + } > + return true; > +} > + > +bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) > +{ > + struct ast_device *ast = to_ast_device(dev); > + u32 i, boot_address, offset, data; > + u32 *pEDIDidx; > + > + if (!ast_dp501_is_connected(ast)) > + return false; > + > + if (ast->config_mode == ast_use_p2a) { > + boot_address = get_fw_base(ast); > > + /* Read EDID */ > + offset = AST_DP501_EDID_DATA; > + for (i = 0; i < 128; i += 4) { > + data = ast_mindwm(ast, boot_address + offset + i); > + pEDIDidx = (u32 *)(ediddata + i); > + *pEDIDidx = data; > + } > + } else { > /* Read EDID */ > offset = AST_DP501_EDID_DATA; > for (i = 0; i < 128; i += 4) { > diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h > index c9659e72002f..848a9f1403e8 100644 > --- a/drivers/gpu/drm/ast/ast_drv.h > +++ b/drivers/gpu/drm/ast/ast_drv.h > @@ -514,6 +514,7 @@ void ast_patch_ahb_2500(struct ast_device *ast); > /* ast dp501 */ > void ast_set_dp501_video_output(struct drm_device *dev, u8 mode); > bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); > +bool ast_dp501_is_connected(struct ast_device *ast); > bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata); > u8 ast_get_dp501_max_clk(struct drm_device *dev); > void ast_init_3rdtx(struct drm_device *dev); > @@ -522,6 +523,7 @@ void ast_init_3rdtx(struct drm_device *dev); > struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev); > > /* aspeed DP */ > +bool ast_astdp_is_connected(struct ast_device *ast); > int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata); > void ast_dp_launch(struct drm_device *dev); > void ast_dp_power_on_off(struct drm_device *dev, bool no); > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > index 1a8293162fec..e7f36ec73817 100644 > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -1582,8 +1582,21 @@ static int ast_dp501_connector_helper_get_modes(struct drm_connector *connector) > return 0; > } > > +static int ast_dp501_connector_helper_detect_ctx(struct drm_connector *connector, > + struct drm_modeset_acquire_ctx *ctx, > + bool force) > +{ > + struct ast_device *ast = to_ast_device(connector->dev); > + > + if (ast_dp501_is_connected(ast)) > + return connector_status_connected; > + return connector_status_disconnected; > +} > + > + > static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs = { > .get_modes = ast_dp501_connector_helper_get_modes, > + .detect_ctx = ast_dp501_connector_helper_detect_ctx, > }; > > static const struct drm_connector_funcs ast_dp501_connector_funcs = { > @@ -1608,7 +1621,7 @@ static int ast_dp501_connector_init(struct drm_device *dev, struct drm_connector > connector->interlace_allowed = 0; > connector->doublescan_allowed = 0; > > - connector->polled = DRM_CONNECTOR_POLL_CONNECT; > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; > > return 0; > } > @@ -1680,8 +1693,20 @@ static int ast_astdp_connector_helper_get_modes(struct drm_connector *connector) > return 0; > } > > +static int ast_astdp_connector_helper_detect_ctx(struct drm_connector *connector, > + struct drm_modeset_acquire_ctx *ctx, > + bool force) > +{ > + struct ast_device *ast = to_ast_device(connector->dev); > + > + if (ast_astdp_is_connected(ast)) > + return connector_status_connected; > + return connector_status_disconnected; > +} > + > static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs = { > .get_modes = ast_astdp_connector_helper_get_modes, > + .detect_ctx = ast_astdp_connector_helper_detect_ctx, > }; > > static const struct drm_connector_funcs ast_astdp_connector_funcs = { > @@ -1706,7 +1731,7 @@ static int ast_astdp_connector_init(struct drm_device *dev, struct drm_connector > connector->interlace_allowed = 0; > connector->doublescan_allowed = 0; > > - connector->polled = DRM_CONNECTOR_POLL_CONNECT; > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; > > return 0; > } > @@ -1903,5 +1928,7 @@ int ast_mode_config_init(struct ast_device *ast) > > drm_mode_config_reset(dev); > > + drm_kms_helper_poll_init(dev); > + > return 0; > }
On 13/07/2023 09:00, Jammy Huang wrote: > Hi Jocelyn, > > Thanks for your work. > > On 2023/7/13 下午 02:40, Jocelyn Falempe wrote: >> Aspeed always report the display port as "connected", because it >> doesn't set a .detect_ctx callback. >> Fix this by providing the proper detect callback for astdp and dp501. >> >> This also fixes the following regression: >> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no >> EDID on DP") The default resolution is now 640x480 when no monitor is >> connected. But Aspeed graphics is mostly used in servers, where no >> monitor >> is attached. This also affects the remote BMC resolution to 640x480, >> which >> is inconvenient, and breaks the anaconda installer. >> >> v2: Add .detect callback to the dp/dp501 connector (Jani Nikula) >> v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann) >> Add a BMC virtual connector >> v4: Better indent detect_ctx() functions (Thomas Zimmermann) >> v5: Enable polling of the dp and dp501 connector status >> (Thomas Zimmermann) >> >> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID >> on DP") >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/ast/ast_dp.c | 11 ++++++++++ >> drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++----------- >> drivers/gpu/drm/ast/ast_drv.h | 2 ++ >> drivers/gpu/drm/ast/ast_mode.c | 31 +++++++++++++++++++++++++-- >> 4 files changed, 67 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c >> index 6dc1a09504e1..bf78f3d4aa3f 100644 >> --- a/drivers/gpu/drm/ast/ast_dp.c >> +++ b/drivers/gpu/drm/ast/ast_dp.c >> @@ -7,6 +7,17 @@ >> #include <drm/drm_print.h> >> #include "ast_drv.h" >> +bool ast_astdp_is_connected(struct ast_device *ast) >> +{ >> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, >> ASTDP_MCU_FW_EXECUTING)) >> + return false; >> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, >> ASTDP_LINK_SUCCESS)) >> + return false; >> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD)) >> + return false; >> + return true; >> +} > > * ASPDP_LINK_SUCCESS is true, when DP link training is finished. The DP > link quality is good > > enough to deliver video data. > > * ASTDP_HPD is true, when there is DP sink pull high HPD. > > Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would > suggest to remove > > the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for > connected status. > > If you want to check all these three status, please change the order, > FW_EXECUTING -> HPD -> > > LINK_SUCCESS. Thanks for the detailed explanations. I looked at other drivers to see if HPD is good enough for "connected" status, but I didn't find a clear answer. There is also a drm_link_status, but that looks to be mostly unused. https://elixir.bootlin.com/linux/latest/source/include/drm/drm_connector.h#L331 So I think I will follow your suggestion, and remove the check for ASTDP_LINK_SUCCESS. For the BMC connector patch, you know if there is a register setting I can check to see if a BMC is present or not ? Best regards,
On 13/07/2023 10:17, Jocelyn Falempe wrote: > On 13/07/2023 09:00, Jammy Huang wrote: >> Hi Jocelyn, >> >> Thanks for your work. >> >> On 2023/7/13 下午 02:40, Jocelyn Falempe wrote: >>> Aspeed always report the display port as "connected", because it >>> doesn't set a .detect_ctx callback. >>> Fix this by providing the proper detect callback for astdp and dp501. >>> >>> This also fixes the following regression: >>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no >>> EDID on DP") The default resolution is now 640x480 when no monitor is >>> connected. But Aspeed graphics is mostly used in servers, where no >>> monitor >>> is attached. This also affects the remote BMC resolution to 640x480, >>> which >>> is inconvenient, and breaks the anaconda installer. >>> >>> v2: Add .detect callback to the dp/dp501 connector (Jani Nikula) >>> v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann) >>> Add a BMC virtual connector >>> v4: Better indent detect_ctx() functions (Thomas Zimmermann) >>> v5: Enable polling of the dp and dp501 connector status >>> (Thomas Zimmermann) >>> >>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID >>> on DP") >>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/gpu/drm/ast/ast_dp.c | 11 ++++++++++ >>> drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++----------- >>> drivers/gpu/drm/ast/ast_drv.h | 2 ++ >>> drivers/gpu/drm/ast/ast_mode.c | 31 +++++++++++++++++++++++++-- >>> 4 files changed, 67 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c >>> index 6dc1a09504e1..bf78f3d4aa3f 100644 >>> --- a/drivers/gpu/drm/ast/ast_dp.c >>> +++ b/drivers/gpu/drm/ast/ast_dp.c >>> @@ -7,6 +7,17 @@ >>> #include <drm/drm_print.h> >>> #include "ast_drv.h" >>> +bool ast_astdp_is_connected(struct ast_device *ast) >>> +{ >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, >>> ASTDP_MCU_FW_EXECUTING)) >>> + return false; >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, >>> ASTDP_LINK_SUCCESS)) >>> + return false; >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, >>> ASTDP_HPD)) >>> + return false; >>> + return true; >>> +} >> >> * ASPDP_LINK_SUCCESS is true, when DP link training is finished. The >> DP link quality is good >> >> enough to deliver video data. >> >> * ASTDP_HPD is true, when there is DP sink pull high HPD. >> >> Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would >> suggest to remove >> >> the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for >> connected status. >> >> If you want to check all these three status, please change the order, >> FW_EXECUTING -> HPD -> >> >> LINK_SUCCESS. > > Thanks for the detailed explanations. > I looked at other drivers to see if HPD is good enough for "connected" > status, but I didn't find a clear answer. > There is also a drm_link_status, but that looks to be mostly unused. > https://elixir.bootlin.com/linux/latest/source/include/drm/drm_connector.h#L331 > > So I think I will follow your suggestion, and remove the check for > ASTDP_LINK_SUCCESS. I've run a test on my server, and even when nothing is plugged, the ASTDP_HPD is still true. So I will just change the order of the checks, and keep the ASTDP_LINK_SUCCESS. Best regards,
Hi Jocelyn, On 2023/7/13 下午 04:17, Jocelyn Falempe wrote: > On 13/07/2023 09:00, Jammy Huang wrote: >> Hi Jocelyn, >> >> Thanks for your work. >> >> On 2023/7/13 下午 02:40, Jocelyn Falempe wrote: >>> Aspeed always report the display port as "connected", because it >>> doesn't set a .detect_ctx callback. >>> Fix this by providing the proper detect callback for astdp and dp501. >>> >>> This also fixes the following regression: >>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no >>> EDID on DP") The default resolution is now 640x480 when no monitor is >>> connected. But Aspeed graphics is mostly used in servers, where no >>> monitor >>> is attached. This also affects the remote BMC resolution to 640x480, >>> which >>> is inconvenient, and breaks the anaconda installer. >>> >>> v2: Add .detect callback to the dp/dp501 connector (Jani Nikula) >>> v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann) >>> Add a BMC virtual connector >>> v4: Better indent detect_ctx() functions (Thomas Zimmermann) >>> v5: Enable polling of the dp and dp501 connector status >>> (Thomas Zimmermann) >>> >>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no >>> EDID on DP") >>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/gpu/drm/ast/ast_dp.c | 11 ++++++++++ >>> drivers/gpu/drm/ast/ast_dp501.c | 37 >>> ++++++++++++++++++++++----------- >>> drivers/gpu/drm/ast/ast_drv.h | 2 ++ >>> drivers/gpu/drm/ast/ast_mode.c | 31 +++++++++++++++++++++++++-- >>> 4 files changed, 67 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ast/ast_dp.c >>> b/drivers/gpu/drm/ast/ast_dp.c >>> index 6dc1a09504e1..bf78f3d4aa3f 100644 >>> --- a/drivers/gpu/drm/ast/ast_dp.c >>> +++ b/drivers/gpu/drm/ast/ast_dp.c >>> @@ -7,6 +7,17 @@ >>> #include <drm/drm_print.h> >>> #include "ast_drv.h" >>> +bool ast_astdp_is_connected(struct ast_device *ast) >>> +{ >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, >>> ASTDP_MCU_FW_EXECUTING)) >>> + return false; >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, >>> ASTDP_LINK_SUCCESS)) >>> + return false; >>> + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, >>> ASTDP_HPD)) >>> + return false; >>> + return true; >>> +} >> >> * ASPDP_LINK_SUCCESS is true, when DP link training is finished. The >> DP link quality is good >> >> enough to deliver video data. >> >> * ASTDP_HPD is true, when there is DP sink pull high HPD. >> >> Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would >> suggest to remove >> >> the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for >> connected status. >> >> If you want to check all these three status, please change the order, >> FW_EXECUTING -> HPD -> >> >> LINK_SUCCESS. > > Thanks for the detailed explanations. > I looked at other drivers to see if HPD is good enough for "connected" > status, but I didn't find a clear answer. > There is also a drm_link_status, but that looks to be mostly unused. > https://elixir.bootlin.com/linux/latest/source/include/drm/drm_connector.h#L331 > > > So I think I will follow your suggestion, and remove the check for > ASTDP_LINK_SUCCESS. > > > For the BMC connector patch, you know if there is a register setting I > can check to see if a BMC is present or not ? I think you can use VGA CRD0[7] whose definition is as below: DRAM Initial Selection (see note 1) 0: VBIOS Initial the DRAM 1: SOC Firmware Initial the DRAM If CRD0[7] is 1, there is BMC. > > Best regards, >
Hi Am 13.07.23 um 11:05 schrieb Jammy Huang: [...] >> >> >> For the BMC connector patch, you know if there is a register setting I >> can check to see if a BMC is present or not ? > > I think you can use VGA CRD0[7] whose definition is as below: > > DRAM Initial Selection (see note 1) > 0: VBIOS Initial the DRAM > 1: SOC Firmware Initial the DRAM > > If CRD0[7] is 1, there is BMC. Thank you. That seems very helpful. Best regards Thomas > >> >> Best regards, >>
On 13/07/2023 11:12, Thomas Zimmermann wrote: > Hi > > Am 13.07.23 um 11:05 schrieb Jammy Huang: > [...] >>> >>> >>> For the BMC connector patch, you know if there is a register setting >>> I can check to see if a BMC is present or not ? >> >> I think you can use VGA CRD0[7] whose definition is as below: >> >> DRAM Initial Selection (see note 1) >> 0: VBIOS Initial the DRAM >> 1: SOC Firmware Initial the DRAM >> >> If CRD0[7] is 1, there is BMC. > > Thank you. That seems very helpful. Thank a lot, I will run some tests, and add this check in my v6.
On 13/07/2023 11:31, Jocelyn Falempe wrote: > On 13/07/2023 11:12, Thomas Zimmermann wrote: >> Hi >> >> Am 13.07.23 um 11:05 schrieb Jammy Huang: >> [...] >>>> >>>> >>>> For the BMC connector patch, you know if there is a register setting >>>> I can check to see if a BMC is present or not ? >>> >>> I think you can use VGA CRD0[7] whose definition is as below: >>> >>> DRAM Initial Selection (see note 1) >>> 0: VBIOS Initial the DRAM >>> 1: SOC Firmware Initial the DRAM >>> >>> If CRD0[7] is 1, there is BMC. >> >> Thank you. That seems very helpful. > > Thank a lot, I will run some tests, and add this check in my v6. > Unfortunately, on my testing server (which has BMC, and I'm using it) the bit(7) of CRD0 is set to 0. Here is the complete register read: AST read[d0] 0x41 So I will keep the always enabled BMC virtual connector.
diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c index 6dc1a09504e1..bf78f3d4aa3f 100644 --- a/drivers/gpu/drm/ast/ast_dp.c +++ b/drivers/gpu/drm/ast/ast_dp.c @@ -7,6 +7,17 @@ #include <drm/drm_print.h> #include "ast_drv.h" +bool ast_astdp_is_connected(struct ast_device *ast) +{ + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, ASTDP_MCU_FW_EXECUTING)) + return false; + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, ASTDP_LINK_SUCCESS)) + return false; + if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD)) + return false; + return true; +} + int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata) { struct ast_device *ast = to_ast_device(dev); diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c index a5d285850ffb..f10d53b0c94f 100644 --- a/drivers/gpu/drm/ast/ast_dp501.c +++ b/drivers/gpu/drm/ast/ast_dp501.c @@ -272,11 +272,9 @@ static bool ast_launch_m68k(struct drm_device *dev) return true; } -bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) +bool ast_dp501_is_connected(struct ast_device *ast) { - struct ast_device *ast = to_ast_device(dev); - u32 i, boot_address, offset, data; - u32 *pEDIDidx; + u32 boot_address, offset, data; if (ast->config_mode == ast_use_p2a) { boot_address = get_fw_base(ast); @@ -292,14 +290,6 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) data = ast_mindwm(ast, boot_address + offset); if (!(data & AST_DP501_PNP_CONNECTED)) return false; - - /* Read EDID */ - offset = AST_DP501_EDID_DATA; - for (i = 0; i < 128; i += 4) { - data = ast_mindwm(ast, boot_address + offset + i); - pEDIDidx = (u32 *)(ediddata + i); - *pEDIDidx = data; - } } else { if (!ast->dp501_fw_buf) return false; @@ -319,7 +309,30 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) data = readl(ast->dp501_fw_buf + offset); if (!(data & AST_DP501_PNP_CONNECTED)) return false; + } + return true; +} + +bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata) +{ + struct ast_device *ast = to_ast_device(dev); + u32 i, boot_address, offset, data; + u32 *pEDIDidx; + + if (!ast_dp501_is_connected(ast)) + return false; + + if (ast->config_mode == ast_use_p2a) { + boot_address = get_fw_base(ast); + /* Read EDID */ + offset = AST_DP501_EDID_DATA; + for (i = 0; i < 128; i += 4) { + data = ast_mindwm(ast, boot_address + offset + i); + pEDIDidx = (u32 *)(ediddata + i); + *pEDIDidx = data; + } + } else { /* Read EDID */ offset = AST_DP501_EDID_DATA; for (i = 0; i < 128; i += 4) { diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index c9659e72002f..848a9f1403e8 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -514,6 +514,7 @@ void ast_patch_ahb_2500(struct ast_device *ast); /* ast dp501 */ void ast_set_dp501_video_output(struct drm_device *dev, u8 mode); bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); +bool ast_dp501_is_connected(struct ast_device *ast); bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata); u8 ast_get_dp501_max_clk(struct drm_device *dev); void ast_init_3rdtx(struct drm_device *dev); @@ -522,6 +523,7 @@ void ast_init_3rdtx(struct drm_device *dev); struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev); /* aspeed DP */ +bool ast_astdp_is_connected(struct ast_device *ast); int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata); void ast_dp_launch(struct drm_device *dev); void ast_dp_power_on_off(struct drm_device *dev, bool no); diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 1a8293162fec..e7f36ec73817 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1582,8 +1582,21 @@ static int ast_dp501_connector_helper_get_modes(struct drm_connector *connector) return 0; } +static int ast_dp501_connector_helper_detect_ctx(struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx, + bool force) +{ + struct ast_device *ast = to_ast_device(connector->dev); + + if (ast_dp501_is_connected(ast)) + return connector_status_connected; + return connector_status_disconnected; +} + + static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs = { .get_modes = ast_dp501_connector_helper_get_modes, + .detect_ctx = ast_dp501_connector_helper_detect_ctx, }; static const struct drm_connector_funcs ast_dp501_connector_funcs = { @@ -1608,7 +1621,7 @@ static int ast_dp501_connector_init(struct drm_device *dev, struct drm_connector connector->interlace_allowed = 0; connector->doublescan_allowed = 0; - connector->polled = DRM_CONNECTOR_POLL_CONNECT; + connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; return 0; } @@ -1680,8 +1693,20 @@ static int ast_astdp_connector_helper_get_modes(struct drm_connector *connector) return 0; } +static int ast_astdp_connector_helper_detect_ctx(struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx, + bool force) +{ + struct ast_device *ast = to_ast_device(connector->dev); + + if (ast_astdp_is_connected(ast)) + return connector_status_connected; + return connector_status_disconnected; +} + static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs = { .get_modes = ast_astdp_connector_helper_get_modes, + .detect_ctx = ast_astdp_connector_helper_detect_ctx, }; static const struct drm_connector_funcs ast_astdp_connector_funcs = { @@ -1706,7 +1731,7 @@ static int ast_astdp_connector_init(struct drm_device *dev, struct drm_connector connector->interlace_allowed = 0; connector->doublescan_allowed = 0; - connector->polled = DRM_CONNECTOR_POLL_CONNECT; + connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; return 0; } @@ -1903,5 +1928,7 @@ int ast_mode_config_init(struct ast_device *ast) drm_mode_config_reset(dev); + drm_kms_helper_poll_init(dev); + return 0; }
Aspeed always report the display port as "connected", because it doesn't set a .detect_ctx callback. Fix this by providing the proper detect callback for astdp and dp501. This also fixes the following regression: Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP") The default resolution is now 640x480 when no monitor is connected. But Aspeed graphics is mostly used in servers, where no monitor is attached. This also affects the remote BMC resolution to 640x480, which is inconvenient, and breaks the anaconda installer. v2: Add .detect callback to the dp/dp501 connector (Jani Nikula) v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann) Add a BMC virtual connector v4: Better indent detect_ctx() functions (Thomas Zimmermann) v5: Enable polling of the dp and dp501 connector status (Thomas Zimmermann) Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP") Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/ast/ast_dp.c | 11 ++++++++++ drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++----------- drivers/gpu/drm/ast/ast_drv.h | 2 ++ drivers/gpu/drm/ast/ast_mode.c | 31 +++++++++++++++++++++++++-- 4 files changed, 67 insertions(+), 14 deletions(-)