| Message ID | 1476863178-15237-1-git-send-email-Eugeniy.Paltsev@synopsys.com |
|---|---|
| State | New |
| Headers | show |
On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: > ARC PGU driver starts crashing on initialization after > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer > of "drm_i2c_encoder_driver" structure, which doesn't exist after > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. > So, when we call "encoder_init" function from this structure driver > crashes. > > Bootlog: > ------------------------------------->8-------------------------------- > [drm] Initialized drm 1.1.0 20060810 > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e000000 > Path: (null) > CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-00001-gb5642252fa01-dirty #8 > task: 9a058000 task.stack: 9a032000 > > [ECR ]: 0x00220100 => Invalid Read @ 0x00000004 by insn @ 0x803934e8 > [EFA ]: 0x00000004 > [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 > [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 > [STAT32]: 0x00000846 : K DE E2 E1 > BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x00000000 > LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x00000000 > r00: 0x8063c118 r01: 0x805b98ac r02: 0x00000b11 > r03: 0x00000000 r04: 0x9a010f54 r05: 0x00000000 > r06: 0x00000001 r07: 0x00000000 r08: 0x00000028 > r09: 0x00000001 r10: 0x00000007 r11: 0x00000054 > r12: 0x720a3033 > > Stack Trace: > drm_atomic_helper_connector_dpms+0xa4/0x230 > arcpgu_drm_hdmi_init+0xbc/0x228 > arcpgu_probe+0x168/0x244 > platform_drv_probe+0x26/0x64 > really_probe+0x1f0/0x32c > __driver_attach+0xa8/0xd0 > bus_for_each_dev+0x3c/0x74 > bus_add_driver+0xc2/0x184 > driver_register+0x50/0xec > do_one_initcall+0x3a/0x120 > kernel_init_freeable+0x108/0x1a0 > ------------------------------------->8-------------------------------- > > Fix ARC PGU driver to be able work with drm bridge hdmi encoder > interface. The hdmi connector code isn't needed anymore as we expect > the adv7511 bridge driver to create/manage the connector. Looks good now. Reviewed-by: Archit Taneja <architt@codeaurora.org> > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > Changes for v2: > - remove bridge functions call from kms driver > - get rid of drm_encoder_slave interface > - coding style cleanup > > drivers/gpu/drm/arc/arcpgu_hdmi.c | 159 ++++---------------------------------- > 1 file changed, 17 insertions(+), 142 deletions(-) > > diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c > index b7a8b2a..b69c66b 100644 > --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c > +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c > @@ -14,170 +14,45 @@ > * > */ > > -#include <drm/drm_crtc_helper.h> > +#include <drm/drm_crtc.h> > #include <drm/drm_encoder_slave.h> > -#include <drm/drm_atomic_helper.h> > > #include "arcpgu.h" > > -struct arcpgu_drm_connector { > - struct drm_connector connector; > - struct drm_encoder_slave *encoder_slave; > -}; > - > -static int arcpgu_drm_connector_get_modes(struct drm_connector *connector) > -{ > - const struct drm_encoder_slave_funcs *sfuncs; > - struct drm_encoder_slave *slave; > - struct arcpgu_drm_connector *con = > - container_of(connector, struct arcpgu_drm_connector, connector); > - > - slave = con->encoder_slave; > - if (slave == NULL) { > - dev_err(connector->dev->dev, > - "connector_get_modes: cannot find slave encoder for connector\n"); > - return 0; > - } > - > - sfuncs = slave->slave_funcs; > - if (sfuncs->get_modes == NULL) > - return 0; > - > - return sfuncs->get_modes(&slave->base, connector); > -} > - > -static enum drm_connector_status > -arcpgu_drm_connector_detect(struct drm_connector *connector, bool force) > -{ > - enum drm_connector_status status = connector_status_unknown; > - const struct drm_encoder_slave_funcs *sfuncs; > - struct drm_encoder_slave *slave; > - > - struct arcpgu_drm_connector *con = > - container_of(connector, struct arcpgu_drm_connector, connector); > - > - slave = con->encoder_slave; > - if (slave == NULL) { > - dev_err(connector->dev->dev, > - "connector_detect: cannot find slave encoder for connector\n"); > - return status; > - } > - > - sfuncs = slave->slave_funcs; > - if (sfuncs && sfuncs->detect) > - return sfuncs->detect(&slave->base, connector); > - > - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); > - return status; > -} > - > -static void arcpgu_drm_connector_destroy(struct drm_connector *connector) > -{ > - drm_connector_unregister(connector); > - drm_connector_cleanup(connector); > -} > - > -static const struct drm_connector_helper_funcs > -arcpgu_drm_connector_helper_funcs = { > - .get_modes = arcpgu_drm_connector_get_modes, > -}; > - > -static const struct drm_connector_funcs arcpgu_drm_connector_funcs = { > - .dpms = drm_helper_connector_dpms, > - .reset = drm_atomic_helper_connector_reset, > - .detect = arcpgu_drm_connector_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = arcpgu_drm_connector_destroy, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > -}; > - > -static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = { > - .dpms = drm_i2c_encoder_dpms, > - .mode_fixup = drm_i2c_encoder_mode_fixup, > - .mode_set = drm_i2c_encoder_mode_set, > - .prepare = drm_i2c_encoder_prepare, > - .commit = drm_i2c_encoder_commit, > - .detect = drm_i2c_encoder_detect, > -}; > - > static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = { > .destroy = drm_encoder_cleanup, > }; > > int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np) > { > - struct arcpgu_drm_connector *arcpgu_connector; > - struct drm_i2c_encoder_driver *driver; > - struct drm_encoder_slave *encoder; > - struct drm_connector *connector; > - struct i2c_client *i2c_slave; > - int ret; > + struct drm_encoder *encoder; > + struct drm_bridge *bridge; > + > + int ret = 0; > > encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL); > if (encoder == NULL) > return -ENOMEM; > > - i2c_slave = of_find_i2c_device_by_node(np); > - if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) { > - dev_err(drm->dev, "failed to find i2c slave encoder\n"); > - return -EPROBE_DEFER; > - } > - > - if (i2c_slave->dev.driver == NULL) { > - dev_err(drm->dev, "failed to find i2c slave driver\n"); > + /* Locate drm bridge from the hdmi encoder DT node */ > + bridge = of_drm_find_bridge(np); > + if (!bridge) > return -EPROBE_DEFER; > - } > > - driver = > - to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver)); > - ret = driver->encoder_init(i2c_slave, drm, encoder); > - if (ret) { > - dev_err(drm->dev, "failed to initialize i2c encoder slave\n"); > - return ret; > - } > - > - encoder->base.possible_crtcs = 1; > - encoder->base.possible_clones = 0; > - ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs, > + encoder->possible_crtcs = 1; > + encoder->possible_clones = 0; > + ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs, > DRM_MODE_ENCODER_TMDS, NULL); > if (ret) > return ret; > > - drm_encoder_helper_add(&encoder->base, > - &arcpgu_drm_encoder_helper_funcs); > - > - arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector), > - GFP_KERNEL); > - if (!arcpgu_connector) { > - ret = -ENOMEM; > - goto error_encoder_cleanup; > - } > - > - connector = &arcpgu_connector->connector; > - drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs); > - ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs, > - DRM_MODE_CONNECTOR_HDMIA); > - if (ret < 0) { > - dev_err(drm->dev, "failed to initialize drm connector\n"); > - goto error_encoder_cleanup; > - } > + /* Link drm_bridge to encoder */ > + bridge->encoder = encoder; > + encoder->bridge = bridge; > > - ret = drm_mode_connector_attach_encoder(connector, &encoder->base); > - if (ret < 0) { > - dev_err(drm->dev, "could not attach connector to encoder\n"); > - drm_connector_unregister(connector); > - goto error_connector_cleanup; > - } > - > - arcpgu_connector->encoder_slave = encoder; > - > - return 0; > - > -error_connector_cleanup: > - drm_connector_cleanup(connector); > + ret = drm_bridge_attach(drm, bridge); > + if (ret) > + drm_encoder_cleanup(encoder); > > -error_encoder_cleanup: > - drm_encoder_cleanup(&encoder->base); > return ret; > } >
Hi Archit, all, On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote: > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: > > > > ARC PGU driver starts crashing on initialization after > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer > > of "drm_i2c_encoder_driver" structure, which doesn't exist after > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. > > So, when we call "encoder_init" function from this structure driver > > crashes. [snip] > Looks good now. > > Reviewed-by: Archit Taneja <architt@codeaurora.org> And IMHO it would be really good to get this one back-ported to 4.8 because it really fixes kernel crash if ARC PGU driver is used. It might be a bit of a problem because patch itself a little-bit larger than formal requirement for stable backports but let's see if it gets accepted. -Alexey
Hi Daniel, > -----Original Message----- > From: linux-snps-arc [mailto:linux-snps-arc-bounces@lists.infradead.org] On Behalf Of Alexey Brodkin > Sent: 19 октября 2016 г. 12:33 > To: dri-devel@lists.freedesktop.org; architt@codeaurora.org; Eugeniy.Paltsev@synopsys.com > Cc: linux-snps-arc@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge > > Hi Archit, all, > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote: > > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: > > > > > > ARC PGU driver starts crashing on initialization after > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. > > > So, when we call "encoder_init" function from this structure driver > > > crashes. > > [snip] > > > Looks good now. > > > > Reviewed-by: Archit Taneja <architt@codeaurora.org> > > And IMHO it would be really good to get this one back-ported to 4.8 > because it really fixes kernel crash if ARC PGU driver is used. > > It might be a bit of a problem because patch itself a little-bit larger > than formal requirement for stable backports but let's see if it gets accepted. Could you please pick this one up? I may alternatively send a pull-request to David but not sure if 1 patch worth it. Also if that's not really too late it would be good to get this one in 4.9 since the patch In question fixes a real driver crash on its instantiation. Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount of changes I think there's barely a chance for it it to be accepted in stable branches... which in its turn makes at least 4.9 very desirable. -Alexey
Hi Daniel, David, On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote: > Hi Daniel, > > > > > -----Original Message----- > > From: linux-snps-arc [mailto:linux-snps-arc-bounces@lists.infradead.org] On Behalf Of Alexey Brodkin > > Sent: 19 октября 2016 г. 12:33 > > To: dri-devel@lists.freedesktop.org; architt@codeaurora.org; Eugeniy.Paltsev@synopsys.com > > Cc: linux-snps-arc@lists.infradead.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge > > > > Hi Archit, all, > > > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote: > > > > > > > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: > > > > > > > > > > > > ARC PGU driver starts crashing on initialization after > > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' > > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer > > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after > > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. > > > > So, when we call "encoder_init" function from this structure driver > > > > crashes. > > > > [snip] > > > > > > > > Looks good now. > > > > > > Reviewed-by: Archit Taneja <architt@codeaurora.org> > > > > And IMHO it would be really good to get this one back-ported to 4.8 > > because it really fixes kernel crash if ARC PGU driver is used. > > > > It might be a bit of a problem because patch itself a little-bit larger > > than formal requirement for stable backports but let's see if it gets accepted. > > Could you please pick this one up? > I may alternatively send a pull-request to David but not sure if 1 patch worth it. > > Also if that's not really too late it would be good to get this one in 4.9 since the patch > In question fixes a real driver crash on its instantiation. > Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount > of changes I think there's barely a chance for it it to be accepted in stable branches... > which in its turn makes at least 4.9 very desirable. Any chance this one gets accepted anytime soon? Regards, Alexey
Hi Daniel, David, On Wed, 2016-11-02 at 12:23 +0000, Alexey Brodkin wrote: > Hi Daniel, David, > > On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote: > > > > Hi Daniel, > > > > > > > > > > > -----Original Message----- > > > From: linux-snps-arc [mailto:linux-snps-arc-bounces@lists.infradead.org] On Behalf Of Alexey Brodkin > > > Sent: 19 октября 2016 г. 12:33 > > > To: dri-devel@lists.freedesktop.org; architt@codeaurora.org; Eugeniy.Paltsev@synopsys.com > > > Cc: linux-snps-arc@lists.infradead.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge > > > > > > Hi Archit, all, > > > > > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote: > > > > > > > > > > > > > > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: > > > > > > > > > > > > > > > > > > > > ARC PGU driver starts crashing on initialization after > > > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' > > > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer > > > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after > > > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. > > > > > So, when we call "encoder_init" function from this structure driver > > > > > crashes. > > > > > > [snip] > > > > > > > > > > > > > > > Looks good now. > > > > > > > > Reviewed-by: Archit Taneja <architt@codeaurora.org> > > > > > > And IMHO it would be really good to get this one back-ported to 4.8 > > > because it really fixes kernel crash if ARC PGU driver is used. > > > > > > It might be a bit of a problem because patch itself a little-bit larger > > > than formal requirement for stable backports but let's see if it gets accepted. > > > > Could you please pick this one up? > > I may alternatively send a pull-request to David but not sure if 1 patch worth it. > > > > Also if that's not really too late it would be good to get this one in 4.9 since the patch > > In question fixes a real driver crash on its instantiation. > > Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount > > of changes I think there's barely a chance for it it to be accepted in stable branches... > > which in its turn makes at least 4.9 very desirable. > > Any chance this one gets accepted anytime soon? Please treat this as another polite reminder to apply this patch. If you prefer I may send a pull-request otherwise. Regards, Alexey
On 10 November 2016 at 21:06, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Daniel, David, > > On Wed, 2016-11-02 at 12:23 +0000, Alexey Brodkin wrote: >> Hi Daniel, David, >> >> On Mon, 2016-10-24 at 18:33 +0000, Alexey Brodkin wrote: >> > >> > Hi Daniel, >> > >> > > >> > > >> > > -----Original Message----- >> > > From: linux-snps-arc [mailto:linux-snps-arc-bounces@lists.infradead.org] On Behalf Of Alexey Brodkin >> > > Sent: 19 октября 2016 г. 12:33 >> > > To: dri-devel@lists.freedesktop.org; architt@codeaurora.org; Eugeniy.Paltsev@synopsys.com >> > > Cc: linux-snps-arc@lists.infradead.org; linux-kernel@vger.kernel.org >> > > Subject: Re: [PATCH v2] drm/arcpgu: Accommodate adv7511 switch to DRM bridge >> > > >> > > Hi Archit, all, >> > > >> > > On Wed, 2016-10-19 at 14:43 +0530, Archit Taneja wrote: >> > > > >> > > > >> > > > >> > > > On 10/19/2016 01:16 PM, Eugeniy Paltsev wrote: >> > > > > >> > > > > >> > > > > >> > > > > ARC PGU driver starts crashing on initialization after >> > > > > 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' >> > > > > This happenes because in "arcpgu_drm_hdmi_init" function we get pointer >> > > > > of "drm_i2c_encoder_driver" structure, which doesn't exist after >> > > > > adv7511 hdmi encoder interface changed from slave encoder to drm bridge. >> > > > > So, when we call "encoder_init" function from this structure driver >> > > > > crashes. >> > > >> > > [snip] >> > > >> > > > >> > > > >> > > > Looks good now. >> > > > >> > > > Reviewed-by: Archit Taneja <architt@codeaurora.org> >> > > >> > > And IMHO it would be really good to get this one back-ported to 4.8 >> > > because it really fixes kernel crash if ARC PGU driver is used. >> > > >> > > It might be a bit of a problem because patch itself a little-bit larger >> > > than formal requirement for stable backports but let's see if it gets accepted. >> > >> > Could you please pick this one up? >> > I may alternatively send a pull-request to David but not sure if 1 patch worth it. >> > >> > Also if that's not really too late it would be good to get this one in 4.9 since the patch >> > In question fixes a real driver crash on its instantiation. >> > Actually driver crash happens since 4.8 but I failed to notice it earlier and given amount >> > of changes I think there's barely a chance for it it to be accepted in stable branches... >> > which in its turn makes at least 4.9 very desirable. >> >> Any chance this one gets accepted anytime soon? > > Please treat this as another polite reminder to apply this patch. > If you prefer I may send a pull-request otherwise. Please send a pull request for -fixes. For everyone else, pull requests for single patches is not overkill, it fits into my workflow a lot better. Dave.
diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c index b7a8b2a..b69c66b 100644 --- a/drivers/gpu/drm/arc/arcpgu_hdmi.c +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -14,170 +14,45 @@ * */ -#include <drm/drm_crtc_helper.h> +#include <drm/drm_crtc.h> #include <drm/drm_encoder_slave.h> -#include <drm/drm_atomic_helper.h> #include "arcpgu.h" -struct arcpgu_drm_connector { - struct drm_connector connector; - struct drm_encoder_slave *encoder_slave; -}; - -static int arcpgu_drm_connector_get_modes(struct drm_connector *connector) -{ - const struct drm_encoder_slave_funcs *sfuncs; - struct drm_encoder_slave *slave; - struct arcpgu_drm_connector *con = - container_of(connector, struct arcpgu_drm_connector, connector); - - slave = con->encoder_slave; - if (slave == NULL) { - dev_err(connector->dev->dev, - "connector_get_modes: cannot find slave encoder for connector\n"); - return 0; - } - - sfuncs = slave->slave_funcs; - if (sfuncs->get_modes == NULL) - return 0; - - return sfuncs->get_modes(&slave->base, connector); -} - -static enum drm_connector_status -arcpgu_drm_connector_detect(struct drm_connector *connector, bool force) -{ - enum drm_connector_status status = connector_status_unknown; - const struct drm_encoder_slave_funcs *sfuncs; - struct drm_encoder_slave *slave; - - struct arcpgu_drm_connector *con = - container_of(connector, struct arcpgu_drm_connector, connector); - - slave = con->encoder_slave; - if (slave == NULL) { - dev_err(connector->dev->dev, - "connector_detect: cannot find slave encoder for connector\n"); - return status; - } - - sfuncs = slave->slave_funcs; - if (sfuncs && sfuncs->detect) - return sfuncs->detect(&slave->base, connector); - - dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); - return status; -} - -static void arcpgu_drm_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - -static const struct drm_connector_helper_funcs -arcpgu_drm_connector_helper_funcs = { - .get_modes = arcpgu_drm_connector_get_modes, -}; - -static const struct drm_connector_funcs arcpgu_drm_connector_funcs = { - .dpms = drm_helper_connector_dpms, - .reset = drm_atomic_helper_connector_reset, - .detect = arcpgu_drm_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = arcpgu_drm_connector_destroy, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = { - .dpms = drm_i2c_encoder_dpms, - .mode_fixup = drm_i2c_encoder_mode_fixup, - .mode_set = drm_i2c_encoder_mode_set, - .prepare = drm_i2c_encoder_prepare, - .commit = drm_i2c_encoder_commit, - .detect = drm_i2c_encoder_detect, -}; - static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = { .destroy = drm_encoder_cleanup, }; int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np) { - struct arcpgu_drm_connector *arcpgu_connector; - struct drm_i2c_encoder_driver *driver; - struct drm_encoder_slave *encoder; - struct drm_connector *connector; - struct i2c_client *i2c_slave; - int ret; + struct drm_encoder *encoder; + struct drm_bridge *bridge; + + int ret = 0; encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL); if (encoder == NULL) return -ENOMEM; - i2c_slave = of_find_i2c_device_by_node(np); - if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) { - dev_err(drm->dev, "failed to find i2c slave encoder\n"); - return -EPROBE_DEFER; - } - - if (i2c_slave->dev.driver == NULL) { - dev_err(drm->dev, "failed to find i2c slave driver\n"); + /* Locate drm bridge from the hdmi encoder DT node */ + bridge = of_drm_find_bridge(np); + if (!bridge) return -EPROBE_DEFER; - } - driver = - to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver)); - ret = driver->encoder_init(i2c_slave, drm, encoder); - if (ret) { - dev_err(drm->dev, "failed to initialize i2c encoder slave\n"); - return ret; - } - - encoder->base.possible_crtcs = 1; - encoder->base.possible_clones = 0; - ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs, + encoder->possible_crtcs = 1; + encoder->possible_clones = 0; + ret = drm_encoder_init(drm, encoder, &arcpgu_drm_encoder_funcs, DRM_MODE_ENCODER_TMDS, NULL); if (ret) return ret; - drm_encoder_helper_add(&encoder->base, - &arcpgu_drm_encoder_helper_funcs); - - arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector), - GFP_KERNEL); - if (!arcpgu_connector) { - ret = -ENOMEM; - goto error_encoder_cleanup; - } - - connector = &arcpgu_connector->connector; - drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs); - ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA); - if (ret < 0) { - dev_err(drm->dev, "failed to initialize drm connector\n"); - goto error_encoder_cleanup; - } + /* Link drm_bridge to encoder */ + bridge->encoder = encoder; + encoder->bridge = bridge; - ret = drm_mode_connector_attach_encoder(connector, &encoder->base); - if (ret < 0) { - dev_err(drm->dev, "could not attach connector to encoder\n"); - drm_connector_unregister(connector); - goto error_connector_cleanup; - } - - arcpgu_connector->encoder_slave = encoder; - - return 0; - -error_connector_cleanup: - drm_connector_cleanup(connector); + ret = drm_bridge_attach(drm, bridge); + if (ret) + drm_encoder_cleanup(encoder); -error_encoder_cleanup: - drm_encoder_cleanup(&encoder->base); return ret; }
ARC PGU driver starts crashing on initialization after 'commit e12c2f645557 ("drm/i2c: adv7511: Convert to drm_bridge")' This happenes because in "arcpgu_drm_hdmi_init" function we get pointer of "drm_i2c_encoder_driver" structure, which doesn't exist after adv7511 hdmi encoder interface changed from slave encoder to drm bridge. So, when we call "encoder_init" function from this structure driver crashes. Bootlog: ------------------------------------->8-------------------------------- [drm] Initialized drm 1.1.0 20060810 arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab arcpgu e0017000.pgu: assigned reserved memory node frame_buffer@9e000000 Path: (null) CPU: 0 PID: 1 Comm: swapper Not tainted 4.8.0-00001-gb5642252fa01-dirty #8 task: 9a058000 task.stack: 9a032000 [ECR ]: 0x00220100 => Invalid Read @ 0x00000004 by insn @ 0x803934e8 [EFA ]: 0x00000004 [BLINK ]: drm_atomic_helper_connector_dpms+0xa6/0x230 [ERET ]: drm_atomic_helper_connector_dpms+0xa4/0x230 [STAT32]: 0x00000846 : K DE E2 E1 BTA: 0x8016d949 SP: 0x9a033e34 FP: 0x00000000 LPS: 0x8036f6fc LPE: 0x8036f700 LPC: 0x00000000 r00: 0x8063c118 r01: 0x805b98ac r02: 0x00000b11 r03: 0x00000000 r04: 0x9a010f54 r05: 0x00000000 r06: 0x00000001 r07: 0x00000000 r08: 0x00000028 r09: 0x00000001 r10: 0x00000007 r11: 0x00000054 r12: 0x720a3033 Stack Trace: drm_atomic_helper_connector_dpms+0xa4/0x230 arcpgu_drm_hdmi_init+0xbc/0x228 arcpgu_probe+0x168/0x244 platform_drv_probe+0x26/0x64 really_probe+0x1f0/0x32c __driver_attach+0xa8/0xd0 bus_for_each_dev+0x3c/0x74 bus_add_driver+0xc2/0x184 driver_register+0x50/0xec do_one_initcall+0x3a/0x120 kernel_init_freeable+0x108/0x1a0 ------------------------------------->8-------------------------------- Fix ARC PGU driver to be able work with drm bridge hdmi encoder interface. The hdmi connector code isn't needed anymore as we expect the adv7511 bridge driver to create/manage the connector. Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- Changes for v2: - remove bridge functions call from kms driver - get rid of drm_encoder_slave interface - coding style cleanup drivers/gpu/drm/arc/arcpgu_hdmi.c | 159 ++++---------------------------------- 1 file changed, 17 insertions(+), 142 deletions(-)