Hello Russell,
On 08/18/2016 05:32 PM, Russell King - ARM Linux wrote:
On Tue, Aug 16, 2016 at 11:26:43PM +0300, Vladimir Zapolskiy wrote:
This change is needed to properly lock I2C bus driver, which serves
DDC.
The change fixes an overflow over zero of I2C bus driver user counter:
root@imx6q:~# lsmod
Not tainted
dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000
dw_hdmi_imx 3498 0 - Live 0xbf00d000
dw_hdmi 16398 2 dw_hdmi_ahb_audio,dw_hdmi_imx, Live 0xbf004000
i2c_imx 16687 0 - Live 0xbf017000
root@imx6q:~# rmmod dw_hdmi_imx
root@imx6q:~# lsmod
Not tainted
dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000
dw_hdmi 16398 1 dw_hdmi_ahb_audio, Live 0xbf004000
i2c_imx 16687 -1 - Live 0xbf017000
^^
root@imx6q:~# rmmod i2c_imx
rmmod: ERROR: Module i2c_imx is in use
Note that prior to this change put_device() coupled with
of_find_i2c_adapter_by_node() was missing on error path of
dw_hdmi_bind(), added i2c_put_adapter() there along with the change.
I *guess* this is the right thing, but it would be nice to see the
results with the patch applied in the commit description. Nevertheless:
Acked-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx>
thank you for review.
For information this is a result after applying the fix (+1 to i2c_imx users):
root@imx6q# lsmod
Not tainted
dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000
i2c_imx 16687 1 - Live 0xbf011000
dw_hdmi_imx 3498 0 - Live 0xbf00d000
dw_hdmi 18925 2 dw_hdmi_ahb_audio,dw_hdmi_imx, Live 0xbf004000
root@imx6q:~# rmmod dw_hdmi_imx
root@imx6q:~# lsmod
Not tainted
dw_hdmi_ahb_audio 4082 0 - Live 0xbf02c000
i2c_imx 16687 0 - Live 0xbf011000
dw_hdmi 18925 1 dw_hdmi_ahb_audio, Live 0xbf004000
No weird negative use count.
I have another pending change against DW HDMI, to avoid git-am conflicts
I'll rebase it on top of this one and resend today later on.
I'd also like to see the DDC bits extracted from the various imx
drivers, because this is surely common code - I've had this floating
around for a few years but never got around to finishing it off and
submitting it. If folk think this is a good idea, and want to take
the idea on, that's fine by me.
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/bridge/dw-hdmi.c | 98 +++++++++----------------------------
drivers/gpu/drm/drm_ddc_connector.c | 91 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/imx/imx-tve.c | 59 ++++++----------------
include/drm/drm_ddc_connector.h | 33 +++++++++++++
5 files changed, 163 insertions(+), 120 deletions(-)
I've briefly reviewed the changes and in my opinion this is a good
to have generalization of DDC connector, hopefully DRM people agree.
Moreover I assume that in case of getting modes over I2C/DDC most of
the .get_modes callbacks share almost the same code sequence:
drm_get_edid()
drm_detect_hdmi_monitor()
drm_mode_connector_update_edid_property()
drm_add_edid_modes()
drm_edid_to_eld()
I'm not absolutely sure, but probably this can be generalized as well.
--
With best wishes,
Vladimir
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
