On 09/15/2016 05:57 PM, Ville Syrjälä wrote:
> On Thu, Sep 15, 2016 at 04:59:55PM +0200, Vincent ABRIOU wrote:
>>
>>
>> On 09/15/2016 04:27 PM, Ville Syrjälä wrote:
>>> On Wed, Sep 14, 2016 at 01:40:02PM +0200, Vincent Abriou wrote:
>>>> When a plane is going to be enabled we re-evaluate the possible crtcs
>>>> for the associated drm plane. Only the crtc on which the plane should be
>>>> displayed is considered possible until the plane is disabled.
>>>> Indeed STI hardware does not allow to dynamically change
>>>> the plane's crtc/mixer assignment when the plane is in use (gdp is
>>>> running).
>>>>
>>>> Signed-off-by: Vincent Abriou <vincent.abriou@xxxxxx>
>>>> ---
>>>> drivers/gpu/drm/sti/sti_gdp.c | 15 +++++++++++++++
>>>> drivers/gpu/drm/sti/sti_plane.h | 2 ++
>>>> 2 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
>>>> index 3fc62c1..f7cd671 100644
>>>> --- a/drivers/gpu/drm/sti/sti_gdp.c
>>>> +++ b/drivers/gpu/drm/sti/sti_gdp.c
>>>> @@ -71,6 +71,9 @@ static struct gdp_format_to_str {
>>>> #define GDP_NODE_NB_BANK 2
>>>> #define GDP_NODE_PER_FIELD 2
>>>>
>>>> +#define MAIN_CRTC_MASK BIT(0)
>>>> +#define AUX_CRTC_MASK BIT(1)
>>>> +
>>>> struct sti_gdp_node {
>>>> u32 gam_gdp_ctl;
>>>> u32 gam_gdp_agc;
>>>> @@ -690,6 +693,12 @@ static int sti_gdp_atomic_check(struct drm_plane *drm_plane,
>>>> }
>>>> }
>>>>
>>>> + /* re-evaluate the possible crtcs */
>>>> + if (mixer->id == STI_MIXER_MAIN)
>>>> + drm_plane->possible_crtcs = MAIN_CRTC_MASK;
>>>> + else
>>>> + drm_plane->possible_crtcs = AUX_CRTC_MASK;
>>>
>>> This stuff isn't meant to be changed dynamically. There's no event for
>>> telling userspace to re-examine this sort of information.
>>
>> Yes sure. But by doing this, I let the userspace the ability to fix plan
>> assignment by it self by re-evaluating the possible CRTC. Before new
>> plane assignment.
>
> Only if it would re-fetch all planes/crtcs/etc. resources between every
> operation. Doing that would suck big time. And with atomic that's not
> even a theoretical option since everything would be configured with a
> single ioctl.
>
I test with weston-1.11.0 and the behavior changed.
It is now forbidden to set a plane on 2 differents CRTC.
I will not going further on this patch.
BR
Vincent
>> The kernel driver is then flexible enough to avoid Kernel crash.
>
> If the kernel crashes due to an an unsupported plane configuration,
> then the kernel has to be fixed.
>
>>
>> BR
>> Vincent
>>
>>>
>>>> +
>>>> DRM_DEBUG_KMS("CRTC:%d (%s) drm plane:%d (%s)\n",
>>>> crtc->base.id, sti_mixer_to_str(mixer),
>>>> drm_plane->base.id, sti_plane_to_str(plane));
>>>> @@ -846,6 +855,9 @@ static void sti_gdp_atomic_disable(struct drm_plane *drm_plane,
>>>> {
>>>> struct sti_plane *plane = to_sti_plane(drm_plane);
>>>>
>>>> + /* restore possible crtcs value with the initial value */
>>>> + drm_plane->possible_crtcs = plane->init_possible_crtcs;
>>>> +
>>>> if (!drm_plane->crtc) {
>>>> DRM_DEBUG_DRIVER("drm plane:%d not enabled\n",
>>>> drm_plane->base.id);
>>>> @@ -917,6 +929,9 @@ struct drm_plane *sti_gdp_create(struct drm_device *drm_dev,
>>>>
>>>> sti_gdp_init(gdp);
>>>>
>>>> + /* store the initial value of possible crtcs */
>>>> + gdp->plane.init_possible_crtcs = possible_crtcs;
>>>> +
>>>> res = drm_universal_plane_init(drm_dev, &gdp->plane.drm_plane,
>>>> possible_crtcs,
>>>> &sti_gdp_plane_helpers_funcs,
>>>> diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
>>>> index ce3e8d6..70c5312 100644
>>>> --- a/drivers/gpu/drm/sti/sti_plane.h
>>>> +++ b/drivers/gpu/drm/sti/sti_plane.h
>>>> @@ -66,12 +66,14 @@ struct sti_fps_info {
>>>> * @plane: drm plane it is bound to (if any)
>>>> * @desc: plane type & id
>>>> * @status: to know the status of the plane
>>>> + * @init_possile_crtcs: store the initial possible crtc value
>>>> * @fps_info: frame per second info
>>>> */
>>>> struct sti_plane {
>>>> struct drm_plane drm_plane;
>>>> enum sti_plane_desc desc;
>>>> enum sti_plane_status status;
>>>> + u32 init_possible_crtcs;
>>>> struct sti_fps_info fps_info;
>>>> };
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
