| Message ID | 1482334144-9223-3-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Shashank, On 21-12-2016 15:29, Shashank Sharma wrote: > This patch adds a small helper function, which clears the cached > information about a hot-pluggable display, from connector. On event > This will run on event of a hot-unplug, keeping the connector's display > info up-to-date, avoiding any errors due to invalid cached data. > > Cc: Jose Abreu <joabreu@synopsys.com> > > Suggested-by: Jose Abreu <joabreu@synopsys.com> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 7cff91e..9e97b45 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) > } > > /** > + * drm_helper_clear_display_info - clean cached display information for > + * hot pluggable displays, on event of hot-unplug > + * @connector: connector under event > + */ > +void drm_helper_clear_display_info(struct drm_connector *connector) > +{ > + struct drm_display_info *info = &connector->display_info; > + > + memset(info, 0, sizeof(*info)); > +} > + > +/** > * drm_helper_probe_single_connector_modes - get complete set of display modes > * @connector: connector to probe > * @maxX: max width for modes > @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", > connector->base.id, connector->name); > drm_mode_connector_update_edid_property(connector, NULL); > + > + /* > + * Connector status change to disconnected, time to clean > + * cached display information > + */ > + if (connector->status == connector_status_disconnected) > + drm_helper_clear_display_info(connector); > + I don't know if this is the right place to do this because it is a helper and I don't know if it is used by all the drivers. We may need something more general that is always called when probing modes, or force drivers that don't use the helper to use the drm_helper_clear_display_info function. As I told you before, I'm new to dri-devel so we need more comments. Best regards, Jose Miguel Abreu > verbose_prune = false; > goto prune; > }
On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote: > Hi Shashank, > > > On 21-12-2016 15:29, Shashank Sharma wrote: > > This patch adds a small helper function, which clears the cached > > information about a hot-pluggable display, from connector. On event > > This will run on event of a hot-unplug, keeping the connector's display > > info up-to-date, avoiding any errors due to invalid cached data. > > > > Cc: Jose Abreu <joabreu@synopsys.com> > > > > Suggested-by: Jose Abreu <joabreu@synopsys.com> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > > --- > > drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > > index 7cff91e..9e97b45 100644 > > --- a/drivers/gpu/drm/drm_probe_helper.c > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) > > } > > > > /** > > + * drm_helper_clear_display_info - clean cached display information for > > + * hot pluggable displays, on event of hot-unplug > > + * @connector: connector under event > > + */ > > +void drm_helper_clear_display_info(struct drm_connector *connector) > > +{ > > + struct drm_display_info *info = &connector->display_info; > > + > > + memset(info, 0, sizeof(*info)); > > +} > > + > > +/** > > * drm_helper_probe_single_connector_modes - get complete set of display modes > > * @connector: connector to probe > > * @maxX: max width for modes > > @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", > > connector->base.id, connector->name); > > drm_mode_connector_update_edid_property(connector, NULL); > > + > > + /* > > + * Connector status change to disconnected, time to clean > > + * cached display information > > + */ > > + if (connector->status == connector_status_disconnected) > > + drm_helper_clear_display_info(connector); > > + > > I don't know if this is the right place to do this because it is > a helper and I don't know if it is used by all the drivers. We > may need something more general that is always called when > probing modes, or force drivers that don't use the helper to use > the drm_helper_clear_display_info function. As I told you before, > I'm new to dri-devel so we need more comments. Seems reasonable to me, since afaik all drivers do use the probe helpers. -Daniel > > Best regards, > Jose Miguel Abreu > > > verbose_prune = false; > > goto prune; > > } > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Regards Shashank On 12/27/2016 3:07 PM, Daniel Vetter wrote: > On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote: >> Hi Shashank, >> >> >> On 21-12-2016 15:29, Shashank Sharma wrote: >>> This patch adds a small helper function, which clears the cached >>> information about a hot-pluggable display, from connector. On event >>> This will run on event of a hot-unplug, keeping the connector's display >>> info up-to-date, avoiding any errors due to invalid cached data. >>> >>> Cc: Jose Abreu <joabreu@synopsys.com> >>> >>> Suggested-by: Jose Abreu <joabreu@synopsys.com> >>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >>> --- >>> drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >>> index 7cff91e..9e97b45 100644 >>> --- a/drivers/gpu/drm/drm_probe_helper.c >>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>> @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) >>> } >>> >>> /** >>> + * drm_helper_clear_display_info - clean cached display information for >>> + * hot pluggable displays, on event of hot-unplug >>> + * @connector: connector under event >>> + */ >>> +void drm_helper_clear_display_info(struct drm_connector *connector) >>> +{ >>> + struct drm_display_info *info = &connector->display_info; >>> + >>> + memset(info, 0, sizeof(*info)); >>> +} >>> + >>> +/** >>> * drm_helper_probe_single_connector_modes - get complete set of display modes >>> * @connector: connector to probe >>> * @maxX: max width for modes >>> @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, >>> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", >>> connector->base.id, connector->name); >>> drm_mode_connector_update_edid_property(connector, NULL); >>> + >>> + /* >>> + * Connector status change to disconnected, time to clean >>> + * cached display information >>> + */ >>> + if (connector->status == connector_status_disconnected) >>> + drm_helper_clear_display_info(connector); >>> + >> I don't know if this is the right place to do this because it is >> a helper and I don't know if it is used by all the drivers. We >> may need something more general that is always called when >> probing modes, or force drivers that don't use the helper to use >> the drm_helper_clear_display_info function. As I told you before, >> I'm new to dri-devel so we need more comments. > Seems reasonable to me, since afaik all drivers do use the probe helpers. > -Daniel This was my understanding too. Jose, you think there would be any drivers who dont use this probe ? - Shashank >> Best regards, >> Jose Miguel Abreu >> >>> verbose_prune = false; >>> goto prune; >>> } >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Shashank, On 29-12-2016 05:53, Sharma, Shashank wrote: > Regards > > Shashank > > > On 12/27/2016 3:07 PM, Daniel Vetter wrote: >> On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote: >>> Hi Shashank, >>> >>> >>> On 21-12-2016 15:29, Shashank Sharma wrote: >>>> This patch adds a small helper function, which clears the >>>> cached >>>> information about a hot-pluggable display, from connector. >>>> On event >>>> This will run on event of a hot-unplug, keeping the >>>> connector's display >>>> info up-to-date, avoiding any errors due to invalid cached >>>> data. >>>> >>>> Cc: Jose Abreu <joabreu@synopsys.com> >>>> >>>> Suggested-by: Jose Abreu <joabreu@synopsys.com> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c >>>> b/drivers/gpu/drm/drm_probe_helper.c >>>> index 7cff91e..9e97b45 100644 >>>> --- a/drivers/gpu/drm/drm_probe_helper.c >>>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>>> @@ -164,6 +164,18 @@ void >>>> drm_kms_helper_poll_enable_locked(struct drm_device *dev) >>>> } >>>> /** >>>> + * drm_helper_clear_display_info - clean cached display >>>> information for >>>> + * hot pluggable displays, on event of hot-unplug >>>> + * @connector: connector under event >>>> + */ >>>> +void drm_helper_clear_display_info(struct drm_connector >>>> *connector) >>>> +{ >>>> + struct drm_display_info *info = &connector->display_info; >>>> + >>>> + memset(info, 0, sizeof(*info)); >>>> +} >>>> + >>>> +/** >>>> * drm_helper_probe_single_connector_modes - get complete >>>> set of display modes >>>> * @connector: connector to probe >>>> * @maxX: max width for modes >>>> @@ -288,6 +300,14 @@ int >>>> drm_helper_probe_single_connector_modes(struct drm_connector >>>> *connector, >>>> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", >>>> connector->base.id, connector->name); >>>> drm_mode_connector_update_edid_property(connector, >>>> NULL); >>>> + >>>> + /* >>>> + * Connector status change to disconnected, time to >>>> clean >>>> + * cached display information >>>> + */ >>>> + if (connector->status == >>>> connector_status_disconnected) >>>> + drm_helper_clear_display_info(connector); >>>> + >>> I don't know if this is the right place to do this because it is >>> a helper and I don't know if it is used by all the drivers. We >>> may need something more general that is always called when >>> probing modes, or force drivers that don't use the helper to use >>> the drm_helper_clear_display_info function. As I told you >>> before, >>> I'm new to dri-devel so we need more comments. >> Seems reasonable to me, since afaik all drivers do use the >> probe helpers. >> -Daniel > This was my understanding too. Jose, you think there would be > any drivers who dont use this probe ? > - Shashank I found only one driver that don't use this helper: vmwgfx. But, this driver does not seem to use EDID fields, it has a list of preferred video modes and manually adds these modes. So, I think it is safe to add this in the helper as long as future drivers that use EDID use this helper also. Maybe a small comment about this should be added in the helper declaration. Best regards, Jose Miguel Abreu >>> Best regards, >>> Jose Miguel Abreu >>> >>>> verbose_prune = false; >>>> goto prune; >>>> } >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_intel-2Dgfx&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=1G5dnBp7Y6VEifpEnDT2wKFoDRBXnxGXAnA-4883H74&s=y1M2ce128zpR_lBDPSgS_JGm-HoPIJjneK2s3tkrvyo&e= >> >
Regards Shashank On 12/29/2016 3:35 PM, Jose Abreu wrote: > Hi Shashank, > > > On 29-12-2016 05:53, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 12/27/2016 3:07 PM, Daniel Vetter wrote: >>> On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote: >>>> Hi Shashank, >>>> >>>> >>>> On 21-12-2016 15:29, Shashank Sharma wrote: >>>>> This patch adds a small helper function, which clears the >>>>> cached >>>>> information about a hot-pluggable display, from connector. >>>>> On event >>>>> This will run on event of a hot-unplug, keeping the >>>>> connector's display >>>>> info up-to-date, avoiding any errors due to invalid cached >>>>> data. >>>>> >>>>> Cc: Jose Abreu <joabreu@synopsys.com> >>>>> >>>>> Suggested-by: Jose Abreu <joabreu@synopsys.com> >>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ >>>>> 1 file changed, 20 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c >>>>> b/drivers/gpu/drm/drm_probe_helper.c >>>>> index 7cff91e..9e97b45 100644 >>>>> --- a/drivers/gpu/drm/drm_probe_helper.c >>>>> +++ b/drivers/gpu/drm/drm_probe_helper.c >>>>> @@ -164,6 +164,18 @@ void >>>>> drm_kms_helper_poll_enable_locked(struct drm_device *dev) >>>>> } >>>>> /** >>>>> + * drm_helper_clear_display_info - clean cached display >>>>> information for >>>>> + * hot pluggable displays, on event of hot-unplug >>>>> + * @connector: connector under event >>>>> + */ >>>>> +void drm_helper_clear_display_info(struct drm_connector >>>>> *connector) >>>>> +{ >>>>> + struct drm_display_info *info = &connector->display_info; >>>>> + >>>>> + memset(info, 0, sizeof(*info)); >>>>> +} >>>>> + >>>>> +/** >>>>> * drm_helper_probe_single_connector_modes - get complete >>>>> set of display modes >>>>> * @connector: connector to probe >>>>> * @maxX: max width for modes >>>>> @@ -288,6 +300,14 @@ int >>>>> drm_helper_probe_single_connector_modes(struct drm_connector >>>>> *connector, >>>>> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", >>>>> connector->base.id, connector->name); >>>>> drm_mode_connector_update_edid_property(connector, >>>>> NULL); >>>>> + >>>>> + /* >>>>> + * Connector status change to disconnected, time to >>>>> clean >>>>> + * cached display information >>>>> + */ >>>>> + if (connector->status == >>>>> connector_status_disconnected) >>>>> + drm_helper_clear_display_info(connector); >>>>> + >>>> I don't know if this is the right place to do this because it is >>>> a helper and I don't know if it is used by all the drivers. We >>>> may need something more general that is always called when >>>> probing modes, or force drivers that don't use the helper to use >>>> the drm_helper_clear_display_info function. As I told you >>>> before, >>>> I'm new to dri-devel so we need more comments. >>> Seems reasonable to me, since afaik all drivers do use the >>> probe helpers. >>> -Daniel >> This was my understanding too. Jose, you think there would be >> any drivers who dont use this probe ? >> - Shashank > I found only one driver that don't use this helper: vmwgfx. But, > this driver does not seem to use EDID fields, it has a list of > preferred video modes and manually adds these modes. > > So, I think it is safe to add this in the helper as long as > future drivers that use EDID use this helper also. Maybe a small > comment about this should be added in the helper declaration. > > Best regards, > Jose Miguel Abreu Sure, I will add a comment and publish a new patchset. Shashank >>>> Best regards, >>>> Jose Miguel Abreu >>>> >>>>> verbose_prune = false; >>>>> goto prune; >>>>> } >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_intel-2Dgfx&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=1G5dnBp7Y6VEifpEnDT2wKFoDRBXnxGXAnA-4883H74&s=y1M2ce128zpR_lBDPSgS_JGm-HoPIJjneK2s3tkrvyo&e=
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 7cff91e..9e97b45 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -164,6 +164,18 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) } /** + * drm_helper_clear_display_info - clean cached display information for + * hot pluggable displays, on event of hot-unplug + * @connector: connector under event + */ +void drm_helper_clear_display_info(struct drm_connector *connector) +{ + struct drm_display_info *info = &connector->display_info; + + memset(info, 0, sizeof(*info)); +} + +/** * drm_helper_probe_single_connector_modes - get complete set of display modes * @connector: connector to probe * @maxX: max width for modes @@ -288,6 +300,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n", connector->base.id, connector->name); drm_mode_connector_update_edid_property(connector, NULL); + + /* + * Connector status change to disconnected, time to clean + * cached display information + */ + if (connector->status == connector_status_disconnected) + drm_helper_clear_display_info(connector); + verbose_prune = false; goto prune; }
This patch adds a small helper function, which clears the cached information about a hot-pluggable display, from connector. On event This will run on event of a hot-unplug, keeping the connector's display info up-to-date, avoiding any errors due to invalid cached data. Cc: Jose Abreu <joabreu@synopsys.com> Suggested-by: Jose Abreu <joabreu@synopsys.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)