Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Mon, Apr 28, 2014 at 9:08 AM, Ajay kumar ajayn...@gmail.com wrote: Daniel, On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote: We can call panel_enable/disable at the right point. Even the bridge chips expect the same. So, I am not ok with combining the bridge and panel and calling the fxn pointers from crtc_helpers. So, either we leave it the way it is in this patch (call drm_panel functions at right points) or we don't use drm_panel to control the panel sequence and instead use few DT properties and regulators, inside the bridge chip driver. That wasn't really suggested, but instead the idea was to provide a default drm_bridge which wraps the drm_panel so that you could more easily chain them up. What do you mean by default drm_bridge? I just want to clear few things. Let me explain what I have understood: What I understand is that Rob wants me to create a common structure which wraps up drm_panel and drm_bridge into a single structure: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ == Really not sure why this is needed! }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } And now, the above function becomes a common function (also with an ordering issue btw panel and bridge). Where do we keep it? And, where do we call it from? my proposal involves no change to crtc helpers. The later variation (with composite_bridge so you could choose the order) would have a sequence along the lines of: crtc helpers: + bridge_funcs-enable(): - composite_bridge_enable() + cbridge-b1-funcs-enable(): - ptn3460_bridge_enable() + cbridge-b2-funcs-enable(): - panel_bridge_enable(); + pbridge-panel-funcs-enable(): - foo_panel_enable() or if the order needs to be swapped you can use ptn3460_bridge as b2 and panel bridge as b1. BR, -R Also I'm not really happy that the bridge callbacks have been added to the drm helpers (I'd prefer if driver callbacks would bear that responsibility). But you can always create your own drm_bridge integration. Do you mean, the bridge calls should be moved out of common drm_helper functions and called from platform specific drivers instead? In any case your concern that drivers need to control when certain callbacks are called is shared by everyone here afaics. And I also don't see any issues with Rob's proposal in this regard. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch Please let me know if my understanding is correct and correct me if there is a misconception. Regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
Daniel, On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote: We can call panel_enable/disable at the right point. Even the bridge chips expect the same. So, I am not ok with combining the bridge and panel and calling the fxn pointers from crtc_helpers. So, either we leave it the way it is in this patch (call drm_panel functions at right points) or we don't use drm_panel to control the panel sequence and instead use few DT properties and regulators, inside the bridge chip driver. That wasn't really suggested, but instead the idea was to provide a default drm_bridge which wraps the drm_panel so that you could more easily chain them up. What do you mean by default drm_bridge? I just want to clear few things. Let me explain what I have understood: What I understand is that Rob wants me to create a common structure which wraps up drm_panel and drm_bridge into a single structure: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ == Really not sure why this is needed! }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } And now, the above function becomes a common function (also with an ordering issue btw panel and bridge). Where do we keep it? And, where do we call it from? Also I'm not really happy that the bridge callbacks have been added to the drm helpers (I'd prefer if driver callbacks would bear that responsibility). But you can always create your own drm_bridge integration. Do you mean, the bridge calls should be moved out of common drm_helper functions and called from platform specific drivers instead? In any case your concern that drivers need to control when certain callbacks are called is shared by everyone here afaics. And I also don't see any issues with Rob's proposal in this regard. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch Please let me know if my understanding is correct and correct me if there is a misconception. Regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar ajayn...@gmail.com wrote: We can call panel_enable/disable at the right point. Even the bridge chips expect the same. So, I am not ok with combining the bridge and panel and calling the fxn pointers from crtc_helpers. So, either we leave it the way it is in this patch (call drm_panel functions at right points) or we don't use drm_panel to control the panel sequence and instead use few DT properties and regulators, inside the bridge chip driver. That wasn't really suggested, but instead the idea was to provide a default drm_bridge which wraps the drm_panel so that you could more easily chain them up. Also I'm not really happy that the bridge callbacks have been added to the drm helpers (I'd prefer if driver callbacks would bear that responsibility). But you can always create your own drm_bridge integration. In any case your concern that drivers need to control when certain callbacks are called is shared by everyone here afaics. And I also don't see any issues with Rob's proposal in this regard. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Thu, Apr 24, 2014 at 11:08 PM, Ajay kumar ajayn...@gmail.com wrote: On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote: On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote: Rob, On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. These calls are for a bridge to sync with the encoder calls. We might end up defining pre-enable/enable/disable/post-disable for a panel to sync with the bridge calls! I have explained below. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) Ok. Consider this case: Currently, we have the sequence as bridge-pre_enable, encoder_enable, bridge-enable And, the bridge should obviously be enabled in bridge-pre_enable. Now, where do you choose to call panel-pre_enable? -- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs. -- at the last step of bridge-pre_enable, after we pull up/down the bridge GPIOs. Ideally, PTN3460 expects it to be the second case, and PS8625 expects it to be the first case. So, we will end up adding pre_pre_enable, post_pre_enable to accomodate such scenarios. ok, well I think we can deal with this with a slight modification of my original proposal. Split drm_panel_bridge into drm_composite_bridge and drm_panel_bridge: struct drm_composite_bridge { struct drm_bridge base; struct drm_bridge *b1, *b2; } static void drm_composite_bridge_enable(struct drm_bridge *bridge) { struct drm_composite_bridge *cbridge = to_composite_bridge(bridge); cbridge-b1-funcs-enable(cbridge-b1); cbridge-b2-funcs-enable(cbridge-b2); } .. and so on .. struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; } static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pbridge = to_panel_bridge(bridge); pbridge-panel-funcs-enable(pbridge-panel); } .. and so on.. then in initialization, order can be managed like: if (panel_first) bridge = drm_composite_bridge_init(panel_bridge, actual_bridge) else bridge = drm_composite_bridge_init(actual_bridge, panel_bridge); possibly parametrize drm_panel_bridge if we need to map panel-enable() to bridge-pre_enable() vs bridge-enable(), etc.. Well, this really does seems complex to me. Don't you think just having a drm_panel inside drm_bridge structure is sufficient?! And, make a call for pre_panel_enable and post_panel_enable around bridge-pre_enable..and so on.? Adding more comments: The beauty of drm_panel is in the flexibility which it provides. We can call panel_enable/disable at the right point. Even the -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
Sorry for the previous reply. Here goes the full explaination. On Fri, Apr 25, 2014 at 11:40 AM, Ajay kumar ajayn...@gmail.com wrote: On Thu, Apr 24, 2014 at 11:08 PM, Ajay kumar ajayn...@gmail.com wrote: On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote: On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote: Rob, On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. These calls are for a bridge to sync with the encoder calls. We might end up defining pre-enable/enable/disable/post-disable for a panel to sync with the bridge calls! I have explained below. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) Ok. Consider this case: Currently, we have the sequence as bridge-pre_enable, encoder_enable, bridge-enable And, the bridge should obviously be enabled in bridge-pre_enable. Now, where do you choose to call panel-pre_enable? -- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs. -- at the last step of bridge-pre_enable, after we pull up/down the bridge GPIOs. Ideally, PTN3460 expects it to be the second case, and PS8625 expects it to be the first case. So, we will end up adding pre_pre_enable, post_pre_enable to accomodate such scenarios. ok, well I think we can deal with this with a slight modification of my original proposal. Split drm_panel_bridge into drm_composite_bridge and drm_panel_bridge: struct drm_composite_bridge { struct drm_bridge base; struct drm_bridge *b1, *b2; } static void drm_composite_bridge_enable(struct drm_bridge *bridge) { struct drm_composite_bridge *cbridge = to_composite_bridge(bridge); cbridge-b1-funcs-enable(cbridge-b1); cbridge-b2-funcs-enable(cbridge-b2); } .. and so on .. struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; } static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pbridge = to_panel_bridge(bridge); pbridge-panel-funcs-enable(pbridge-panel); } .. and so on.. then in initialization, order can be managed like: if (panel_first) bridge = drm_composite_bridge_init(panel_bridge, actual_bridge) else bridge = drm_composite_bridge_init(actual_bridge, panel_bridge); possibly parametrize drm_panel_bridge if we need to map panel-enable() to bridge-pre_enable() vs bridge-enable(), etc.. Well, this really does seems complex to me. Don't you think just having a drm_panel inside drm_bridge structure is sufficient?! And, make a call for pre_panel_enable and post_panel_enable around bridge-pre_enable..and so on.? Adding more comments: The beauty of drm_panel is in the flexibility which it provides. We can call panel_enable/disable at the right point. Even the bridge chips expect the same. So, I am not ok with combining the bridge and panel and calling the fxn pointers from crtc_helpers. So, either we leave it the way it is in this patch (call drm_panel functions at right points) or we don't use drm_panel to control the panel sequence and instead use few DT properties and regulators, inside the bridge chip driver. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) So, putting these drm_panel controls inside individual bridge drivers will work, but keeping them in crtc_helpers might break stuff. I'm not talking about crtc changing helpers. BR, -R Thanks and regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
Rob, On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. These calls are for a bridge to sync with the encoder calls. We might end up defining pre-enable/enable/disable/post-disable for a panel to sync with the bridge calls! I have explained below. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) Ok. Consider this case: Currently, we have the sequence as bridge-pre_enable, encoder_enable, bridge-enable And, the bridge should obviously be enabled in bridge-pre_enable. Now, where do you choose to call panel-pre_enable? -- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs. -- at the last step of bridge-pre_enable, after we pull up/down the bridge GPIOs. Ideally, PTN3460 expects it to be the second case, and PS8625 expects it to be the first case. So, we will end up adding pre_pre_enable, post_pre_enable to accomodate such scenarios. So, we leave the choice for the individual bridge chip drivers to implement panel power up/down controls in the place where they wish to. Thanks and regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote: Rob, On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. These calls are for a bridge to sync with the encoder calls. We might end up defining pre-enable/enable/disable/post-disable for a panel to sync with the bridge calls! I have explained below. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) Ok. Consider this case: Currently, we have the sequence as bridge-pre_enable, encoder_enable, bridge-enable And, the bridge should obviously be enabled in bridge-pre_enable. Now, where do you choose to call panel-pre_enable? -- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs. -- at the last step of bridge-pre_enable, after we pull up/down the bridge GPIOs. Ideally, PTN3460 expects it to be the second case, and PS8625 expects it to be the first case. So, we will end up adding pre_pre_enable, post_pre_enable to accomodate such scenarios. ok, well I think we can deal with this with a slight modification of my original proposal. Split drm_panel_bridge into drm_composite_bridge and drm_panel_bridge: struct drm_composite_bridge { struct drm_bridge base; struct drm_bridge *b1, *b2; } static void drm_composite_bridge_enable(struct drm_bridge *bridge) { struct drm_composite_bridge *cbridge = to_composite_bridge(bridge); cbridge-b1-funcs-enable(cbridge-b1); cbridge-b2-funcs-enable(cbridge-b2); } .. and so on .. struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; } static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pbridge = to_panel_bridge(bridge); pbridge-panel-funcs-enable(pbridge-panel); } .. and so on.. then in initialization, order can be managed like: if (panel_first) bridge = drm_composite_bridge_init(panel_bridge, actual_bridge) else bridge = drm_composite_bridge_init(actual_bridge, panel_bridge); possibly parametrize drm_panel_bridge if we need to map panel-enable() to bridge-pre_enable() vs bridge-enable(), etc.. BR, -R So, we leave the choice for the individual bridge chip drivers to implement panel power up/down controls in the place where they wish to. Thanks and regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Thu, Apr 24, 2014 at 10:55 PM, Rob Clark robdcl...@gmail.com wrote: On Thu, Apr 24, 2014 at 12:55 PM, Ajay kumar ajayn...@gmail.com wrote: Rob, On Thu, Apr 24, 2014 at 9:41 PM, Rob Clark robdcl...@gmail.com wrote: On Wed, Apr 23, 2014 at 3:02 PM, Ajay kumar ajayn...@gmail.com wrote: Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) well, this is why bridge has pre-enable/enable/disable/post-disable hooks, so you can choose before or after.. These calls are for a bridge to sync with the encoder calls. We might end up defining pre-enable/enable/disable/post-disable for a panel to sync with the bridge calls! I have explained below. -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. I am not convinced that a generic panel/bridge adapter is not possible. Maybe we need more fine grained fxn ptr callbacks, although seems like pre+post should give you enough. It would certainly be easier than having to add panel support in each individual bridge driver (which seems like it will certainly result that only certain combinations of panel+bridge actually work as intended) Ok. Consider this case: Currently, we have the sequence as bridge-pre_enable, encoder_enable, bridge-enable And, the bridge should obviously be enabled in bridge-pre_enable. Now, where do you choose to call panel-pre_enable? -- as the first step of bridge-pre_enable, before we pull up/down bridge GPIOs. -- at the last step of bridge-pre_enable, after we pull up/down the bridge GPIOs. Ideally, PTN3460 expects it to be the second case, and PS8625 expects it to be the first case. So, we will end up adding pre_pre_enable, post_pre_enable to accomodate such scenarios. ok, well I think we can deal with this with a slight modification of my original proposal. Split drm_panel_bridge into drm_composite_bridge and drm_panel_bridge: struct drm_composite_bridge { struct drm_bridge base; struct drm_bridge *b1, *b2; } static void drm_composite_bridge_enable(struct drm_bridge *bridge) { struct drm_composite_bridge *cbridge = to_composite_bridge(bridge); cbridge-b1-funcs-enable(cbridge-b1); cbridge-b2-funcs-enable(cbridge-b2); } .. and so on .. struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; } static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pbridge = to_panel_bridge(bridge); pbridge-panel-funcs-enable(pbridge-panel); } .. and so on.. then in initialization, order can be managed like: if (panel_first) bridge = drm_composite_bridge_init(panel_bridge, actual_bridge) else bridge = drm_composite_bridge_init(actual_bridge, panel_bridge); possibly parametrize drm_panel_bridge if we need to map panel-enable() to bridge-pre_enable() vs bridge-enable(), etc.. Well, this really does seems complex to me. Don't you think just having a drm_panel inside drm_bridge structure is sufficient?! And, make a call for pre_panel_enable and post_panel_enable around bridge-pre_enable..and so on.? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, sometimes panel controls needs to be done before bridge controls, ... and so on. If you don't need a real bridge, just create one of these w/ pb-bridge==NULL, otherwise create it as a wrapper for your real bridge. BR, -R On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote: attach ptn3460 connector to drm_panel and support drm_panel routines, if a valid drm_panel object is passed to ptn3460_init. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: Address few coding style comments from Jingoo Han drivers/gpu/drm/bridge/Kconfig |1 + drivers/gpu/drm/bridge/ptn3460.c| 20 +++- drivers/gpu/drm/exynos/exynos_dp_core.c | 16 include/drm/bridge/ptn3460.h|6 -- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..3bc6845 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -2,4 +2,5 @@ config DRM_PTN3460 tristate PTN3460 DP/LVDS bridge depends on DRM select DRM_KMS_HELPER + select DRM_PANEL ---help--- diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index f1d2afc..3920202 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/gpio.h #include linux/delay.h +#include drm/drm_panel.h #include drmP.h #include drm_edid.h @@ -38,6 +39,7 @@ struct ptn3460_bridge { struct i2c_client *client; struct drm_encoder *encoder; struct drm_bridge *bridge; + struct drm_panel *panel; struct edid *edid; int gpio_pd_n; int gpio_rst_n; @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) gpio_set_value(ptn_bridge-gpio_rst_n, 1); } + drm_panel_pre_enable(ptn_bridge-panel); + /* * There's a bug in the PTN chip where it falsely asserts hotplug before * it is fully functional. We're forced to wait for the maximum start up @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) static void ptn3460_enable(struct drm_bridge *bridge) { + struct ptn3460_bridge *ptn_bridge = bridge-driver_private; + + if (ptn_bridge-enabled) + drm_panel_enable(ptn_bridge-panel); } static void ptn3460_disable(struct drm_bridge *bridge) @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge) ptn_bridge-enabled = false; + drm_panel_disable(ptn_bridge-panel); + drm_panel_post_disable(ptn_bridge-panel); + if (gpio_is_valid(ptn_bridge-gpio_rst_n)) gpio_set_value(ptn_bridge-gpio_rst_n, 1); @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector) power_off = !ptn_bridge-enabled; ptn3460_pre_enable(ptn_bridge-bridge); + ptn3460_enable(ptn_bridge-bridge); edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (!edid) { @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { }; int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, - struct i2c_client *client, struct device_node *node) + struct i2c_client *client, struct device_node *node, + struct drm_panel *panel) { int ret; struct drm_bridge *bridge; @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; } + if (panel) { + ptn_bridge-panel = panel; + drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector); + } + bridge-driver_private = ptn_bridge; encoder-bridge = bridge; ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD; diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index dbc5ccc..4853f31 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
Sorry for the previous reply, Here goes the full explaination: Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark robdcl...@gmail.com wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, -- sometimes panel controls needs to be done before bridge controls(ptn3460: before RST_N and PD_N) -- sometimes in between the bridge controls (ps8622: one panel control before SLP_N and one after SLP_N) -- sometimes panel controls needs to be done after bridge controls. So, putting these drm_panel controls inside individual bridge drivers will work, but keeping them in crtc_helpers might break stuff. Thanks and regards, Ajay Kumar -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } ... and so on. If you don't need a real bridge, just create one of these w/ pb-bridge==NULL, otherwise create it as a wrapper for your real bridge. BR, -R On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote: attach ptn3460 connector to drm_panel and support drm_panel routines, if a valid drm_panel object is passed to ptn3460_init. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: Address few coding style comments from Jingoo Han drivers/gpu/drm/bridge/Kconfig |1 + drivers/gpu/drm/bridge/ptn3460.c| 20 +++- drivers/gpu/drm/exynos/exynos_dp_core.c | 16 include/drm/bridge/ptn3460.h|6 -- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..3bc6845 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -2,4 +2,5 @@ config DRM_PTN3460 tristate PTN3460 DP/LVDS bridge depends on DRM select DRM_KMS_HELPER + select DRM_PANEL ---help--- diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index f1d2afc..3920202 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/gpio.h #include linux/delay.h +#include drm/drm_panel.h #include drmP.h #include drm_edid.h @@ -38,6 +39,7 @@ struct ptn3460_bridge { struct i2c_client *client; struct drm_encoder *encoder; struct drm_bridge *bridge; + struct drm_panel *panel; struct edid *edid; int gpio_pd_n; int gpio_rst_n; @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) gpio_set_value(ptn_bridge-gpio_rst_n, 1); } + drm_panel_pre_enable(ptn_bridge-panel); + /* * There's a bug in the PTN chip where it falsely asserts hotplug before * it is fully functional. We're forced to wait for the maximum start up @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) static void ptn3460_enable(struct drm_bridge *bridge) { + struct ptn3460_bridge *ptn_bridge = bridge-driver_private; + + if (ptn_bridge-enabled) + drm_panel_enable(ptn_bridge-panel); } static void ptn3460_disable(struct drm_bridge *bridge) @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge) ptn_bridge-enabled = false; + drm_panel_disable(ptn_bridge-panel); + drm_panel_post_disable(ptn_bridge-panel); + if (gpio_is_valid(ptn_bridge-gpio_rst_n)) gpio_set_value(ptn_bridge-gpio_rst_n, 1); @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector) power_off = !ptn_bridge-enabled; ptn3460_pre_enable(ptn_bridge-bridge); + ptn3460_enable(ptn_bridge-bridge); edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (!edid) { @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { }; int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, - struct i2c_client *client, struct device_node *node) + struct i2c_client *client, struct device_node *node, + struct drm_panel *panel) { int ret; struct drm_bridge *bridge; @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; } + if (panel) { + ptn_bridge-panel = panel; + drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector); + } + bridge-driver_private = ptn_bridge; encoder-bridge = bridge; ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD; diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index dbc5ccc..4853f31 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct bridge_init *bridge) /* returns the number of bridges attached */ static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, - struct drm_encoder *encoder) + struct drm_encoder *encoder, struct
Re: [PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
On Tue, Apr 22, 2014 at 07:34:03AM -0400, Rob Clark wrote: So what about, rather than adding drm_panel support to each bridge individually, we introduce a drm_panel_bridge (with a form of chaining).. ie: struct drm_panel_bridge { struct drm_bridge base; struct drm_panel *panel; struct drm_bridge *bridge; /* optional */ }; static void drm_panel_bridge_enable(struct drm_bridge *bridge) { struct drm_panel_bridge *pb = to_panel_bridge(bridge); if (pb-bridge) pb-bridge-funcs-enable(pb-bridge); pb-panel-funcs-enable(pb-panel); } ... and so on. If you don't need a real bridge, just create one of these w/ pb-bridge==NULL, otherwise create it as a wrapper for your real bridge. Yeah I think that's how I'd have implemented some generic abstraction for drivers using the crtc helpers. This allows you to keep bridge drivers, panel drivers and anything else you might have in your driver to feed the pixel stream to those 2 guys separate. And it also allows you to set it all up in different ways, e.g. using device tree metadata, or acpi or some other table hardcoded in a video rom somewhere. Imo we also should have something similar to chain up drm_bridge devices. tbh I'm not terribly happy about how the current integration with the crtc modeset helpers is done ... -Daniel BR, -R On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar ajaykumar...@samsung.com wrote: attach ptn3460 connector to drm_panel and support drm_panel routines, if a valid drm_panel object is passed to ptn3460_init. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: Address few coding style comments from Jingoo Han drivers/gpu/drm/bridge/Kconfig |1 + drivers/gpu/drm/bridge/ptn3460.c| 20 +++- drivers/gpu/drm/exynos/exynos_dp_core.c | 16 include/drm/bridge/ptn3460.h|6 -- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..3bc6845 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -2,4 +2,5 @@ config DRM_PTN3460 tristate PTN3460 DP/LVDS bridge depends on DRM select DRM_KMS_HELPER + select DRM_PANEL ---help--- diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index f1d2afc..3920202 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/gpio.h #include linux/delay.h +#include drm/drm_panel.h #include drmP.h #include drm_edid.h @@ -38,6 +39,7 @@ struct ptn3460_bridge { struct i2c_client *client; struct drm_encoder *encoder; struct drm_bridge *bridge; + struct drm_panel *panel; struct edid *edid; int gpio_pd_n; int gpio_rst_n; @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) gpio_set_value(ptn_bridge-gpio_rst_n, 1); } + drm_panel_pre_enable(ptn_bridge-panel); + /* * There's a bug in the PTN chip where it falsely asserts hotplug before * it is fully functional. We're forced to wait for the maximum start up @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) static void ptn3460_enable(struct drm_bridge *bridge) { + struct ptn3460_bridge *ptn_bridge = bridge-driver_private; + + if (ptn_bridge-enabled) + drm_panel_enable(ptn_bridge-panel); } static void ptn3460_disable(struct drm_bridge *bridge) @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge) ptn_bridge-enabled = false; + drm_panel_disable(ptn_bridge-panel); + drm_panel_post_disable(ptn_bridge-panel); + if (gpio_is_valid(ptn_bridge-gpio_rst_n)) gpio_set_value(ptn_bridge-gpio_rst_n, 1); @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector) power_off = !ptn_bridge-enabled; ptn3460_pre_enable(ptn_bridge-bridge); + ptn3460_enable(ptn_bridge-bridge); edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (!edid) { @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { }; int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, - struct i2c_client *client, struct device_node *node) + struct i2c_client *client, struct device_node *node, + struct drm_panel *panel) { int ret; struct drm_bridge *bridge; @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; } + if (panel) { +
[PATCH V2 7/9] drm/bridge: ptn3460: add drm_panel controls
attach ptn3460 connector to drm_panel and support drm_panel routines, if a valid drm_panel object is passed to ptn3460_init. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: Address few coding style comments from Jingoo Han drivers/gpu/drm/bridge/Kconfig |1 + drivers/gpu/drm/bridge/ptn3460.c| 20 +++- drivers/gpu/drm/exynos/exynos_dp_core.c | 16 include/drm/bridge/ptn3460.h|6 -- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 884923f..3bc6845 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -2,4 +2,5 @@ config DRM_PTN3460 tristate PTN3460 DP/LVDS bridge depends on DRM select DRM_KMS_HELPER + select DRM_PANEL ---help--- diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index f1d2afc..3920202 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/gpio.h #include linux/delay.h +#include drm/drm_panel.h #include drmP.h #include drm_edid.h @@ -38,6 +39,7 @@ struct ptn3460_bridge { struct i2c_client *client; struct drm_encoder *encoder; struct drm_bridge *bridge; + struct drm_panel *panel; struct edid *edid; int gpio_pd_n; int gpio_rst_n; @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) gpio_set_value(ptn_bridge-gpio_rst_n, 1); } + drm_panel_pre_enable(ptn_bridge-panel); + /* * There's a bug in the PTN chip where it falsely asserts hotplug before * it is fully functional. We're forced to wait for the maximum start up @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) static void ptn3460_enable(struct drm_bridge *bridge) { + struct ptn3460_bridge *ptn_bridge = bridge-driver_private; + + if (ptn_bridge-enabled) + drm_panel_enable(ptn_bridge-panel); } static void ptn3460_disable(struct drm_bridge *bridge) @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge) ptn_bridge-enabled = false; + drm_panel_disable(ptn_bridge-panel); + drm_panel_post_disable(ptn_bridge-panel); + if (gpio_is_valid(ptn_bridge-gpio_rst_n)) gpio_set_value(ptn_bridge-gpio_rst_n, 1); @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector) power_off = !ptn_bridge-enabled; ptn3460_pre_enable(ptn_bridge-bridge); + ptn3460_enable(ptn_bridge-bridge); edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (!edid) { @@ -265,7 +277,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { }; int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, - struct i2c_client *client, struct device_node *node) + struct i2c_client *client, struct device_node *node, + struct drm_panel *panel) { int ret; struct drm_bridge *bridge; @@ -324,6 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; } + if (panel) { + ptn_bridge-panel = panel; + drm_panel_attach(ptn_bridge-panel, ptn_bridge-connector); + } + bridge-driver_private = ptn_bridge; encoder-bridge = bridge; ptn_bridge-connector.polled = DRM_CONNECTOR_POLL_HPD; diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index dbc5ccc..4853f31 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct bridge_init *bridge) /* returns the number of bridges attached */ static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, - struct drm_encoder *encoder) + struct drm_encoder *encoder, struct drm_panel *panel) { struct bridge_init bridge; int ret; if (find_bridge(nxp,ptn3460, bridge)) { - ret = ptn3460_init(dev, encoder, bridge.client, bridge.node); + ret = ptn3460_init(dev, encoder, bridge.client, bridge.node, + panel); if (!ret) return 1; } @@ -1012,9 +1013,16 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display, dp-encoder = encoder; /* Pre-empt DP connector creation if there's a bridge */ - ret = exynos_drm_attach_lcd_bridge(dp-drm_dev, encoder); - if (ret) + ret = exynos_drm_attach_lcd_bridge(dp-drm_dev, encoder, dp-drm_panel); + if (ret) { + /* +* Also set dp-drm_panel = NULL so that we