* [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins @ 2024-02-28 11:37 Adam Ford 2024-02-28 11:37 ` [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD Adam Ford 2024-02-28 16:31 ` [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Laurent Pinchart 0 siblings, 2 replies; 6+ messages in thread From: Adam Ford @ 2024-02-28 11:37 UTC (permalink / raw) To: dri-devel, linux-arm-kernel Cc: aford, Adam Ford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel The IRQ registration currently assumes that the GPIO is dedicated to it, but that may not necessarily be the case. If the board has another device sharing the IRQ, it won't be registered and the hot-plug detect fails. This is easily fixed by add the IRQF_SHARED flag. Signed-off-by: Adam Ford <aford173@gmail.com> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b5518ff97165..21f08b2ae265 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -1318,7 +1318,8 @@ static int adv7511_probe(struct i2c_client *i2c) ret = devm_request_threaded_irq(dev, i2c->irq, NULL, adv7511_irq_handler, - IRQF_ONESHOT, dev_name(dev), + IRQF_ONESHOT | IRQF_SHARED, + dev_name(dev), adv7511); if (ret) goto err_unregister_audio; -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD 2024-02-28 11:37 [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Adam Ford @ 2024-02-28 11:37 ` Adam Ford 2024-02-28 16:33 ` Laurent Pinchart 2024-02-28 16:31 ` [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Laurent Pinchart 1 sibling, 1 reply; 6+ messages in thread From: Adam Ford @ 2024-02-28 11:37 UTC (permalink / raw) To: dri-devel, linux-arm-kernel Cc: aford, Adam Ford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel The DSI to HDMI bridge supports hot-plut-detect, but the driver didn't previously support a shared IRQ GPIO. With the driver updated, the interrupt can be added to the bridge. Signed-off-by: Adam Ford <aford173@gmail.com> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts b/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts index 8de4dd268908..d854c94ec997 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts +++ b/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts @@ -405,6 +405,8 @@ adv_bridge: hdmi@3d { compatible = "adi,adv7535"; reg = <0x3d>, <0x3c>, <0x3e>, <0x3f>; reg-names = "main", "cec", "edid", "packet"; + interrupt-parent = <&gpio4>; + interrupts = <27 IRQ_TYPE_EDGE_FALLING>; adi,dsi-lanes = <4>; #sound-dai-cells = <0>; -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD 2024-02-28 11:37 ` [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD Adam Ford @ 2024-02-28 16:33 ` Laurent Pinchart 0 siblings, 0 replies; 6+ messages in thread From: Laurent Pinchart @ 2024-02-28 16:33 UTC (permalink / raw) To: Adam Ford Cc: dri-devel, linux-arm-kernel, aford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel Hi Adam, Thank you for the patch. On Wed, Feb 28, 2024 at 05:37:36AM -0600, Adam Ford wrote: > The DSI to HDMI bridge supports hot-plut-detect, but the > driver didn't previously support a shared IRQ GPIO. With > the driver updated, the interrupt can be added to the bridge. You can reflow the commit message to 72 columns. > Signed-off-by: Adam Ford <aford173@gmail.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts b/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts > index 8de4dd268908..d854c94ec997 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts > +++ b/arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dts > @@ -405,6 +405,8 @@ adv_bridge: hdmi@3d { > compatible = "adi,adv7535"; > reg = <0x3d>, <0x3c>, <0x3e>, <0x3f>; > reg-names = "main", "cec", "edid", "packet"; > + interrupt-parent = <&gpio4>; > + interrupts = <27 IRQ_TYPE_EDGE_FALLING>; > adi,dsi-lanes = <4>; > #sound-dai-cells = <0>; > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins 2024-02-28 11:37 [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Adam Ford 2024-02-28 11:37 ` [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD Adam Ford @ 2024-02-28 16:31 ` Laurent Pinchart 2024-03-03 15:44 ` Adam Ford 1 sibling, 1 reply; 6+ messages in thread From: Laurent Pinchart @ 2024-02-28 16:31 UTC (permalink / raw) To: Adam Ford Cc: dri-devel, linux-arm-kernel, aford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel Hi Adam, Thank you for the patch. On Wed, Feb 28, 2024 at 05:37:35AM -0600, Adam Ford wrote: > The IRQ registration currently assumes that the GPIO is > dedicated to it, but that may not necessarily be the case. > If the board has another device sharing the IRQ, it won't be > registered and the hot-plug detect fails. This is easily > fixed by add the IRQF_SHARED flag. > > Signed-off-by: Adam Ford <aford173@gmail.com> > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index b5518ff97165..21f08b2ae265 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -1318,7 +1318,8 @@ static int adv7511_probe(struct i2c_client *i2c) > > ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > adv7511_irq_handler, > - IRQF_ONESHOT, dev_name(dev), > + IRQF_ONESHOT | IRQF_SHARED, > + dev_name(dev), This looks fine, but the IRQ handler doesn't. static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) { unsigned int irq0, irq1; int ret; ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); if (ret < 0) return ret; ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); if (ret < 0) return ret; regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) schedule_work(&adv7511->hpd_work); if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { adv7511->edid_read = true; if (adv7511->i2c_main->irq) wake_up_all(&adv7511->wq); } #ifdef CONFIG_DRM_I2C_ADV7511_CEC adv7511_cec_irq_process(adv7511, irq1); #endif return 0; } static irqreturn_t adv7511_irq_handler(int irq, void *devid) { struct adv7511 *adv7511 = devid; int ret; ret = adv7511_irq_process(adv7511, true); return ret < 0 ? IRQ_NONE : IRQ_HANDLED; } The function will return IRQ_HANDLED as long as the registers can be read, even if they don't report any interrupt. > adv7511); > if (ret) > goto err_unregister_audio; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins 2024-02-28 16:31 ` [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Laurent Pinchart @ 2024-03-03 15:44 ` Adam Ford 2024-03-03 15:52 ` Laurent Pinchart 0 siblings, 1 reply; 6+ messages in thread From: Adam Ford @ 2024-03-03 15:44 UTC (permalink / raw) To: Laurent Pinchart Cc: dri-devel, linux-arm-kernel, aford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel On Wed, Feb 28, 2024 at 10:31 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Adam, > > Thank you for the patch. > > On Wed, Feb 28, 2024 at 05:37:35AM -0600, Adam Ford wrote: > > The IRQ registration currently assumes that the GPIO is > > dedicated to it, but that may not necessarily be the case. > > If the board has another device sharing the IRQ, it won't be > > registered and the hot-plug detect fails. This is easily > > fixed by add the IRQF_SHARED flag. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index b5518ff97165..21f08b2ae265 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > @@ -1318,7 +1318,8 @@ static int adv7511_probe(struct i2c_client *i2c) > > > > ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > > adv7511_irq_handler, > > - IRQF_ONESHOT, dev_name(dev), > > + IRQF_ONESHOT | IRQF_SHARED, > > + dev_name(dev), > > This looks fine, but the IRQ handler doesn't. Thanks for the review. > > static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) > { > unsigned int irq0, irq1; > int ret; > > ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); > if (ret < 0) > return ret; > > ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); > if (ret < 0) > return ret; If I did a check here and returned if there was no IRQ to handle, would that be sufficient? --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -477,6 +477,11 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) if (ret < 0) return ret; + /* If there is no IRQ to handle, exit indicating no IRQ handled */ + if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) && + !(irq1 & ADV7511_INT1_DDC_ERROR)) + return -1; + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); With this, I would expect adv7511_irq_handler to return IRQ_NONE. If you're OK with that approach, I can do that. If you want me to merge adv7511_irq_handler, and adv7511_irq_process, I can do that too to make the return codes a little more intuitive. adam > > regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); > regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); > > if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) > schedule_work(&adv7511->hpd_work); > > if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { > adv7511->edid_read = true; > > if (adv7511->i2c_main->irq) > wake_up_all(&adv7511->wq); > } > > #ifdef CONFIG_DRM_I2C_ADV7511_CEC > adv7511_cec_irq_process(adv7511, irq1); > #endif > > return 0; > } > > static irqreturn_t adv7511_irq_handler(int irq, void *devid) > { > struct adv7511 *adv7511 = devid; > int ret; > > ret = adv7511_irq_process(adv7511, true); > return ret < 0 ? IRQ_NONE : IRQ_HANDLED; > } > > The function will return IRQ_HANDLED as long as the registers can be > read, even if they don't report any interrupt. > > > adv7511); > > if (ret) > > goto err_unregister_audio; > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins 2024-03-03 15:44 ` Adam Ford @ 2024-03-03 15:52 ` Laurent Pinchart 0 siblings, 0 replies; 6+ messages in thread From: Laurent Pinchart @ 2024-03-03 15:52 UTC (permalink / raw) To: Adam Ford Cc: dri-devel, linux-arm-kernel, aford, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, devicetree, imx, linux-kernel On Sun, Mar 03, 2024 at 09:44:03AM -0600, Adam Ford wrote: > On Wed, Feb 28, 2024 at 10:31 AM Laurent Pinchart wrote: > > On Wed, Feb 28, 2024 at 05:37:35AM -0600, Adam Ford wrote: > > > The IRQ registration currently assumes that the GPIO is > > > dedicated to it, but that may not necessarily be the case. > > > If the board has another device sharing the IRQ, it won't be > > > registered and the hot-plug detect fails. This is easily > > > fixed by add the IRQF_SHARED flag. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > index b5518ff97165..21f08b2ae265 100644 > > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > > @@ -1318,7 +1318,8 @@ static int adv7511_probe(struct i2c_client *i2c) > > > > > > ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > > > adv7511_irq_handler, > > > - IRQF_ONESHOT, dev_name(dev), > > > + IRQF_ONESHOT | IRQF_SHARED, > > > + dev_name(dev), > > > > This looks fine, but the IRQ handler doesn't. > > Thanks for the review. > > > static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) > > { > > unsigned int irq0, irq1; > > int ret; > > > > ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); > > if (ret < 0) > > return ret; > > > > ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); > > if (ret < 0) > > return ret; > > If I did a check here and returned if there was no IRQ to handle, > would that be sufficient? > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -477,6 +477,11 @@ static int adv7511_irq_process(struct adv7511 > *adv7511, bool process_hpd) > if (ret < 0) > return ret; > > + /* If there is no IRQ to handle, exit indicating no IRQ handled */ > + if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) && > + !(irq1 & ADV7511_INT1_DDC_ERROR)) If these are the only interrupt sources that the driver enables, this is fine. > + return -1; Maybe a defined error code instead ? > + > regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); > regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); > > With this, I would expect adv7511_irq_handler to return IRQ_NONE. If > you're OK with that approach, I can do that. If you want me to merge > adv7511_irq_handler, and adv7511_irq_process, I can do that too to > make the return codes a little more intuitive. > > > > > regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0); > > regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); > > > > if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) > > schedule_work(&adv7511->hpd_work); > > > > if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { > > adv7511->edid_read = true; > > > > if (adv7511->i2c_main->irq) > > wake_up_all(&adv7511->wq); > > } > > > > #ifdef CONFIG_DRM_I2C_ADV7511_CEC > > adv7511_cec_irq_process(adv7511, irq1); > > #endif > > > > return 0; > > } > > > > static irqreturn_t adv7511_irq_handler(int irq, void *devid) > > { > > struct adv7511 *adv7511 = devid; > > int ret; > > > > ret = adv7511_irq_process(adv7511, true); > > return ret < 0 ? IRQ_NONE : IRQ_HANDLED; > > } > > > > The function will return IRQ_HANDLED as long as the registers can be > > read, even if they don't report any interrupt. > > > > > adv7511); > > > if (ret) > > > goto err_unregister_audio; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-03 15:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-28 11:37 [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Adam Ford 2024-02-28 11:37 ` [PATCH 2/2] arm64: dts: imx8mp-beacon-kit: Enable HDMI bridge HPD Adam Ford 2024-02-28 16:33 ` Laurent Pinchart 2024-02-28 16:31 ` [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins Laurent Pinchart 2024-03-03 15:44 ` Adam Ford 2024-03-03 15:52 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).