Hi,
Thanks for those patches, that's really a good enhancement to the ast
driver, and fits well with the BMC virtual connector.
I've reviewed the whole series, and it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
--
Jocelyn
On 20/03/2024 10:34, Thomas Zimmermann wrote:
Detect the status of the VGA or SIL164 conenctor by polling the DDC
channel. Update the status at runtime and flip the BMC status as well.
Patches 1 adds a missing include statement that will be required to
make later patches compile.
Patches 2 and 3 simplify the VGA and SIL164 connectors, such that no
additional data structure is required. These patches have been reviewed
before as part of the series at [1].
Patches 4 to 10 improve the I2C code that is used to retrieve the
monitor's EDID data. It's now fully managed, it acquires the necessary
lock automatically and it is called DDC, which better represents its
purpose than I2C.
Patches 11 to 13 finally implement polling. Patch 11 updates ast's
EDID code to be up-to-date. The helper drm_connector_get_modes() reads
the EDID via DDC and updates the property. No driver code is required.
Patch 12 uses a similar pattern to detect the presence of the monitor
and sets the connector status accordingly. As polling also needs to be
cleaned up, patch 13 adds the necessary helpers to do so.
Tested on AST2500 hardware and BMC output. The BMC connector now also
flips its status correctly at runtime.
[1] https://patchwork.freedesktop.org/series/104547/
v5:
- share implementation in drm_connector_helper_detect_ctx() (Maxime)
- test for DDC presence with drm_probe_ddc() (Maxime, Jani)
- perform managed cleanup of poll thread
Thomas Zimmermann (13):
drm/ast: Include <linux/of.h> where necessary
drm/ast: Fail probing if DDC channel could not be initialized
drm/ast: Remove struct ast_{vga,sil165}_connector
drm/ast: Allocate instance of struct ast_i2c_chan with managed helpers
drm/ast: Move DDC code to ast_ddc.{c,h}
drm/ast: Rename struct ast_i2c_chan to struct ast_ddc
drm/ast: Pass AST device to ast_ddc_create()
drm/ast: Store AST device in struct ast_ddc
drm/ast: Rename struct i2c_algo_bit_data callbacks and their
parameters
drm/ast: Acquire I/O-register lock in DDC code
drm/ast: Use drm_connector_helper_get_modes()
drm/ast: Implement polling for VGA and SIL164 connectors
drm/ast: Automatically clean up poll helper
drivers/gpu/drm/ast/Makefile | 10 +-
drivers/gpu/drm/ast/{ast_i2c.c => ast_ddc.c} | 120 +++++++++------
drivers/gpu/drm/ast/ast_ddc.h | 20 +++
drivers/gpu/drm/ast/ast_drv.c | 1 +
drivers/gpu/drm/ast/ast_drv.h | 39 +----
drivers/gpu/drm/ast/ast_main.c | 1 +
drivers/gpu/drm/ast/ast_mode.c | 147 +++++--------------
drivers/gpu/drm/drm_probe_helper.c | 56 +++++++
include/drm/drm_probe_helper.h | 5 +
9 files changed, 206 insertions(+), 193 deletions(-)
rename drivers/gpu/drm/ast/{ast_i2c.c => ast_ddc.c} (54%)
create mode 100644 drivers/gpu/drm/ast/ast_ddc.h